This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]