<div dir="ltr"><div>Hi Sanjoy,</div><div><br></div><div>Thank you for looking into this!</div><div>Yes, your patch does fix my larger test case too. My algorithm gets double performance improvement with the patch, as the loop now has a smaller instruction set and succeeds to unroll w/o any extra #pragma's.</div><div><br></div><div>I also ran the LLVM tests against the patch. There are 6 new failures:</div><div>    Analysis/LoopAccessAnalysis/number-of-memchecks.ll</div><div>    Analysis/LoopAccessAnalysis/reverse-memcheck-bounds.ll</div><div>    Analysis/ScalarEvolution/flags-from-poison.ll</div><div>    Analysis/ScalarEvolution/nsw-offset-assume.ll</div><div>    Analysis/ScalarEvolution/nsw-offset.ll</div><div>    Analysis/ScalarEvolution/nsw.ll</div><div><br></div><div>I haven't inspected these failures in detail yet, but it's likely the tests merely need to be adjusted to handle the new no-wrap flags the patch introduced. I will double-check this soon.</div><div><br></div><div>Kind regards,</div><div>Oleg</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 22, 2016 at 11:02 AM, Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Oleg,<br>
<br>
I think the problem here is that SCEV forgets to propagate no-wrap<br>
flags when folding "{S,+,X}+T ==> {S+T,+,X}".<br>
<br>
I haven't carefully thought about the implications and whether the<br>
change is even correct, but the appended patch fixes the test case<br>
you've attached.  I'll give it some more thought and if it holds up<br>
I'll check it in in the next few days.  Meanwhile if you have a larger<br>
test case that you extracted indvar_test.cpp from, I'd be interested<br>
in hearing if this change works there as well.<br>
<br>
diff --git a/lib/Analysis/ScalarEvolution.cpp b/lib/Analysis/ScalarEvolution.cpp<br>
index 39ced1e..2e87902 100644<br>
--- a/lib/Analysis/ScalarEvolution.cpp<br>
+++ b/lib/Analysis/ScalarEvolution.cpp<br>
@@ -2274,19 +2274,19 @@ const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *> &Ops,<br>
       }<br>
<br>
     // If we found some loop invariants, fold them into the recurrence.<br>
     if (!LIOps.empty()) {<br>
       //  NLI + LI + {Start,+,Step}  -->  NLI + {LI+Start,+,Step}<br>
       LIOps.push_back(AddRec->getStart());<br>
<br>
       SmallVector<const SCEV *, 4> AddRecOps(AddRec->op_begin(),<br>
                                              AddRec->op_end());<br>
-      AddRecOps[0] = getAddExpr(LIOps);<br>
+      AddRecOps[0] = getAddExpr(LIOps, Flags);<br>
<br>
       // Build the new addrec. Propagate the NUW and NSW flags if both the<br>
       // outer add and the inner addrec are guaranteed to have no overflow.<br>
       // Always propagate NW.<br>
       Flags = AddRec->getNoWrapFlags(setFlags(Flags, SCEV::FlagNW));<br>
       const SCEV *NewRec = getAddRecExpr(AddRecOps, AddRecLoop, Flags);<br>
<br>
       // If all of the other operands were loop invariant, we are done.<br>
       if (Ops.size() == 1) return NewRec;<br>
<br>
Thanks!<span class="HOEnZb"><font color="#888888"><br>
-- Sanjoy<br>
</font></span></blockquote></div><br></div>