On 7/19/23 04:11, Xiao Zeng wrote:
+ else if (TARGET_ZICOND
+ && (code == EQ || code == NE)
+ && GET_MODE_CLASS (mode) == MODE_INT)
+ {
+ need_eq_ne_p = true;
+ /* 0 + imm */
+ if (GET_CODE (cons) == CONST_INT && cons == const0_rtx
+ && GET_CODE (alt) == CONST_INT && alt != const0_rtx)
+ {
+ riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ alt = force_reg (mode, alt);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ cons, alt)));
+ return true;
+ }
+ /* imm + imm */
+ else if (GET_CODE (cons) == CONST_INT && cons != const0_rtx
+ && GET_CODE (alt) == CONST_INT && alt != const0_rtx)
+ {
+ riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ alt = force_reg (mode, alt);
+ rtx temp1 = gen_reg_rtx (mode);
+ rtx temp2 = GEN_INT(-1 * INTVAL (cons));
+ riscv_emit_binary(PLUS, temp1, alt, temp2);
So in this sequence you're just computing a constant since both ALT and
CONS are constants. It's better to just form the constant directly,
then force that into a register because it'll make the costing more
correct, particularly if the resulting constant needs more than one
instruction to synthesize.
And a nit. There should always be a space between a function name and
its argument list.
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ const0_rtx, alt)));
+ riscv_emit_binary(PLUS, dest, dest, cons);
+ return true;
I don't see how this can be correct from a code generation standpoint.
You compute ALT-CONS into TEMP1 earlier. But you never use TEMP1 after
that. I think you meant to use TEMP1 instead of ALT as the false arm if
the IF-THEN-ELSE you constructed.
In general you should be using CONST0_RTX (mode) rather than const0_rtx.
+ }
+ /* imm + reg */
+ else if (GET_CODE (cons) == CONST_INT && cons != const0_rtx
+ && GET_CODE (alt) == REG)
+ {
+ /* Optimize for register value of 0. */
+ if (op0 == alt && op1 == const0_rtx)
+ {
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ cons = force_reg (mode, cons);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ cons, alt)));
+ return true;
+ }
+ riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ rtx temp1 = gen_reg_rtx (mode);
+ rtx temp2 = GEN_INT(-1 * INTVAL (cons));
+ riscv_emit_binary(PLUS, temp1, alt, temp2);
Here you have to be careful if CONS is -2048. You negate it resulting
in +2048 which can't be used in an addi. This will cause the entire
sequence to fail due to an unrecognized insn. It would be better to
handle that scenario directly so the generated sequence is still valid.
By generating recognizable code in that case we let the costing model
determine if the conditional move sequence is better than the branching
sequence.
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ const0_rtx, alt)));
I think we have the same problem with the use of ALT here rather than
TEMP1 that we had in the previous case.
+ /* reg + imm */
+ else if (GET_CODE (cons) == REG
+ && GET_CODE (alt) == CONST_INT && alt != const0_rtx)
+ {
+ /* Optimize for register value of 0. */
+ if (op0 == cons && op1 == const0_rtx)
+ {
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ alt = force_reg (mode, alt);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ cons, alt)));
+ return true;
+ }
+ riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ rtx temp1 = gen_reg_rtx (mode);
+ rtx temp2 = GEN_INT(-1 * INTVAL (alt));
+ riscv_emit_binary(PLUS, temp1, cons, temp2);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ temp1, const0_rtx)));
+ riscv_emit_binary(PLUS, dest, dest, alt);
+ return true;
This has basically the same issues as the imm + reg case.
+ }
+ /* reg + reg */
+ else if (GET_CODE (cons) == REG && GET_CODE (alt) == REG)
+ {
+ rtx reg1 = gen_reg_rtx (mode);
+ rtx reg2 = gen_reg_rtx (mode);
+ riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
+ rtx cond1 = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ rtx cond2 = gen_rtx_fmt_ee (code == NE ? EQ : NE,
+ GET_MODE (op0), op0, op1);
+ emit_insn (gen_rtx_SET (reg2,
+ gen_rtx_IF_THEN_ELSE (mode, cond2,
+ const0_rtx, cons)));
+ emit_insn (gen_rtx_SET (reg1,
+ gen_rtx_IF_THEN_ELSE (mode, cond1,
+ const0_rtx, alt)));
+ riscv_emit_binary(IOR, dest, reg1, reg2);
+ return true;
This probably should detect the case where alt or cons is the same as
op0. Otherwise I would expect the costing model to reject some cases
that are actually profitable.
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 6b8c2e8e268..b4147c7a79c 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2484,7 +2484,7 @@
(if_then_else:GPR (match_operand 1 "comparison_operator")
(match_operand:GPR 2 "reg_or_0_operand")
(match_operand:GPR 3 "sfb_alu_operand")))
- "TARGET_SFB_ALU || TARGET_XTHEADCONDMOV"
+ "TARGET_SFB_ALU || TARGET_XTHEADCONDMOV || TARGET_ZICOND"
Note the predicate for operand2 will reject all constants except 0.
Anyway, I'm still cleaning this up and adding the bits from Ventana
which handle the relational conditions as well as FP conditionals.
Jeff
