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

[Eli Friedman]

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).

-Eli