[PATCH] D11212: [SCEV] Apply NSW and NUW flags via poison value analysis

[Bjarke Hammersholt Roune]

broune marked an inline comment as done.
broune added a comment.

Thank you to Sanjoy and Andy for the review.


================
Comment at: include/llvm/Analysis/ValueTracking.h:293
@@ +292,3 @@
+
+  /// Return true if this function can prove that the instruction I will
+  /// always transfer execution to one of its successors (including the next
----------------
Argh, sorry about that. Done.

================
Comment at: lib/Analysis/ValueTracking.cpp:3326
@@ +3325,3 @@
+         !isa<CallInst>(I) &&    // could throw and might not terminate
+         !isa<InvokeInst>(I) &&  // might not terminate
+         !isa<ResumeInst>(I) &&  // has no successors
----------------
sanjoy wrote:
> Minor nit: `invoke`s can also throw (see https://llvm.org/bugs/show_bug.cgi?id=24185, especially https://llvm.org/bugs/show_bug.cgi?id=24185).  Btw, I got the idea for testing for 24185 bug from reading this function.
As I understand the bug, an invoke could throw somewhere other than to the landingpad successor in the CFG, if the landingpad is not a match for the exception thrown, so I updated the comment. I referred to the bug you mentioned.


http://reviews.llvm.org/D11212