[PATCH] ScalarEvolution incorrectly assumes that the start of certain add recurrences don't overflow

[hfinkel at anl.gov]

================
Comment at: lib/Analysis/ScalarEvolution.cpp:1370
@@ -1369,1 +1369,3 @@
+  // cannot assume the increment of PreStart to not overflow even if PreAR does
+  // not overflow.
 
----------------
sanjoy wrote:
> hfinkel wrote:
> > Seems like we could still pick up some trivial cases (loops with only one exit block, only one backedge, etc.). Maybe we could do more with a dominance query?
> > 
> I think we can prove overflow if the loop has only one exit.  Having only one backedge does not give us anything though (the counterexample in this change also has only one backedge).
> 
> That said, it is not clear to me how important this optimization is (for instance, removing the optimization does not fail any existing unit tests).  I'd like to fix the bug first and check in a correct version of this optimization as a later change.  I plan to generalize this part of SCEV to `nuw`s so I'll be touching this area in the future anyway.
Well, I don't know either. Not affecting regressions tests, unfortunately, is not a good measure of how important it is. Please add the loop-has-one-exit check, and a regression test -- this handles some large fraction of performance-sensitive loops in general --  and otherwise, LGTM.

(We should try not to regress trunk, even performance-wise, any more than necessary, even in between official releases, because most of us pull internal releases on offset schedules).

http://reviews.llvm.org/D7331

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/