[llvm] r303349 - [JumpThreading] Dont RAUW condition incorrectly
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Thu May 18 06:12:18 PDT 2017
Author: annat
Date: Thu May 18 08:12:18 2017
New Revision: 303349
URL: http://llvm.org/viewvc/llvm-project?rev=303349&view=rev
Log:
[JumpThreading] Dont RAUW condition incorrectly
Summary:
We have a bug when RAUWing the condition if experimental.guard or assumes is a use of that
condition. This is because LazyValueInfo may have used the guards/assumes to identify the
value of the condition at the end of the block. RAUW replaces the uses
at the guard/assume as well as uses before the guard/assume. Both of
these are incorrect.
For now, disable RAUW for conditions and fix the logic as a next
step: https://reviews.llvm.org/D33257
Reviewers: sanjoy, reames, trentxintong
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D33279
Modified:
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
llvm/trunk/test/Transforms/JumpThreading/assume.ll
llvm/trunk/test/Transforms/JumpThreading/fold-not-thread.ll
llvm/trunk/test/Transforms/JumpThreading/guards.ll
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=303349&r1=303348&r2=303349&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Thu May 18 08:12:18 2017
@@ -833,15 +833,13 @@ bool JumpThreadingPass::ProcessBlock(Bas
CondBr->eraseFromParent();
if (CondCmp->use_empty())
CondCmp->eraseFromParent();
- else if (CondCmp->getParent() == BB) {
- // If the fact we just learned is true for all uses of the
- // condition, replace it with a constant value
- auto *CI = Ret == LazyValueInfo::True ?
- ConstantInt::getTrue(CondCmp->getType()) :
- ConstantInt::getFalse(CondCmp->getType());
- CondCmp->replaceAllUsesWith(CI);
- CondCmp->eraseFromParent();
- }
+ // TODO: We can safely replace *some* uses of the CondInst if it has
+ // exactly one value as returned by LVI. RAUW is incorrect in the
+ // presence of guards and assumes, that have the `Cond` as the use. This
+ // is because we use the guards/assume to reason about the `Cond` value
+ // at the end of block, but RAUW unconditionally replaces all uses
+ // including the guards/assumes themselves and the uses before the
+ // guard/assume.
return true;
}
@@ -1327,14 +1325,13 @@ bool JumpThreadingPass::ProcessThreadabl
if (auto *CondInst = dyn_cast<Instruction>(Cond)) {
if (CondInst->use_empty() && !CondInst->mayHaveSideEffects())
CondInst->eraseFromParent();
- else if (OnlyVal && OnlyVal != MultipleVal &&
- CondInst->getParent() == BB) {
- // If we just learned Cond is the same value for all uses of the
- // condition, replace it with a constant value
- CondInst->replaceAllUsesWith(OnlyVal);
- if (!CondInst->mayHaveSideEffects())
- CondInst->eraseFromParent();
- }
+ // TODO: We can safely replace *some* uses of the CondInst if it has
+ // exactly one value as returned by LVI. RAUW is incorrect in the
+ // presence of guards and assumes, that have the `Cond` as the use. This
+ // is because we use the guards/assume to reason about the `Cond` value
+ // at the end of block, but RAUW unconditionally replaces all uses
+ // including the guards/assumes themselves and the uses before the
+ // guard/assume.
}
return true;
}
Modified: llvm/trunk/test/Transforms/JumpThreading/assume.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/assume.ll?rev=303349&r1=303348&r2=303349&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/assume.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/assume.ll Thu May 18 08:12:18 2017
@@ -56,6 +56,50 @@ return:
ret i32 %retval.0
}
+ at g = external global i32
+
+; Check that we do prove a fact using an assume within the block.
+; FIXME: We can fold the assume based on the semantics of assume.
+; CHECK-LABEL: @can_fold_assume
+; CHECK: %notnull = icmp ne i32* %array, null
+; CHECK-NEXT: call void @llvm.assume(i1 %notnull)
+; CHECK-NEXT: ret void
+define void @can_fold_assume(i32* %array) {
+ %notnull = icmp ne i32* %array, null
+ call void @llvm.assume(i1 %notnull)
+ br i1 %notnull, label %normal, label %error
+
+normal:
+ ret void
+
+error:
+ store atomic i32 0, i32* @g unordered, align 4
+ ret void
+}
+
+declare void @f(i1)
+declare void @exit()
+; We can fold the assume but not the uses before the assume.
+define void @dont_fold_incorrectly(i32* %array) {
+; CHECK-LABEL:@dont_fold_incorrectly
+; CHECK: @f(i1 %notnull)
+; CHECK-NEXT: exit()
+; CHECK-NEXT: assume(i1 %notnull)
+; CHECK-NEXT: ret void
+ %notnull = icmp ne i32* %array, null
+ call void @f(i1 %notnull)
+ call void @exit()
+ call void @llvm.assume(i1 %notnull)
+ br i1 %notnull, label %normal, label %error
+
+normal:
+ ret void
+
+error:
+ store atomic i32 0, i32* @g unordered, align 4
+ ret void
+}
+
; Function Attrs: nounwind
declare void @llvm.assume(i1) #1
Modified: llvm/trunk/test/Transforms/JumpThreading/fold-not-thread.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/fold-not-thread.ll?rev=303349&r1=303348&r2=303349&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/fold-not-thread.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/fold-not-thread.ll Thu May 18 08:12:18 2017
@@ -133,10 +133,10 @@ L3:
ret void
}
-; Make sure we can do the RAUW for %add...
+; FIXME: Make sure we can do the RAUW for %add...
;
; CHECK-LABEL: @rauw_if_possible(
-; CHECK: call void @f4(i32 96)
+; CHECK: call void @f4(i32 %add)
define void @rauw_if_possible(i32 %value) nounwind {
entry:
%cmp = icmp eq i32 %value, 32
Modified: llvm/trunk/test/Transforms/JumpThreading/guards.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/guards.ll?rev=303349&r1=303348&r2=303349&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/guards.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/guards.ll Thu May 18 08:12:18 2017
@@ -181,3 +181,97 @@ Exit:
; CHECK-NEXT: ret void
ret void
}
+
+declare void @never_called()
+
+; Assume the guard is always taken and we deoptimize, so we never reach the
+; branch below that guard. We should *never* change the behaviour of a guard from
+; `must deoptimize` to `may deoptimize`, since this affects the program
+; semantics.
+define void @dont_fold_guard(i8* %addr, i32 %i, i32 %length) {
+; CHECK-LABEL: dont_fold_guard
+; CHECK: experimental.guard(i1 %wide.chk)
+
+entry:
+ br label %BBPred
+
+BBPred:
+ %cond = icmp eq i8* %addr, null
+ br i1 %cond, label %zero, label %not_zero
+
+zero:
+ unreachable
+
+not_zero:
+ %c1 = icmp ult i32 %i, %length
+ %c2 = icmp eq i32 %i, 0
+ %wide.chk = and i1 %c1, %c2
+ call void(i1, ...) @llvm.experimental.guard(i1 %wide.chk) [ "deopt"() ]
+ br i1 %c2, label %unreachedBB2, label %unreachedBB1
+
+unreachedBB2:
+ call void @never_called()
+ ret void
+
+unreachedBB1:
+ ret void
+}
+
+
+; same as dont_fold_guard1 but condition %cmp is not an instruction.
+; We cannot fold the guard under any circumstance.
+; FIXME: We can merge unreachableBB2 into not_zero.
+define void @dont_fold_guard2(i8* %addr, i1 %cmp, i32 %i, i32 %length) {
+; CHECK-LABEL: dont_fold_guard2
+; CHECK: guard(i1 %cmp)
+
+entry:
+ br label %BBPred
+
+BBPred:
+ %cond = icmp eq i8* %addr, null
+ br i1 %cond, label %zero, label %not_zero
+
+zero:
+ unreachable
+
+not_zero:
+ call void(i1, ...) @llvm.experimental.guard(i1 %cmp) [ "deopt"() ]
+ br i1 %cmp, label %unreachedBB2, label %unreachedBB1
+
+unreachedBB2:
+ call void @never_called()
+ ret void
+
+unreachedBB1:
+ ret void
+}
+
+; Same as dont_fold_guard1 but use switch instead of branch.
+; triggers source code `ProcessThreadableEdges`.
+declare void @f(i1)
+define void @dont_fold_guard3(i1 %cmp1, i32 %i) nounwind {
+; CHECK-LABEL: dont_fold_guard3
+; CHECK-LABEL: L2:
+; CHECK-NEXT: %cmp = icmp eq i32 %i, 0
+; CHECK-NEXT: guard(i1 %cmp)
+; CHECK-NEXT: @f(i1 %cmp)
+; CHECK-NEXT: ret void
+entry:
+ br i1 %cmp1, label %L0, label %L3
+L0:
+ %cmp = icmp eq i32 %i, 0
+ call void(i1, ...) @llvm.experimental.guard(i1 %cmp) [ "deopt"() ]
+ switch i1 %cmp, label %L3 [
+ i1 false, label %L1
+ i1 true, label %L2
+ ]
+
+L1:
+ ret void
+L2:
+ call void @f(i1 %cmp)
+ ret void
+L3:
+ ret void
+}
More information about the llvm-commits
mailing list