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.
Whether (((u) & 0x00ff0000) >> 8) is expanded as logical or arithmetic shift
doesn't really matter for code behavior, though perhaps we could during
expansion check if the most significant bit is known to be zero as in this
case and ask target about arithmetic vs. logical right shift costs and
choose the less costly if they aren't even.
Jakub
