maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[SimpleLoopUnswitch] Don't unswitch constant conditions 

Added an additional check for constants after simplification of
"select _, true, false" pattern. We need to prevent attempts to unswitch constant
conditions for two reasons:
a) Doing that doesn't make any sense, in the best case it will just burn
some compile time.
b) SimpleLoopUnswitch isn't designed to unswitch constant conditions
(due to (a)), so attempting that can cause miscompiles. The attached
testcase is an example of such miscompile.

Also added an assertion that'll make sure we aren't trying to replace
constants, so it will help us prevent such bugs in future. The assertion
from D110751 is another layer of protection against such cases.

Reviewed By: aeubanks
Differential Revision: https://reviews.llvm.org/D110752
This commit is contained in:
Daniil Suchkov 2021-09-28 23:51:15 +00:00
parent bdd52e8bae
commit 45bd8d9477
2 changed files with 12 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
View File

@ -2370,7 +2370,9 @@ static void unswitchNontrivialInvariants(
ConstantInt *ContinueReplacement =
Direction ? ConstantInt::getFalse(BI->getContext())
: ConstantInt::getTrue(BI->getContext());
for (Value *Invariant : Invariants)
for (Value *Invariant : Invariants) {
assert(!isa<Constant>(Invariant) &&
"Should not be replacing constant values!");
// Use make_early_inc_range here as set invalidates the iterator.
for (Use &U : llvm::make_early_inc_range(Invariant->uses())) {
Instruction *UserI = dyn_cast<Instruction>(U.getUser());
@ -2385,6 +2387,7 @@ static void unswitchNontrivialInvariants(
DT.dominates(ClonedPH, UserI->getParent()))
U.set(UnswitchedReplacement);
}
}
}
// We can change which blocks are exit blocks of all the cloned sibling
@ -2727,6 +2730,9 @@ static bool unswitchBestCondition(
Cond = CondNext;
BI->setCondition(Cond);
if (isa<Constant>(Cond))
continue;
if (L.isLoopInvariant(BI->getCondition())) {
UnswitchCandidates.push_back({BI, {BI->getCondition()}});
continue;

19
llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-invariant-select-bug.ll
View File

@ -1,31 +1,22 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes='simple-loop-unswitch<nontrivial>' -S < %s | FileCheck %s
; FIXME: We should not replace `true` with `false` here!
; If we try to replace uses of `true` outside of `@foo`, we'll see it here.
define i1 @bar() {
; CHECK-LABEL: @bar(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: ret i1 true
;
ret i1 true
}
; FIXME: We shouldn't unswitch this loop!
; We shouldn't unswitch this loop.
define void @foo() {
; CHECK-LABEL: @foo(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 true, label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
; CHECK: entry.split.us:
; CHECK-NEXT: br label [[HEADER_US:%.*]]
; CHECK: header.us:
; CHECK-NEXT: [[VAL_US:%.*]] = select i1 true, i1 true, i1 false
; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
; CHECK: exit.split.us:
; CHECK-NEXT: br label [[EXIT:%.*]]
; CHECK: entry.split:
; CHECK-NEXT: br label [[HEADER:%.*]]
; CHECK: header:
; CHECK-NEXT: [[VAL:%.*]] = select i1 false, i1 false, i1 false
; CHECK-NEXT: br label [[HEADER]]
; CHECK-NEXT: [[VAL:%.*]] = select i1 true, i1 true, i1 false
; CHECK-NEXT: br i1 true, label [[EXIT:%.*]], label [[HEADER]]
; CHECK: exit:
; CHECK-NEXT: ret void
;