[llvm] r271151 - [SCEV] Don't always add no-wrap flags to post-inc add recs

[Sanjoy Das via llvm-commits]

So D20778 is okay to land?

On Wed, Jun 1, 2016 at 3:02 AM, Johannes Doerfert
<doerfert at cs.uni-saarland.de> wrote:
> Hey Sanjoy,
>
> On 05/30, Sanjoy Das wrote:
>> On Mon, May 30, 2016 at 3:41 AM, Johannes Doerfert
>> <doerfert at cs.uni-saarland.de> wrote:
>> > Hey Sanjoy,
>> >
>> > I looked at the example in your commit message and I am not sure why/how
>> > our test case is affected. Does your patch really only change post-inc
>> > add recurrences? I ask because I do see the same output when I change
>> > the test case (e.g., move computation around). Maybe I misinterpret
>> > "post-inc add recurrence" because in the version below I would have
>> > though that there is no "post-inc add recurrence", even though I still
>> > get:
>>
>> So there may be a terminology issue here.  By post-inc add recurrence,
>> I mean the incremented version of an add recurrence
> Yes, there was a terminology problem. Thanks for resolving it.
>
>> >  %indvars.iv = phi i64 [ %indvars.iv.next, %bb4 ], [ 0, %bb ]
>> >    -->  {0,+,1}<nuw><nsw><%bb2>                                [EXPECTED]
>>
>> ^ this is the add recurrence
>>
>> >  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>> >    -->  {1,+,1}<nuw><%bb2>                                  [NSW MISSING]
>>
>> ^ this is the post-inc version of the previous add recurrence.  It is
>> also an add recurrence in its own right, but relative to {0,+,1}, it
>> is post-inc.
>>
>>
>> The SCEV change in question fixes an issue where SCEV would transfer
>> the no-wrap flags of an add recurrence to its post-inc variant
>> unconditionally (and incorrectly).
> OK.
>
>> Now in your example the {1,+,1} add recurrence _is_ nsw (and the fact
>> that nsw is missing is a weakness in
>> ScalarEvolution::isSCEVExprNeverPoison).  We should eventually address
>> that root cause, but if we canonicalize the loop via -loop-rotate,
>> SCEV is able to prove nsw; and D20778 does just that.  Alternately, we
>> could keep around the XFAIL till the weakness in
>> ScalarEvolution::isSCEVExprNeverPoison is addressed, but generally it
>> is not that useful to make optimization passes work better over
>> non-canonical IR when we can just be better at canonicalizing the IR
>> itself.
> I am fine with the -loop-rotate option (maybe even keep a XFAILED copy
> around to increase coverage later but that is not strictly necessary).
>
> Thanks again for your patience and explanation,
>   Johannes
>
> --
>
> Johannes Doerfert
> Researcher / PhD Student
>
> Compiler Design Lab (Prof. Hack)
> Saarland University, Computer Science
> Building E1.3, Room 4.31
>
> Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
> Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert



-- 
Sanjoy Das
http://playingwithpointers.com