On Mon, Sep 28, 2026 at 12:28 PM Naveen <[email protected]> wrote: > > The generic conditional-move expansion currently expands both operands > before looking for a target-specific conditional negate or complement > operation. This misses expressions such as: > > cond ? -value : value > cond ? ~value : value > > Recognize these forms including reversed operands and SSA expression > definitions before expanding the conditional-move operands. Try the > target-specific operation while keeping the conditional-move recursion > guard active. If both expansions succeed, select the cheaper sequence.
So there's no optab for this? I couldn't quickly decipher what two expansions you try, can you explain? In my view these RTL expansion time tricks, dependent on TER(!), should become less, not more. Instead we should perform this more advanced "instruction selection" beforehand, in the ISEL pass. But that requires optabs (and we'd match to direct internal function calls). Richard. > gcc/ChangeLog: > PR target/126245 > * expr.cc (get_unary_op_operand): New. > (get_conditional_neg_operand): New. > (expand_conditional_comparison): New. > (expand_cond_expr_using_cmove): Recognize conditional negate and > complement expressions and try target-specific expansions. > > gcc/testsuite/ChangeLog: > PR target/126245 > * gcc.target/riscv/pr126245.c: New test. > > Signed-off-by: Naveen <[email protected]> > --- > gcc/expr.cc | 231 +++++++++++++++++----- > gcc/testsuite/gcc.target/riscv/pr126245.c | 24 +++ > 2 files changed, 201 insertions(+), 54 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/pr126245.c > > diff --git a/gcc/expr.cc b/gcc/expr.cc > index a9157dc5d41..b1b342b4106 100644 > --- a/gcc/expr.cc > +++ b/gcc/expr.cc > @@ -9657,6 +9657,82 @@ expand_expr_real (tree exp, rtx target, machine_mode > tmode, > return ret; > } > > +/* Return the operand of a unary operation CODE in EXP. */ > + > +static tree > +get_unary_op_operand (tree exp, enum tree_code code) > +{ > + STRIP_NOPS (exp); > + > + if (TREE_CODE (exp) == code) > + return TREE_OPERAND (exp, 0); > + > + gimple *stmt = get_def_for_expr (exp, code); > + return stmt ? gimple_assign_rhs1 (stmt) : NULL_TREE; > +} > + > +/* Return the operand of a conditional negate or complement in EXP, and set > + CODE to the corresponding RTL operation. */ > + > +static tree > +get_conditional_neg_operand (tree exp, rtx_code *code) > +{ > + tree operand = get_unary_op_operand (exp, NEGATE_EXPR); > + if (operand) > + { > + *code = NEG; > + return operand; > + } > + > + operand = get_unary_op_operand (exp, BIT_NOT_EXPR); > + if (operand) > + *code = NOT; > + return operand; > +} > + > +/* Expand EXP into a comparison suitable for the conditional move expanders. > + This is kept in one place since both candidate sequences need the same > + condition. */ > + > +static void > +expand_conditional_comparison (tree exp, rtx_comparison *comparison, > + int *unsignedp) > +{ > + gimple *srcstmt = NULL; > + if (TREE_CODE (exp) == SSA_NAME > + && (srcstmt = get_def_for_expr_class (exp, tcc_comparison)) > + && !VECTOR_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (srcstmt)))) > + { > + tree type = TREE_TYPE (gimple_assign_rhs1 (srcstmt)); > + enum tree_code cmpcode = gimple_assign_rhs_code (srcstmt); > + comparison->op0 = expand_normal (gimple_assign_rhs1 (srcstmt)); > + comparison->op1 = expand_normal (gimple_assign_rhs2 (srcstmt)); > + comparison->mode = TYPE_MODE (type); > + *unsignedp = TYPE_UNSIGNED (type); > + comparison->code = convert_tree_comp_to_rtx (cmpcode, *unsignedp); > + } > + else if (COMPARISON_CLASS_P (exp) > + && !VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0)))) > + { > + tree type = TREE_TYPE (TREE_OPERAND (exp, 0)); > + enum tree_code cmpcode = TREE_CODE (exp); > + comparison->op0 = expand_normal (TREE_OPERAND (exp, 0)); > + comparison->op1 = expand_normal (TREE_OPERAND (exp, 1)); > + comparison->mode = TYPE_MODE (type); > + *unsignedp = TYPE_UNSIGNED (type); > + comparison->code = convert_tree_comp_to_rtx (cmpcode, *unsignedp); > + } > + else > + { > + comparison->op0 = expand_normal (exp); > + comparison->op1 = const0_rtx; > + comparison->code = NE; > + comparison->mode = GET_MODE (comparison->op0); > + if (comparison->mode == VOIDmode) > + comparison->mode = TYPE_MODE (TREE_TYPE (exp)); > + } > +} > + > /* Try to expand the conditional expression which is represented by > TREEOP0 ? TREEOP1 : TREEOP2 using conditional moves. If it succeeds > return the rtl reg which represents the result. Otherwise return > @@ -9667,11 +9743,7 @@ expand_cond_expr_using_cmove (tree treeop0 > ATTRIBUTE_UNUSED, > tree treeop1 ATTRIBUTE_UNUSED, > tree treeop2 ATTRIBUTE_UNUSED) > { > - rtx insn; > - rtx op00, op01, op1, op2; > - enum rtx_code comparison_code; > - machine_mode comparison_mode; > - gimple *srcstmt; > + rtx op1, op2; > rtx temp; > tree type = TREE_TYPE (treeop1); > int unsignedp = TYPE_UNSIGNED (type); > @@ -9699,45 +9771,84 @@ expand_cond_expr_using_cmove (tree treeop0 > ATTRIBUTE_UNUSED, > else > temp = assign_temp (type, 0, 1); > > - expanding_cond_expr_using_cmove = true; > - start_sequence (); > - expand_operands (treeop1, treeop2, > - mode == orig_mode ? temp : NULL_RTX, &op1, &op2, > - EXPAND_NORMAL); > + /* Find a conditional negation or complement before expanding either arm. > + Expanding the negation first would lose this opportunity and leave the > + target with a conditional move between X and -X. */ > + tree conditional_neg_operand = NULL_TREE; > + rtx_code conditional_neg_code = UNKNOWN; > + bool reverse_conditional_neg = false; > > - if (TREE_CODE (treeop0) == SSA_NAME > - && (srcstmt = get_def_for_expr_class (treeop0, tcc_comparison)) > - && !VECTOR_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (srcstmt)))) > + tree treeop1_nops = treeop1; > + tree treeop2_nops = treeop2; > + STRIP_NOPS (treeop1_nops); > + STRIP_NOPS (treeop2_nops); > + > + tree neg_operand = get_conditional_neg_operand (treeop1_nops, > + &conditional_neg_code); > + if (neg_operand) > { > - type = TREE_TYPE (gimple_assign_rhs1 (srcstmt)); > - enum tree_code cmpcode = gimple_assign_rhs_code (srcstmt); > - op00 = expand_normal (gimple_assign_rhs1 (srcstmt)); > - op01 = expand_normal (gimple_assign_rhs2 (srcstmt)); > - comparison_mode = TYPE_MODE (type); > - unsignedp = TYPE_UNSIGNED (type); > - comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp); > - } > - else if (COMPARISON_CLASS_P (treeop0) > - && !VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (treeop0, 0)))) > - { > - type = TREE_TYPE (TREE_OPERAND (treeop0, 0)); > - enum tree_code cmpcode = TREE_CODE (treeop0); > - op00 = expand_normal (TREE_OPERAND (treeop0, 0)); > - op01 = expand_normal (TREE_OPERAND (treeop0, 1)); > - unsignedp = TYPE_UNSIGNED (type); > - comparison_mode = TYPE_MODE (type); > - comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp); > + STRIP_NOPS (neg_operand); > + if (operand_equal_p (neg_operand, treeop2_nops, 0)) > + conditional_neg_operand = treeop2; > } > - else > + > + if (!conditional_neg_operand) > { > - op00 = expand_normal (treeop0); > - op01 = const0_rtx; > - comparison_code = NE; > - comparison_mode = GET_MODE (op00); > - if (comparison_mode == VOIDmode) > - comparison_mode = TYPE_MODE (TREE_TYPE (treeop0)); > + rtx_code other_neg_code = UNKNOWN; > + tree other_neg_operand > + = get_conditional_neg_operand (treeop2_nops, &other_neg_code); > + if (other_neg_operand) > + { > + STRIP_NOPS (other_neg_operand); > + if (operand_equal_p (other_neg_operand, treeop1_nops, 0)) > + { > + conditional_neg_code = other_neg_code; > + conditional_neg_operand = treeop1; > + reverse_conditional_neg = true; > + } > + } > } > - expanding_cond_expr_using_cmove = false; > + > + expanding_cond_expr_using_cmove = true; > + > + rtx_insn *neg_seq = NULL; > + if (conditional_neg_operand > + && !TREE_SIDE_EFFECTS (conditional_neg_operand)) > + { > + start_sequence (); > + rtx value = expand_normal (conditional_neg_operand); > + rtx_comparison comparison; > + int comparison_unsignedp = unsignedp; > + expand_conditional_comparison (treeop0, &comparison, > + &comparison_unsignedp); > + > + if (reverse_conditional_neg) > + comparison.code = reverse_condition (comparison.code); > + > + rtx neg_insn = NULL_RTX; > + if (comparison.code != UNKNOWN) > + { > + rtx cond = gen_rtx_fmt_ee (comparison.code, comparison.mode, > + comparison.op0, comparison.op1); > + if (GET_MODE (value) != mode) > + value = gen_lowpart (mode, value); > + neg_insn = emit_conditional_neg_or_complement > + (temp, conditional_neg_code, mode, cond, value, value); > + } > + > + rtx_insn *seq = end_sequence (); > + if (neg_insn) > + neg_seq = seq; > + } > + > + start_sequence (); > + expand_operands (treeop1, treeop2, > + mode == orig_mode ? temp : NULL_RTX, &op1, &op2, > + EXPAND_NORMAL); > + rtx_comparison comparison; > + int comparison_unsignedp = unsignedp; > + expand_conditional_comparison (treeop0, &comparison, > + &comparison_unsignedp); > > if (GET_MODE (op1) != mode) > op1 = gen_lowpart (mode, op1); > @@ -9746,25 +9857,37 @@ expand_cond_expr_using_cmove (tree treeop0 > ATTRIBUTE_UNUSED, > op2 = gen_lowpart (mode, op2); > > /* Try to emit the conditional move. */ > - insn = emit_conditional_move (temp, > - { comparison_code, op00, op01, > - comparison_mode }, > - op1, op2, mode, > - unsignedp); > + rtx insn = emit_conditional_move (temp, > + comparison, op1, op2, mode, > + comparison_unsignedp); > + rtx_insn *cmove_seq = end_sequence (); > + if (!insn) > + cmove_seq = NULL; > > - /* If we could do the conditional move, emit the sequence, > - and return. */ > - if (insn) > + rtx_insn *seq = NULL; > + if (neg_seq && cmove_seq) > { > - rtx_insn *seq = end_sequence (); > - emit_insn (seq); > - return convert_modes (orig_mode, mode, temp, 0); > + bool speed_p = optimize_insn_for_speed_p (); > + unsigned neg_cost = seq_cost (neg_seq, speed_p); > + unsigned cmove_cost = seq_cost (cmove_seq, speed_p); > + if (neg_cost == cmove_cost) > + { > + neg_cost = seq_cost (neg_seq, !speed_p); > + cmove_cost = seq_cost (cmove_seq, !speed_p); > + } > + seq = neg_cost < cmove_cost ? neg_seq : cmove_seq; > } > + else if (neg_seq) > + seq = neg_seq; > + else if (cmove_seq) > + seq = cmove_seq; > > - /* Otherwise discard the sequence and fall back to code with > - branches. */ > - end_sequence (); > - return NULL_RTX; > + expanding_cond_expr_using_cmove = false; > + if (!seq) > + return NULL_RTX; > + > + emit_insn (seq); > + return convert_modes (orig_mode, mode, temp, 0); > } > > /* A helper function for expand_expr_real_2 to be used with a > diff --git a/gcc/testsuite/gcc.target/riscv/pr126245.c > b/gcc/testsuite/gcc.target/riscv/pr126245.c > new file mode 100644 > index 00000000000..d3ae337be58 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/pr126245.c > @@ -0,0 +1,24 @@ > +/* PR target/126245 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -march=rv64g_zicond -mabi=lp64d" { target { rv64 } } } > */ > +/* { dg-options "-O2 -march=rv32g_zicond -mabi=ilp32d" { target { rv32 } } } > */ > + > +unsigned long > +f_cmp_lt (unsigned long x, unsigned long y) > +{ > + unsigned long cmp = x < y; > + return (x ^ -cmp) + cmp; > +} > + > +/* { dg-final { scan-assembler-times {\msltu\M} 1 } } */ > +/* { dg-final { scan-assembler-times {\mneg\M} 1 } } */ > +/* { dg-final { scan-assembler-times {\mxor\M} 1 } } */ > +/* { dg-final { scan-assembler {f_cmp_lt:(?:[^\n]*\n)*?\tadd\t} } } */ > + > +long > +f_cmove_guard_reset (long cond, long a, long b) > +{ > + return cond ? a : b; > +} > + > +/* { dg-final { scan-assembler {\mczero\.(eqz|nez)\M} } } */ > -- > 2.34.1 >
