[Mesa-dev] [PATCH 3/3] nir: Simplify ine/ieq(ineg(a), a) -> ine/ieq(a, 0)

[Thomas Helland]

2015-08-07 0:08 GMT+02:00 Matt Turner <mattst88 at gmail.com>:
> (Don't top quote)
>
> On Thu, Aug 6, 2015 at 7:22 AM, Boyan Ding <boyan.j.ding at gmail.com> wrote:
>> But, isn't -INT_MIN == INT_MIN?
>
> In C (-INT_MIN) is undefined since it relies on behavior of signed
> integer overflow.
>
> In GLSL though, I think you're right. The spec says that addition
> subtraction, and shift operations (weird to list shift operations)
> will wrap. Multiplication and division however do not -- when they
> overflow their results are undefined (per a clarification/change in
> the 4.30 spec).
>
> So yeah, I don't think this is safe.
>
> By the way, a good sanity check is to write a C function and compile
> with gcc and see if it performs the optimization you're implementing.
>
> bool int_cmp(int x) {
>     return -x == x;
> }
>
> 0000000000000000 <int_cmp>:
>    0: 89 f8                 mov    %edi,%eax
>    2: 01 f8                 add    %edi,%eax
>    4: 0f 94 c0             sete   %al
>    7: c3                   retq
>
> I'm not actually sure what it's doing, but the function does return
> true for INT_MIN.

Then we arrived at the same conclusion.
This patch is total rubish.
I actually did almost the same trick you suggest Matt,
when Boyan pointed out the issue.
I just hooked up a

if (-INT_MIN == INT_MIN)
   fprintf(stderr, "Yuuuup, %d, %d", -INT_MIN, INT_MIN);

in the middle of the VRP pass and gave it a whirl.
Needless to say, there was a bit of action on stderr .

Regards
Thomas