On 11/11/20 1:33 AM, Stefan Kanthak wrote:
> Jakub Jelinek <ja...@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
