Re: [Patch] sext_hwi: Avoid left shift of negative value undefined behaviour

[Jeff Law <law@redhat.com>]

On 08/18/2015 09:29 AM, Mikael Morin wrote:
> Le 14/08/2015 09:29, Richard Biener a écrit :
>> On Thu, Aug 13, 2015 at 7:47 PM, Mike Stump <mikestump@comcast.net>
>> wrote:
>>> On Aug 13, 2015, at 3:05 AM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> Ok, then guard the << >> with __GCC__ and do the expensive bit stuff
>>>> otherwise.  Just to cater for other host compilers doing sth unsensibly
>>>> implementation defined.
>>>
> I haven't found __GCC__ used anywhere in the source, but I have found
> __GNUC__ both in the source and the documentation, so I have used that.
>
>>> Ick. The guard should be specific to the implementation defined
>>> semantic or undefined semantic, and then when compiling with gcc, we
>>> turn it on.  My take is that when we do this, we should add the 5 or
>>> 10 other most popular compilers to the list of how they behave so
>>> that they all do the cheap path code as well.  If the language
>>> standard were serious in this area, it would specify a header file
>>> that can define the implementation defined and undefined semantics
>>> that are interesting for portable code.  It isn’t.  If it were, we
>>> would then just use the standard defined guards.
>>
> I have used the __GNUC__ macro instead of a direct feature test or a
> list of the 10 most common compilers, to avoid the #else branch being
> dead basically everywhere.  Maybe the #else branch should just be dropped.
>
>> GCC is always used to build stage2+ so I don't see the need to handle
>> alternate host compilers.
>>
>
> Bootstrapped and regression tested on x86_64-pc-linux-gnu.
> Also bootstrapped with clang, but I don't think it means anything.
> What do you think, OK?
>
> Mikael
>
>
>
> noub_sext_2.CL
>
>
> 2015-08-18  Mikael Morin<mikael@gcc.gnu.org>
>
> 	PR other/67042
> 	* hwint.h (sext_hwi): Switch to unsigned for the left shift, and
> 	conditionalize the whole on __GNUC__.  Add fallback code
> 	depending neither on undefined nor implementation-defined behaviour.
>
I think there was some folks that wanted to see a test for whether or 
not the faster version could be used during a stage1 build with a 
non-gcc compiler.


That would presumably be some kind of autoconf test to see what happens 
when shifting into the sign bit.  But I think that'd be a hell of a test 
to write in practice.  I'm sure the simple test would work, but it's far 
more important to know what the compiler is willing to guarantee in the 
general sense, particularly the interactions with the optimizers and 
vrp-like analysis.

I'm OK with the __GNUC__ conditional.

OK for the trunk.

Thanks,
Jeff