diff options
author | Roman Lebedev <lebedev.ri@gmail.com> | 2019-12-20 00:10:07 +0300 |
---|---|---|
committer | Roman Lebedev <lebedev.ri@gmail.com> | 2019-12-20 01:47:57 +0300 |
commit | 92083a295a02f46ecd168438d2145a0ca3c9b6ec (patch) | |
tree | 08d4f9b3b037319f874a8afa60597710e3f7ff69 | |
parent | ffcae008d74c2008697211e66a72ff9a20696bc9 (diff) | |
download | bcm5719-llvm-92083a295a02f46ecd168438d2145a0ca3c9b6ec.tar.gz bcm5719-llvm-92083a295a02f46ecd168438d2145a0ca3c9b6ec.zip |
[ValueTracking] isValidAssumeForContext(): CxtI itself also must transfer execution to successor
This is a pretty rare case, when CxtI and assume are
in the same basic block, with assume being located later.
We were already checking that assumption was guaranteed to be
executed, but we omitted CxtI itself from consideration,
and as the test (miscompile) shows, that is incorrect.
As noted in D71660 review by @nikic.
-rw-r--r-- | llvm/lib/Analysis/ValueTracking.cpp | 9 | ||||
-rw-r--r-- | llvm/test/Transforms/InstCombine/assume.ll | 2 |
2 files changed, 5 insertions, 6 deletions
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index ab021aece2f..c98140e2e93 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -566,11 +566,10 @@ bool llvm::isValidAssumeForContext(const Instruction *Inv, if (Inv == CxtI) return false; - // The context comes first, but they're both in the same block. Make sure - // there is nothing in between that might interrupt the control flow. - for (BasicBlock::const_iterator I = - std::next(BasicBlock::const_iterator(CxtI)), IE(Inv); - I != IE; ++I) + // The context comes first, but they're both in the same block. + // Make sure there is nothing in between that might interrupt + // the control flow, not even CxtI itself. + for (BasicBlock::const_iterator I(CxtI), IE(Inv); I != IE; ++I) if (!isGuaranteedToTransferExecutionToSuccessor(&*I)) return false; diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll index 244413eb56b..bdff7963f4b 100644 --- a/llvm/test/Transforms/InstCombine/assume.ll +++ b/llvm/test/Transforms/InstCombine/assume.ll @@ -314,7 +314,7 @@ define i1 @nonnull4(i32** %a) { define i1 @nonnull5(i32** %a) { ; CHECK-LABEL: @nonnull5( ; CHECK-NEXT: [[LOAD:%.*]] = load i32*, i32** [[A:%.*]], align 8 -; CHECK-NEXT: tail call void @escape(i32* nonnull [[LOAD]]) +; CHECK-NEXT: tail call void @escape(i32* [[LOAD]]) ; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32* [[LOAD]], null ; CHECK-NEXT: tail call void @llvm.assume(i1 [[CMP]]) ; CHECK-NEXT: ret i1 false |