[PATCH] D71660: [ValueTracking] isKnownNonZero() should take non-null-ness assumptions into consideration (PR43267)

Thu Dec 19 12:23:32 PST 2019

nikic requested changes to this revision.
nikic added inline comments.
This revision now requires changes to proceed.

================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:298
 ; CHECK-NEXT:    [[LOAD:%.*]] = load i32*, i32** [[A:%.*]], align 8
-; CHECK-NEXT:    tail call void @escape(i32* [[LOAD]])
+; CHECK-NEXT:    tail call void @escape(i32* nonnull [[LOAD]])
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32* [[LOAD]], null
----------------
nikic wrote:
> Is this transform correct? As `escape` is not `willreturn` we don't necessarily reach the assume below, or?
I think this might be a bug in isValidAssumeForContext(): https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L570 shouldn't be using std::next().

Probably this never mattered before, but I believe this needs to be fixed before this patch can land.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71660/new/

https://reviews.llvm.org/D71660