This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR81503 (SLSR invalid fold)
-- Bill
Bill Schmidt, Ph.D.
GCC for Linux on Power
Linux on Power Toolchain
IBM Linux Technology Center
wschmidt@linux.vnet.ibm.com
> On Aug 3, 2017, at 11:39 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Aug 03, 2017 at 11:29:44AM -0500, Bill Schmidt wrote:
>>> And, wouldn't it be more readable to use:
>>> && (TYPE_UNSIGNED (target_type)
>>> ? (wi::eq_p (bump, wi::zext (bump, prec))
>>> || wi::eq_p (bump + wi::to_widest (maxval) + 1,
>>> wi::zext (bump, prec)))
>>> : wi::eq_p (bump, wi::sext (bump, prec)))
>>> ?
>>
>> Probably. As noted, it's all becoming a bit unreadable with too
>> much negative logic in a long conditional, so I want to clean that
>> up in a follow-up.
>>
>>> For TYPE_UNSIGNED, do you actually need any restriction?
>>> What kind of bump values are wrong for unsigned types and why?
>>
>> If the value of the bump is actually larger than the precision of the
>> type (not likely except for quite small types), say 2 * (maxval + 1)
>> which is congruent to 0, the replacement is wrong.
>
> Ah, ok. Anyway, for unsigned type, perhaps it could be written as:
> && (TYPE_UNSIGNED (target_type)
> ? wi::eq_p (wi::neg_p (bump) ? bump + wi::to_widest (maxval) + 1
> : bump, wi::zext (bump, prec))
> : wi::eq_p (bump, wi::sext (bump, prec)))
> I mean, if bump >= 0, then the bump + wi::to_widest (maxval) + 1
> value has no chance to be equal to zero extended bump, and
> for bump < 0 only that one has a chance.
Yeah, that's true. And arguably my case for the really large bump
causing problems is kind of thin, because the program is probably
already broken in that case anyway. But I think I will sleep better
having the check in there, as somebody other than SLSR will catch
the bug then. ;-)
Thanks for all the help with this one. These corner cases are
always tricky, and I appreciate the extra eyeballs.
Bill
>
> Jakub
>