On 3/25/2026 5:47 PM, Jeffrey Law wrote:
>
>
>
>
> On 3/2/2026 6:26 AM, Daniel Henrique Barboza wrote:
>> From: Daniel Barboza <[email protected]>
>>
>> Given a comparison with the format
>>
>> A << CST1 EQ|NE CST2
>>
>> Turn it into:
>>
>> A & CSTmask EQ|NE CST2 >> CST1
>>
>> Where 'CSTmask' is a bitmask that filters A bits that would be discarded
>> during the shift. The idea is that a bit_and is, on most common targets
>> at least, more efficient than a lshift.
>>
>> Note that we're not handling the case "A << 1 != CST2" because this is a
>> pattern that is optimized by tree-ssa-loop-niter.cc, turning a whole
>> loop into a single CTZ().
>>
>> Bootstrapped and regression tested on x86. Includes aarch64 regression
>> changes pointed by Linaro automatic CI bot.
>>
>> PR tree-optimization/124019
>>
>> gcc/ChangeLog:
>>
>> * match.pd (`A<<CST1 EQ|NE CST2 -> (A&CSTmask) EQ|NE (CST2>>CST1)`):
>> New pattern.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/aarch64/cmp_shifted_reg_1.c: changed shift
>> immediate to "1" to dodge this optimization and keep
>> generating the shift and compare.
>> * gcc.target/aarch64/negs.c: changed shift immediate to "1" in
>> negs_si_test3 and negs_di_test3, and switched the comparison
>> from "== 0" to "!= 0", to dodge this optimization and keep
>> generating "negs". Expected value for both were updated
>> accordingly.
>> * gcc.dg/tree-ssa/pr124019.c: New test.
[ ... ]
A follow-up. This was included in last night's tester run and sure
enough it tripped a code quality regression on cris-elf.
cris/pr93372-4.c
It looks like at the heart of the matter is the new pattern (which a
reminder is a canonicalization pattern) will change that test so that
it's testing the pre-shifted value against 1 rather than the
post-shifted value against zero.
ie, we get this without the new match.pd pattern:
c_4 = a_3(D) >> 1;
_1 = c_4 == 0;
_2 = (int) _1;
*b_6(D) = _2;
Note how we test c_4 which is the result of the shift against zero.
In contrast with the patch
c_4 = a_3(D) >> 1;
_8 = (unsigned int) a_3(D);
_1 = _8 <= 1;
_2 = (int) _1;
*b_6(D) = _2;
Note how we're testing _8 which is converted from a_3. The net is we
actually have to emit the comparison rather than using the condition
codes set by the right shift.
One "solution" here is to use :s on the ashift expression in the
match.pd pattern to apply the canonicalization only when the result is
used once. Of course it's hard to argue that we're doing
canonicalization if we're conditionally applying it.
We could perhaps adjust combine to do something more sensible with stuff
like this:
> Trying 7 -> 8:
> 7: {r29:SI=r35:SI>>0x1;clobber ccr:CC;}
> REG_DEAD r35:SI
> REG_UNUSED ccr:CC
> 8: {r33:SI=r29:SI==0;clobber ccr:CC;}
> REG_UNUSED ccr:CC
> [ ... ]
> Successfully matched this instruction:
> (set (reg/v:SI 29 [ <retval> ])
> (ashiftrt:SI (reg:SI 35 [ a ])
> (const_int 1 [0x1])))
> Failed to match this instruction:
> (set (reg:SI 33 [ _1 ])
> (eq:SI (lshiftrt:SI (reg:SI 35 [ a ])
> (const_int 1 [0x1]))
> (const_int 0 [0])))
So that's combine's generic splitting. Note how it takes the two SETs
from the PARALLEL and emits them separately. That's fine and good. But
also note that when emitted in that manner the output from the first
insn can reasonably used by the second insn. I've seen this pop up in
other contexts as well. That would probably be sufficient to fix this
problem and may help in other scenarios.
Another approach would be a target specific splitter for cris.md. But
I'd consider clean changes to combine to handle this automatically a
better solution.
Anyway, need to turn the ACK into a NAK given the cris regression. We
need to figure that out to turn it back into an ACK.
jeff
