[PATCH] Better __ashlDI3, __ashrDI3 and __lshrDI3 functions, plus fixed __bswapsi2 function

Jeff Law law@redhat.com
Mon Nov 30 01:06:53 GMT 2020



On 11/11/20 1:33 AM, Stefan Kanthak wrote:
> Jakub Jelinek <jakub@redhat.com> wrote:
>
>> On Tue, Nov 10, 2020 at 04:48:10PM -0700, Jeff Law via Gcc-patches wrote:
>>>> @@ -486,10 +425,10 @@
>>>>  SItype
>>>>  __bswapsi2 (SItype u)
>>>>  {
>>>> -  return ((((u) & 0xff000000) >> 24)
>>>> -   | (((u) & 0x00ff0000) >>  8)
>>>> -   | (((u) & 0x0000ff00) <<  8)
>>>> -   | (((u) & 0x000000ff) << 24));
>>>> +  return ((((u) & 0xff000000u) >> 24)
>>>> +   | (((u) & 0x00ff0000u) >>  8)
>>>> +   | (((u) & 0x0000ff00u) <<  8)
>>>> +   | (((u) & 0x000000ffu) << 24));
>>> What's the point of this change? I'm not sure how the signedness of the
>>> constant really matters here.
>> Note 0xff000000 is implicitly 0xff000000U because it doesn't fit into signed
>> int, and that is the only one where the logical vs. arithmetic right shift
>> really matters for correct behavior.
> Ouch: that's but not the point here; what matters is the undefined behaviour of
>       ((u) & 0x000000ff) << 24
>
> 0x000000ff is a signed int, so (u) & 0x000000ff is signed too -- and producing
> a negative value (or overflow) from the left-shift of a signed int, i.e.
> shifting into (or beyond) the sign bit, is undefined behaviour!
I've pushed the bswapsi2 changes to the trunk.

Thanks.
jeff



More information about the Gcc-patches mailing list