Index: lib/Transforms/Scalar/JumpThreading.cpp =================================================================== --- lib/Transforms/Scalar/JumpThreading.cpp +++ lib/Transforms/Scalar/JumpThreading.cpp @@ -833,15 +833,12 @@ 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 of circular logic where the value of `Cond` is calculated + // by LVI based on the guard/assume and we are replacing the + // guard/assume with that value. return true; } @@ -1327,14 +1324,12 @@ if (auto *CondInst = dyn_cast(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 of circular logic where the value of `Cond` is calculated + // by LVI based on the guard/assume and we are replacing the + // guard/assume with that value. } return true; } Index: test/Transforms/JumpThreading/assume.ll =================================================================== --- test/Transforms/JumpThreading/assume.ll +++ test/Transforms/JumpThreading/assume.ll @@ -56,6 +56,28 @@ ret i32 %retval.0 } +@g = external global i32 + +; Check that we do prove a fact using an assume within the block and +; that we *don't* use circular logic to remove the assume. +; CHECK-LABEL: @dont_fold_assume +; CHECK: %notnull = icmp ne i32* %array, null +; CHECK-NEXT: call void @llvm.assume(i1 %notnull) +; CHECK-NEXT: ret void +define void @dont_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 +} + + ; Function Attrs: nounwind declare void @llvm.assume(i1) #1 Index: test/Transforms/JumpThreading/fold-not-thread.ll =================================================================== --- test/Transforms/JumpThreading/fold-not-thread.ll +++ test/Transforms/JumpThreading/fold-not-thread.ll @@ -133,10 +133,10 @@ 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 Index: test/Transforms/JumpThreading/guards.ll =================================================================== --- test/Transforms/JumpThreading/guards.ll +++ test/Transforms/JumpThreading/guards.ll @@ -181,3 +181,97 @@ ; 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 +}