[llvm] 92083a2 - [ValueTracking] isValidAssumeForContext(): CxtI itself also must transfer execution to successor

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2019-12-20T01:47:57+03:00
New Revision: 92083a295a02f46ecd168438d2145a0ca3c9b6ec

URL: https://github.com/llvm/llvm-project/commit/92083a295a02f46ecd168438d2145a0ca3c9b6ec
DIFF: https://github.com/llvm/llvm-project/commit/92083a295a02f46ecd168438d2145a0ca3c9b6ec.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/lib/Analysis/ValueTracking.cpp
    llvm/test/Transforms/InstCombine/assume.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index ab021aece2f4..c98140e2e935 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 244413eb56b0..bdff7963f4bc 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