[PATCH] D97537: [Codegenprepare] Use IV increment instead of IV if we can prove it is not a poisoned value

[Philip Reames via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:3860
+      if (OIVInc->hasNoSignedWrap() || OIVInc->hasNoUnsignedWrap()) {
+        bool ProvedNoPoison = false;
+        // If IVInc is represented as wrapping binary operator with no wrap
----------------
The code that follows looks analogous to programUndefinedIfPoison from ValueTracking.hpp.  (Though you handle slightly different cases.)  I'd encourage you to split this into a helper function and use analogous naming conventions to make it easier to follow.  

Side note - I'd suggest trying the existing implementation from ValueTracking.  While the reason is definitely different, if that's enough to catch your interesting cases, you can save a bunch of work.  :)


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:3874
+          ICmpInst::Predicate Pred;
+          if (match(BB->getTerminator(),
+                    m_Br(m_ICmp(Pred, m_Specific(IVInc), m_Value()),
----------------
Detail on the proof here -- the fact the branch is a loop exit is definitely irrelevant.

It's not clear to me whether branching on poison is immediate UB.  LangRef seems to say "yes", but the code in ValueTracking seems to say "no".  We need to draw in the experts here.  (Juneyoung)


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

https://reviews.llvm.org/D97537