[Bug rtl-optimization/81174] bswap not recognized in |= statement
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81174
--- Comment #3 from Thomas Preud'homme ---
(In reply to Jakub Jelinek from comment #2)
> Simplified testcase:
> static inline unsigned
> bar (unsigned x)
> {
> return ((x & 0x00ff) << 24) | ((x & 0xff00) << 8)
>| ((x & 0x00ff) >> 8) | ((x & 0xff00) >> 24);
> }
>
> unsigned
> foo (unsigned p, unsigned q)
> {
> p &= ~0x180U;
> p |= bar ((q << 7) & 0x180U);
> return p;
> }
>
> The problem is that with |= instead of += the reassoc pass is invoked first
> and reassociates the many | operands and then the bswap pass can't recognize
> the pattern.
> So, either we should consider moving the bswap pass 6 passes earlier (i.e.
> before reassoc1), or if we want to catch that we'd need to be able to
> recognize | operands unrelated to the bswap pattern mixed with | operands
> related to that, and replace just the ones related to the bswap and leave
> the others in.
Two things need to happen for bswap to catch the byteswap in this testcase:
1) do some reassociation to keep expression involving a single source
2) look for byteswap by stopping at all level of the recursion.
Let me illustrate with the testcase reduced by Jakub. Here's the gimple I get
with this testcase before bswap:
foo (unsigned int p, unsigned int q)
{
unsigned int _1;
unsigned int _2;
unsigned int _7;
unsigned int _9;
unsigned int _10;
unsigned int _12;
unsigned int _13;
unsigned int _15;
unsigned int _17;
unsigned int _18;
unsigned int _19;
[100.00%]:
p_4 = p_3(D) & 0xFE7F;
_1 = q_5(D) << 7;
_2 = _1 & 0x0180;
_7 = _2 << 24;
_9 = _2 << 8;
_10 = _9 & 0x00FF;
_12 = _2 >> 8;
_13 = _12 & 0xFF00;
_15 = _2 >> 24;
_17 = _7 | _15;
_18 = p_4 | _17;
_19 = _10 | _18;
p_8 = _13 | _19;
return p_8;
}
1) is needed to ignore expressions based on p when looking at the statement
defining p_8. Currently because p_8 ORs _13 (based on q) and _19 (based on p
and q) the match would fail. Similarly for _19 and _18. Then when looking at
_17 some statements would be missing to get a byteswap. So it would be
necessary to start at p_8 but realize that | is associative and thus
reassociate by only following expressions based on q. bswap need to be extended
so that the analysis for one statement could return several results (here one
for the expression based on p and one for the expression based on q) as well as
the operation that links these results (OR).
2) is needed to stop once statement defining _2 is reached. Currently bswap
would continue to recurse through _1 but due to the bitwise AND the pattern
would not be a byteswap of q and the match would fail. Some form of memoization
would be needed to not make this expensive.
[Bug rtl-optimization/81174] bswap not recognized in |= statement
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81174
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org,
||thopre01 at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek ---
Simplified testcase:
static inline unsigned
bar (unsigned x)
{
return ((x & 0x00ff) << 24) | ((x & 0xff00) << 8)
| ((x & 0x00ff) >> 8) | ((x & 0xff00) >> 24);
}
unsigned
foo (unsigned p, unsigned q)
{
p &= ~0x180U;
p |= bar ((q << 7) & 0x180U);
return p;
}
The problem is that with |= instead of += the reassoc pass is invoked first and
reassociates the many | operands and then the bswap pass can't recognize the
pattern.
So, either we should consider moving the bswap pass 6 passes earlier (i.e.
before reassoc1), or if we want to catch that we'd need to be able to recognize
| operands unrelated to the bswap pattern mixed with | operands related to
that, and replace just the ones related to the bswap and leave the others in.
One could have also several bswap patterns results ored together with reassoc
reordering it, say for p |= bar ((q << 7) & 0x180U) | bar ((r << 7) &
0x180U); above with another unsigned r argument. In that case it would be
better to or the two bar arguments and bswap the result, i.e.
p |= __builtin_bswap32 (((q << 7) & 0x180U) | ((r << 7) & 0x180U));
[Bug rtl-optimization/81174] bswap not recognized in |= statement
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81174 Allan Jensen changed: What|Removed |Added Version|6.1.1 |7.1.0 --- Comment #1 from Allan Jensen --- Also reproduced with gcc 4.8, 4.9, 5 and 7. Works in clang. With gcc 6+ it would sometimes work if bswap was called as part of a constructor.
