On Sun, May 20, 2018 at 1:01 PM, Alexander Monakov <amona...@ispras.ru> wrote:
> On Sun, 20 May 2018, Yuri Gribov wrote:
>
>> Hi all,
>>
>> This fixes PR 85822 by removing incorrect reversal of condition in VRP
>> assertion. Bootstrapped and regtested on x86_64.
>>
>> Ok for trunk?
>
> Please address the following issues:
>
> Use correct PR reference in Changelog.

Ah, right.

> Double-check the comment before the function, I think NE_EXPR and EQ_EXPR
> should be swapped there.

Thanks, fixed.

> Address Richard's request from the bug report:
>
>>> Ok, please make sure to say why not doing anything special for negative
>>> numbers is ok.

True, I didn't notice the "to say" part :/

Comparison
  (a & 11...100...0) == XX...X00..0  // RHS XX...X is covered by
11...100...0 mask i.e. (RHS & MASK) == RHS
means that 'a' can have values
  XX...X00...00
  XX...X00...01
  XX...X00...10
  ...
  XX...X11...11
Both for positive and negative, signed and unsigned RHSs the first
number is less than the last one so we can derive
  XX...X00...00 <= a
  a <= XX...X11...11
in all cases.

> Note there are at least three special cases that are handled incorrectly 
> either
> before or after the patch:
>
>  - not two's complement integers

Commented by Richard.

>  - mask being 0
>  - mask being ~0

Done (AND with 0 or -1 is removed during Gimple generation so I didn't
bother to optimize this special case, just bail out).

-Y

Attachment: pr85822-2.patch
Description: Binary data

Reply via email to