[llvm-commits] PATCH: Preserving the 'nsw' flag in the instruction combiner.

[Nick Lewycky]

On 10 August 2011 13:43, Nick Lewycky <nlewycky at google.com> wrote:

> On 10 August 2011 13:19, Eli Friedman <eli.friedman at gmail.com> wrote:
>
>> On Wed, Aug 10, 2011 at 1:11 PM, Nick Lewycky <nlewycky at google.com>
>> wrote:
>> > On 10 August 2011 13:07, Eli Friedman <eli.friedman at gmail.com> wrote:
>> >>
>> >> On Wed, Aug 10, 2011 at 12:57 PM, Nick Lewycky <nlewycky at google.com>
>> >> wrote:
>> >> > On 10 August 2011 12:37, Pranav Bhandarkar <pranavb at codeaurora.org>
>> >> > wrote:
>> >> >>
>> >> >> Hi Nick,
>> >> >>
>> >> >> >>Thanks for working on this! Firstly, I have a high-level question:
>> >> >> >> why
>> >> >> can't (X +nsw (C1 +nsw C2) always become (X +nsw C3)? Your patch
>> spends
>> >> >> a
>> >> >> lot of time verifying that >>overflow couldn't occur, but you're
>> given
>> >> >> an
>> >> >> assumption a priori that it can't because the nsw flag is present.
>> >> >>
>> >> >> The reason is that 127 (C1) + 2 (C2) = 129 (C3) is not strictly true
>> >> >> always.
>> >> >> For instance, when the type is i8, then C3 is -127.
>> >> >
>> >> > 127 + 2 = 129 remains true with i8 values, because i8 -127 = i8 129.
>> >> > They
>> >> > have the same bit pattern.
>> >> > Can you pick values for X, C1 and C2 which still satisfy the +nsw
>> >> > relationship between each other that can't be converted into X +nsw
>> C3
>> >> > where
>> >> > C3 = C1+C2 (no nsw requirement)? 127 and 2 don't work as
>> >> > counter-examples
>> >> > because that triggers undefined behaviour (it violates the 'nsw' bit
>> >> > present
>> >> > on the add).
>> >>
>> >> X = -128, C1 = 127, C1 = 2?
>> >
>> > I assume C2 = 2 and all i8 values. That doesn't work because the
>> expression
>> > was "(X +nsw (C1 +nsw C2)" and 127 +nsw 2 invokes undefined behaviour,
>> > crossing from 127 to -128/128 to reach -127/129. In fact nsw-adding
>> anything
>> > positive to 127 is invalid, so you'd have to be sure to pick a negative
>> C2
>> > given a C1 of 127.
>>
>> The original expression is ((X +nsw C1) +nsw C2).
>>
>
> Doh! Thanks.
>
> So the original code with your values does:
> ((X +nsw C1) +nsw C2) --> ((-128 +nsw 127) +nsw 2) = 1, and none of the
> nsw's are violated. Good.
>
> After my proposed transform, we get:
> C3 = C1 + C2 --> C3 = 127 + 2 = -127.
> (X +nsw C3) --> (-128 +nsw -127) = 1, and the nsw wasn't violated (we moved
> towards zero 129 times, starting from -128 up past -1 and crossed the
> unsigned boundary to 0 finally landing at 1, never crossing the signed
> boundary).
>

Ok, off-list, Jeff Yasskin convinced me that this can't be the right way to
model nuw/nsw, because "add i8 %X, -1" would cross over both nuw and nsw
boundaries all the time. His model is to do the arithmetic in non-modular
space then check that the result is within [-128..127] for nsw or [0..255]
for nuw. That makes sense, and I'll go back and re-evaluate.

Sorry for the noise!

Nick


>
> I may still be wrong, but I don't think so. :-) More generally I think we
> can always take the intersection of the nuw/nsw/exact flags available on any
> string of the same instruction. Start mixing instructions and things get
> messy though.
>
> Nick
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110810/0347818f/attachment.html>