On 5/27/2026 3:57 AM, Dusan Stojkovic wrote:
Handle more shapes in ifcvt that occur in code on rv64 + zicond:
For example:
int foo (int cond, int val) {
if (cond) val *= 64;
return val;
}
the desired 3-insn output:
li a5,6
czero.eqz a0,a5,a0
sllw a0,a1,a0
in place of the previous 4-insn slli/czero.eqz/czero.nez/add sequence
that noce_try_cmove_arith was producing.
This patch improves cases for:
* int_ShiftLeft_const
* int_Mul_pow2_const
* int_Add\Sub_const
As well as improving tests from PR123322 with
(sign_extend:DI (subreg:SI (reg:DI 138) 0)))
cases.
The peel of the outer SIGN_EXTEND/ZERO_EXTEND is gated on the SUBREG
inside the binop being SUBREG_PROMOTED_VAR_P with a promotion sign
matching the outer extension code. This is the property that
makes (EXT (SUBREG b)) bit-equal to b.
W
The existing profitability gate (targetm.noce_conversion_profitable_p
at end_ifcvt_sequence) already covers this transform, so no separate
cost check is added.
AND is excluded from the peel path because its identity op is -1,
not 0, and noce_try_cond_arith already handles it through a separate
code path.
XOR is a different case which succeeds through noce_convert_multiple_sets
And thus still produces suboptimal code.
2026-05-27 Dusan Stojkovic <[email protected]>
PR target/124956
gcc/ChangeLog:
* ifcvt.cc (noce_peel_extend_around_binop): New helper.
(noce_try_cond_arith): Peel an optional SIGN/ZERO_EXTEND
wrapping of a supported binary op re-apply at success
label.
Accept CONST_INT as the second operand of the binary op.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr123322.c: Reduce the number of czeros expected.
* gcc.target/riscv/pr124956.c: New test.
So at the heart this seems to rely on having the zero/sign extend
wrapping a subreg with the appropriate promoted mode flags. I would
generally expect that such cases would be limited the zero/sign
extension node could be eliminated in the IL with no change semantics.
So if this is working consistently, it likely points to a missed
optimization elsewhere in the compiler.
What we did internally was recognize any wrapping sign/zero extend
early, noting that we had done so and what kind of extension we saw.
We'd strip the extension, let the code run normally, then re-apply the
extension at the end.
That in and of itself wasn't terrible and probably gets the vast
majority of the benefit. There's a bit of code that I noted as "odd"
after the fact, but never chased down why it was added. It's not
obvious if it would still be needed or not.
Just playing around a bit I ended up with the attached patch. I haven't
tested it beyond verifying the testcase gives the desired code.
I wouldn't be surprised if handling constants in get_base is in and of
itself a good thing. So extracting those bits independently might be a
good first step.
I haven't really thought about the implications of SUBREGs in the code,
either narrowing or paradoxical. It may be OK, it may not.
I really didn't like the extra call to get_base_reg_or_constant. But I
didn't see a good solution. In the immediately preceeding fragment I
think we want the subreg stripped, but we need the subreg expression for
all the subsequent code. There may not be a better solution there.
The sequence we generate for the testcase looks like:
(insn 28 0 29 (set (reg:SI 142)
(const_int 6 [0x6])) -1
(nil))
(insn 29 28 30 (set (reg:SI 140)
(if_then_else:SI (eq:DI (reg/v:DI 135 [ cond ])
(const_int 0 [0]))
(const_int 0 [0])
(reg:SI 142))) -1
(nil))
(insn 30 29 31 (set (reg:DI 144)
(sign_extend:DI (ashift:SI (subreg:SI (reg/v:DI 136 [ val ]) 0)
(subreg:QI (reg:SI 140) 0)))) -1
(nil))
(insn 31 30 32 (set (reg:SI 143)
(subreg/s/u:SI (reg:DI 144) 0)) -1
(expr_list:REG_EQUAL (ashift:SI (subreg:SI (reg/v:DI 136 [ val ]) 0)
(subreg:QI (reg:SI 140) 0))
(nil)))
(insn 32 31 0 (set (reg/v:DI 136 [ val ])
(sign_extend:DI (reg:SI 143))) -1
(nil))
insns 31/32 are redundant. They will eventually get eliminated, but
they will be costed. If there's a clean way to avoid those, it would be
good. Otherwise it may be better do go ahead and do the shift in DI,
then extract the bits we want with
(sign_extend:DI (subreg:SI (shift output:DI)))
That's one less useless insn to cost. But I haven't thought at all
about the implications of generating code like that.
Anyway, it's another path to play with.
jeff
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index c151146c1b2..f4118d6cf2d 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3120,12 +3120,16 @@ noce_cond_zero_binary_op_supported (rtx op)
otherwise NULL_RTX for other RTX_CODE. */
static rtx
-get_base_reg (rtx exp)
+get_base_reg_or_constant (rtx exp, bool strip_subreg)
{
if (REG_P (exp))
return exp;
- else if (SUBREG_P (exp))
+ else if (SUBREG_P (exp) && strip_subreg)
return SUBREG_REG (exp);
+ else if (SUBREG_P (exp))
+ return exp;
+ else if (CONST_INT_P (exp))
+ return exp;
return NULL_RTX;
}
@@ -3163,6 +3167,8 @@ noce_try_cond_arith (struct noce_if_info *if_info)
rtx_insn *seq;
rtx_code op;
machine_mode mode = GET_MODE (if_info->x);
+ bool zero_extend = false;
+ bool extend = false;
/* Scalar integral modes are only supported here.
Could support scalar floating point but that
@@ -3177,6 +3183,15 @@ noce_try_cond_arith (struct noce_if_info *if_info)
a = copy_rtx (if_info->a);
b = copy_rtx (if_info->b);
+ /* Strip any extension of A. We will re-apply it later. */
+ if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND)
+ {
+ zero_extend = (op == ZERO_EXTEND);
+ a = XEXP (a, 0);
+ mode = GET_MODE (a);
+ extend = true;
+ }
+
/* Canonicalize x = y : (y op z) to x = (y op z) : y. */
if (REG_P (a) && noce_cond_zero_binary_op_supported (b))
{
@@ -3200,24 +3215,26 @@ noce_try_cond_arith (struct noce_if_info *if_info)
op = GET_CODE (a);
/* Canonicalize x = (z op y) : y to x = (y op z) : y */
- a_op1 = get_base_reg (XEXP (a, 1));
+ a_op1 = get_base_reg_or_constant (XEXP (a, 1), !extend);
if (a_op1 && rtx_equal_p (a_op1, b) && COMMUTATIVE_ARITH_P (a))
{
std::swap (XEXP (a, 0), XEXP (a, 1));
- a_op1 = get_base_reg (XEXP (a, 1));
+ a_op1 = get_base_reg_or_constant (XEXP (a, 1), !extend);
}
if (a_op1 == NULL_RTX)
goto fail;
/* Ensure the cond is of form: x = (y op z) : y */
- a_op0 = get_base_reg (XEXP (a, 0));
+ a_op0 = get_base_reg_or_constant (XEXP (a, 0), true);
if (!(a_op0 && rtx_equal_p (a_op0, b)))
goto fail;
+ a_op0 = get_base_reg_or_constant (XEXP (a, 0), extend);
+
start_sequence ();
- target = gen_reg_rtx (GET_MODE (XEXP (a, op != AND)));
+ target = gen_reg_rtx (GET_MODE (XEXP (a, !extend && op != AND)));
/* AND requires !cond, instead we swap ops around. */
target = noce_emit_cmove (if_info, target, code,
@@ -3313,6 +3330,14 @@ noce_try_cond_arith (struct noce_if_info *if_info)
goto end_seq_n_fail;
success:
+ if (extend)
+ {
+ target = gen_rtx_fmt_e (zero_extend ? ZERO_EXTEND : SIGN_EXTEND,
+ GET_MODE (if_info->x), target);
+ emit_move_insn (if_info->x, target);
+ target = if_info->x;
+ }
+
if (target != if_info->x)
noce_emit_move_insn (if_info->x, target);
