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.
CONFIDENTIALITY: The contents of this e-mail are confidential and intended only
for the above addressee(s). If you are not the intended recipient, or the
person responsible for delivering it to the intended recipient, copying or
delivering it to anyone else or using it in any unauthorized manner is
prohibited and may be unlawful. If you receive this e-mail by mistake, please
notify the sender and the systems administrator at [email protected]
immediately.
---
gcc/ifcvt.cc | 109 +++++++++++++++-----
gcc/testsuite/gcc.target/riscv/pr120553-5.c | 9 +-
gcc/testsuite/gcc.target/riscv/pr120553-6.c | 9 +-
gcc/testsuite/gcc.target/riscv/pr125167-1.c | 15 +++
gcc/testsuite/gcc.target/riscv/pr125167-2.c | 14 +++
5 files changed, 121 insertions(+), 35 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/pr125167-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/pr125167-2.c
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index ab9d35699cb..7b848f11c3c 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -1450,17 +1450,41 @@ noce_try_sign_bit_splat (noce_if_info *if_info)
/* It would be good if this could be extended since constant synthesis
on some platforms will result in blocks which fail this test. */
- if (!noce_simple_bbs (if_info))
- return false;
+ bool simple_bbs = noce_simple_bbs (if_info);
+
+ rtx a = if_info->a;
+ rtx b = if_info->b;
+
+ /* Non-simple blocks are only allowed when a synthesized constant
+ was recovered from a REG_EQUAL or REG_EQUIV note.
+ bb_valid_for_noce_process_p has already checked that such blocks
+ have no side effects and that their temporaries are dead on exit,
+ so their insns can be dropped. */
+ if (!CONST_INT_P (a) && if_info->insn_a)
+ {
+ rtx note = find_reg_equal_equiv_note (if_info->insn_a);
+ if (note && CONST_INT_P (XEXP (note, 0)))
+ a = XEXP (note, 0);
+ }
+
+ if (!CONST_INT_P (b) && if_info->insn_b)
+ {
+ rtx note = find_reg_equal_equiv_note (if_info->insn_b);
+ if (note && CONST_INT_P (XEXP (note, 0)))
+ b = XEXP (note, 0);
+ }
/* Only try this for constants in the true/false arms and a REG
destination. We could select between 0 and a REG pretty
easily with a logical AND. */
- if (!CONST_INT_P (if_info->a)
- || !CONST_INT_P (if_info->b)
+ if (!CONST_INT_P (a)
+ || !CONST_INT_P (b)
|| !REG_P (if_info->x))
return false;
+ if (!simple_bbs && a == if_info->a && b == if_info->b)
+ return false;
+
machine_mode mode = GET_MODE (if_info->x);
/* If the mode of the destination does not match the mode of
@@ -1468,8 +1492,8 @@ noce_try_sign_bit_splat (noce_if_info *if_info)
if (mode != GET_MODE (XEXP (cond, 0)))
return false;
- HOST_WIDE_INT val_a = INTVAL (if_info->a);
- HOST_WIDE_INT val_b = INTVAL (if_info->b);
+ HOST_WIDE_INT val_a = INTVAL (a);
+ HOST_WIDE_INT val_b = INTVAL (b);
rtx_insn *seq;
start_sequence ();
@@ -3324,7 +3348,7 @@ noce_try_abs (noce_if_info *if_info)
static bool
noce_try_sign_mask (noce_if_info *if_info)
{
- rtx cond, t, m, c;
+ rtx cond, t, m, c, f;
rtx_insn *seq;
machine_mode mode;
enum rtx_code code;
@@ -3339,6 +3363,7 @@ noce_try_sign_mask (noce_if_info *if_info)
c = XEXP (cond, 1);
t = NULL_RTX;
+ f = NULL_RTX;
if (if_info->a == const0_rtx)
{
if ((code == LT && c == const0_rtx)
@@ -3351,6 +3376,21 @@ noce_try_sign_mask (noce_if_info *if_info)
|| (code == GT && c == constm1_rtx))
t = if_info->a;
}
+ /* Neither arm is zero. We can still select between the two arms with
+ X = ((T ^ F) & M) ^ F, where M is the sign mask. This evaluates both
+ arms unconditionally, so it is restricted to registers below. */
+ else if ((code == LT && c == const0_rtx)
+ || (code == LE && c == constm1_rtx))
+ {
+ t = if_info->b;
+ f = if_info->a;
+ }
+ else if ((code == GE && c == const0_rtx)
+ || (code == GT && c == constm1_rtx))
+ {
+ t = if_info->a;
+ f = if_info->b;
+ }
if (! t || side_effects_p (t))
return false;
@@ -3360,23 +3400,31 @@ noce_try_sign_mask (noce_if_info *if_info)
if (GET_MODE (m) != mode)
return false;
- /* This is only profitable if T is unconditionally executed/evaluated in the
- original insn sequence or T is cheap and can't trap or fault. The former
- happens if B is the non-zero (T) value and if INSN_B was taken from
- TEST_BB, or there was no INSN_B which can happen for e.g. conditional
- stores to memory. For the cost computation use the block TEST_BB where
- the evaluation will end up after the transformation. */
- t_unconditional
- = (t == if_info->b
- && (if_info->insn_b == NULL_RTX
- || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb));
- if (!(t_unconditional
- || ((set_src_cost (t, mode, if_info->speed_p)
- < COSTS_N_INSNS (2))
- && !may_trap_or_fault_p (t))))
- return false;
+ if (f)
+ {
+ if (!REG_P (t) || !REG_P (f) || GET_MODE (f) != mode)
+ return false;
+ }
+ else
+ {
+ /* This is only profitable if T is unconditionally executed/evaluated in
+ the original insn sequence or T is cheap and can't trap or fault. The
+ former happens if B is the non-zero (T) value and if INSN_B was taken
+ from TEST_BB, or there was no INSN_B which can happen for e.g.
+ conditional stores to memory. For the cost computation use the block
+ TEST_BB where the evaluation will end up after the transformation. */
+ t_unconditional
+ = (t == if_info->b
+ && (if_info->insn_b == NULL_RTX
+ || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb));
+ if (!(t_unconditional
+ || ((set_src_cost (t, mode, if_info->speed_p)
+ < COSTS_N_INSNS (2))
+ && !may_trap_or_fault_p (t))))
+ return false;
+ }
- if (!noce_can_force_operand (t))
+ if (!noce_can_force_operand (t) || (f && !noce_can_force_operand (f)))
return false;
start_sequence ();
@@ -3384,8 +3432,19 @@ noce_try_sign_mask (noce_if_info *if_info)
"(signed) m >> 31" directly. This benefits targets with specialized
insns to obtain the signmask, but still uses ashr_optab otherwise. */
m = emit_store_flag (gen_reg_rtx (mode), LT, m, const0_rtx, mode, 0, -1);
- t = m ? expand_binop (mode, and_optab, m, t, NULL_RTX, 0, OPTAB_DIRECT)
- : NULL_RTX;
+
+ /* Neither arm is zero: X = ((T ^ F) & M) ^ F. */
+ if (m && f)
+ {
+ rtx x1 = expand_binop (mode, xor_optab, t, f, NULL_RTX, 0, OPTAB_DIRECT);
+ rtx x2 = x1 ? expand_binop (mode, and_optab, m, x1, NULL_RTX, 0,
+ OPTAB_DIRECT) : NULL_RTX;
+ t = x2 ? expand_binop (mode, xor_optab, x2, f, NULL_RTX, 0,
+ OPTAB_DIRECT) : NULL_RTX;
+ }
+ else
+ t = m ? expand_binop (mode, and_optab, m, t, NULL_RTX, 0, OPTAB_DIRECT)
+ : NULL_RTX;
if (!t)
{
diff --git a/gcc/testsuite/gcc.target/riscv/pr120553-5.c
b/gcc/testsuite/gcc.target/riscv/pr120553-5.c
index 1e4833091cc..73ba7c1a9bd 100644
--- a/gcc/testsuite/gcc.target/riscv/pr120553-5.c
+++ b/gcc/testsuite/gcc.target/riscv/pr120553-5.c
@@ -82,10 +82,9 @@ T1(62)
T1(63)
#endif
-/* The MSB case isn't handled the way we want. */
-/* { dg-final { scan-assembler-times "\\t(srai)" 126 { target rv64 } } } */
-/* { dg-final { scan-assembler-times "\\t(slli|andi|bclr)" 126 { target rv64 }
} } */
+/* { dg-final { scan-assembler-times "\\t(srai)" 128 { target rv64 } } } */
+/* { dg-final { scan-assembler-times "\\t(slli|andi|bclr)" 128 { target rv64 }
} } */
-/* { dg-final { scan-assembler-times "\\t(srai)" 62 { target rv32 } } } */
-/* { dg-final { scan-assembler-times "\\t(slli|andi|bclr)" 62 { target rv32 }
} } */
+/* { dg-final { scan-assembler-times "\\t(srai)" 64 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\t(slli|andi|bclr)" 64 { target rv32 }
} } */
diff --git a/gcc/testsuite/gcc.target/riscv/pr120553-6.c
b/gcc/testsuite/gcc.target/riscv/pr120553-6.c
index 6c409afb3c8..bb04f17b2f2 100644
--- a/gcc/testsuite/gcc.target/riscv/pr120553-6.c
+++ b/gcc/testsuite/gcc.target/riscv/pr120553-6.c
@@ -82,10 +82,9 @@ T1(62)
T1(63)
#endif
-/* Not working for the high bit case yet. */
-/* { dg-final { scan-assembler-times "\\t(srai)" 126 { target rv64 } } } */
-/* { dg-final { scan-assembler-times "\\t(andn|andi|bclr)" 126 { target rv64 }
} } */
+/* { dg-final { scan-assembler-times "\\t(srai)" 128 { target rv64 } } } */
+/* { dg-final { scan-assembler-times "\\t(andn|andi|bclr)" 128 { target rv64 }
} } */
-/* { dg-final { scan-assembler-times "\\t(srai)" 62 { target rv32 } } } */
-/* { dg-final { scan-assembler-times "\\t(andn|andi|bclr)" 62 { target rv32 }
} } */
+/* { dg-final { scan-assembler-times "\\t(srai)" 64 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\t(andn|andi|bclr)" 64 { target rv32 }
} } */
diff --git a/gcc/testsuite/gcc.target/riscv/pr125167-1.c
b/gcc/testsuite/gcc.target/riscv/pr125167-1.c
new file mode 100644
index 00000000000..ee3037caca9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr125167-1.c
@@ -0,0 +1,15 @@
+/* PR target/125167 */
+/* { dg-do compile { target { rv64 } } } */
+/* { dg-options "-march=rv64gc -mabi=lp64d" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Os" "-Oz" "-Og" } } */
+
+long
+foo2 (long c, long a, long b)
+{
+ return c < 0 ? a : b;
+}
+
+/* { dg-final { scan-assembler-times {\msrai\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mxor\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mand\M} 1 } } */
+/* { dg-final { scan-assembler-not {\m(?:beq|bne|blt|bge|ble|bgt)(?:u|z)?\M} }
} */
diff --git a/gcc/testsuite/gcc.target/riscv/pr125167-2.c
b/gcc/testsuite/gcc.target/riscv/pr125167-2.c
new file mode 100644
index 00000000000..698f932e750
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr125167-2.c
@@ -0,0 +1,14 @@
+/* PR target/125167 */
+/* { dg-do compile { target { rv64 } } } */
+/* { dg-options "-march=rv64gc -mabi=lp64d" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Os" "-Oz" "-Og" } } */
+
+long
+foo3 (long c)
+{
+ return c < 0 ? 16383 : 0;
+}
+
+/* { dg-final { scan-assembler-times {\msrai\M} 1 } } */
+/* { dg-final { scan-assembler-times {\msrli\M} 1 } } */
+/* { dg-final { scan-assembler-not {\m(?:beq|bne|blt|bge|ble|bgt)(?:u|z)?\M} }
} */
--
2.34.1
