On Sun, Jul 11, 2021 at 11:48 AM Roger Sayle <ro...@nextmovesoftware.com> wrote: > > > My sincere apologies for the breakage. My recent patch to fold > bswapN(x)>>C where the constant C was large enough that the result > only contains bits from the low byte, and can therefore avoid > the byte swap contains a minor logic error. The pattern contains > a convert? allowing an extension to occur between the bswap and > the shift. The logic is correct if there's no extension, or the > extension has the same sign as the shift, but I'd mistakenly > convinced myself that these couldn't have different signedness. > > (T)bswap16(x)>>12 is (T)((unsigned char)x>>4) or (T)((signed char)x>>4). > The bug is that for zero-extensions to signed type T, we need to use > the unsigned char variant [the signedness of the byte shift is not > (always) the same as the signedness of T and the original shift]. > > Then because I'm now paranoid, I've also added a clause to handle > the hypothetical (but in practice impossible) sign-extension to an > unsigned type T, which can implemented as (T)(x<<8)>>12. > > This patch has been tested on x86_64-pc-linux-gnu with a "make > bootstrap" and "make -k check" with no new failures, and a new > testcase to confirm it fixes the regression. > > Ok for mainline?
OK. Thanks, Richard. > 2021-07-11 Roger Sayle <ro...@nextmovesoftware.com> > > gcc/ChangeLog > PR tree-optimization/101403 > * gcc/match.pd ((T)bswap(X)>>C): Correctly handle cases where > signedness of the shift is not the same as the signedness of > the type extension. > > gcc/testsuite/ChangeLog > PR tree-optimization/101403 > * gcc.dg/pr101403.c: New test case. > > > Sorry again, > Roger > -- > Roger Sayle > NextMove Software > Cambridge, UK >
