On 10/8/26 8:33 AM, Milan Tripkovic wrote:
This patch fixes PR125167, where two select cases generate
conditional branches on RISC-V even though better sequences exist:
long foo2 (long c, long a, long b) { return c < 0 ? a : b; }
long foo3 (long c) { return c < 0 ? 16383 : 0; }
Patch contains 2 changes: noce_try_sign_mask and noce_try_sign_bit_splat.
noce_try_sign_mask only handled selects where one arm is zero. Extend
it to select between two registers as X = ((T ^ F) & M) ^ F, where M is
the sign mask. When M is all ones the expression evaluates to T, and when
M is zero it evaluates to F. Both arms are evaluated unconditionally,
so this is limited to registers. As in the existing zero-arm case,
the result
is committed without calling noce_conversion_profitable_p.
Foo2 before:
bge a0,zero,.L7
mv a2,a1
.L7:
mv a0,a2
ret
Foo2 after:
srai a0,a0,63
xor a1,a1,a2
and t0,a0,a1
xor a0,t0,a2
ret
noce_try_sign_bit_splat already knows how to build 2^n-1 from the sign
mask, but 16383 takes two insns to synthesize, so the THEN block is not
simple and transformation was rejected early. The value is still
recorded in a
REG_EQUAL note on the last insn of the block, and
bb_valid_for_noce_process_p has already checked that such a block has
no side effects and that its temporaries are dead on exit. So recover
constant arms from REG_EQUAL or REG_EQUIV notes and accept non-simple
blocks whose value was recovered that way.
Foo3 before:
blt a0,zero,.L12
li a0,0
ret
.L12:
li a0,16384
addi a0,a0,-1
ret
Foo3 after:
srai a0,a0,63
srli a0,a0,50
ret
This patch also handles the MSB case in pr120553-5.c and pr120553-6.c
on both rv32 and rv64, which those tests noted as not yet handled, so
adjust the expected instruction counts accordingly.
Regression tested on riscv64 and x86_64.
2026-10-08 Milan Tripkovic <[email protected]>
PR target/125167
gcc/ChangeLog:
* ifcvt.cc (noce_try_sign_bit_splat): Recover synthesized constants
(noce_try_sign_mask): Handle selects between two nonzero registers.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr120553-5.c: Adjust expected counts.
* gcc.target/riscv/pr120553-6.c: Adjust expected counts.
* gcc.target/riscv/pr125167-1.c: New test.
* gcc.target/riscv/pr125167-2.c: New test.
Interesting, I never even thought of trying to use a REG_EQUAL note to
recover something interesting in this code.
So with your change I think noce_try_sign_mask can generate more
instructions whereas it was previously generating just two. So we
probably need to be more careful about costing. I think all you need to
do is adjust the call to noce_commit_sequence to pass true rather than
false for the check_profitable parameter.
The comment before noce_try_sign_bit_split probably needs a minor update
since it can now recover a constant from a REG_EQUAL note and process a
limited set of non-simple blocks.
On testing, LLM review suggests adding coverage for more types. So
maybe generalize the two new tests to work on int and long. We may find
missing cases if you do that. If so, we'll have to evaluate if it's
best to fix them now or just note them as expected failures. LLM review
also testing some other conditions like >= 0, <= -1 and > 1. Finally
it suggests run tests for extra coverage. I'm not sure all this is
really important, so your call whether or not to expanding the testing.
Overall I think this is basically ready to go with the minor adjustments
noted above.
For reasons unknown your patch didn't get reviewed by Sashiko. It looks
like your attachments are coming encoded as which may be the issue.
--_000_AM8PR08MB5762062FA6C69C0EBAC46FA8A9662AM8PR08MB5762eurp_
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Any chance you could avoid having them encoded so that Sashiko has a
better chance of picking them up?
Thanks,
Jeff
