On 7/29/23 03:13, Xiao Zeng wrote:
This patch recognizes Zicond patterns when the select pattern
with condition eq or neq to 0 (using eq as an example), namely:
1 rd = (rs2 == 0) ? non-imm : 0
2 rd = (rs2 == 0) ? non-imm : non-imm
3 rd = (rs2 == 0) ? reg : non-imm
4 rd = (rs2 == 0) ? reg : reg
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_expand_conditional_move): Recognize
Zicond patterns
* config/riscv/riscv.md: Recognize Zicond patterns through mov<mode>cc
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zicond-primitiveSemantics_return_0_imm.c: New test.
* gcc.target/riscv/zicond-primitiveSemantics_return_imm_imm.c: New
test.
* gcc.target/riscv/zicond-primitiveSemantics_return_imm_reg.c: New
test.
* gcc.target/riscv/zicond-primitiveSemantics_return_reg_reg.c: New
test.
---
gcc/config/riscv/riscv.cc | 144 ++++++++++++++++++
gcc/config/riscv/riscv.md | 4 +-
.../zicond-primitiveSemantics_return_0_imm.c | 65 ++++++++
...zicond-primitiveSemantics_return_imm_imm.c | 73 +++++++++
...zicond-primitiveSemantics_return_imm_reg.c | 65 ++++++++
...zicond-primitiveSemantics_return_reg_reg.c | 65 ++++++++
6 files changed, 414 insertions(+), 2 deletions(-)
create mode 100644
gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_0_imm.c
create mode 100644
gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_imm_imm.c
create mode 100644
gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_imm_reg.c
create mode 100644
gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_return_reg_reg.c
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 941ea25e1f2..6ac39f63dd7 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3516,6 +3516,150 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx
cons, rtx alt)
cond, cons, alt)));
return true;
}
+ 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)
A couple nits. Rather than test the GET_CODE (object) == CONST_INT,
instead use CONST_INT_P (object).
Rather than using const0_rtx, use CONST0_RTX (mode). That makes it more
general.
+ {
+ riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
Might as well use "true" rather than "need_eq_ne_p" here and for the
other calls in your new code.
+ /* imm + imm */
+ else if (GET_CODE (cons) == CONST_INT && cons != const0_rtx
+ && GET_CODE (alt) == CONST_INT && alt != const0_rtx)
So same comments on using CONST_INT_P and 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);
+ rtx reg = gen_reg_rtx (mode);
+ rtx temp = GEN_INT (INTVAL (alt) - INTVAL (cons));
+ emit_insn (gen_rtx_SET (reg, temp));
Use force_reg here rather than directly emitting the insn to initialize
"reg". What you're doing works when the difference is small but will
not work when the difference does not fit into a signed 12bit value.
+ /* imm + reg */
+ else if (GET_CODE (cons) == CONST_INT && cons != const0_rtx
+ && GET_CODE (alt) == REG)
Same comments about CONST_INT_P and CONST0_RTX. And instead of using
GET_CODE (object) == REG, use REG_P (object).
+ {
+ /* 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;
+ }
Isn't this only valid for NE? Also use CONST0_RTX in that test.
+ /* Handle the special situation of: -2048 == INTVAL (alt)
+ to avoid failure due to an unrecognized insn. Let the costing
+ model determine if the conditional move sequence is better
+ than the branching sequence. */
+ if (-2048 == INTVAL (cons))
So instead of checking for explicit values, we have SMALL_OPERAND to do
it for us. Also note that for !SMALL_OPERAND we can just force the
value into a register using "force_reg" and all the right things will
happen.
So just add something like this to your original code:
if (!SMALL_OPERAND (INTVAL (cons))
cons = force_reg (mode, cons);
That will result in CONS being either a simple integer constant (when it
is suitable for addi) or a register (all other cases). At that point
you can use it in an arithmetic instruction.
+ /* imm + 0 */
+ 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);
+ cons = force_reg (mode, cons);
+ emit_insn (gen_rtx_SET (dest, gen_rtx_IF_THEN_ELSE (mode, cond,
+ cons, alt)));
+ return true;
+ }
+ /* 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;
+ }
So in this case isn't it only valid for EQ?
+ if (-2048 == INTVAL (alt))
Similar comment as above. When ALT doesn't fit into a SMALL_OPERAND_P,
just force it into a register.
+ /* reg + reg */
+ else if (GET_CODE (cons) == REG && GET_CODE (alt) == REG)
+ {
+ /* Optimize for op0 == cons && op1 == const0_rtx. */
+ if (op0 == cons && op1 == const0_rtx)
+ {
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond,
+ CONST0_RTX (mode),
+ alt)));
+ return true;
+ }
+ /* Optimize for op0 == alt && op1 == const0_rtx. */
+ if (op0 == alt && op1 == const0_rtx)
+ {
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (mode, cond, cons,
+ CONST0_RTX
(mode))));
+ return true;
+ }
I suspect we need some testing of the code in the two sequences above.
I'd bet one is only valid for EQ and the other for NE.
It's improving considerably.
WRT costing. I'm still seeing some weirdness in how conditional moves
are being costed from the if-converter. I think your patch that set the
cost to COSTS_N_INSNS (0) was just working around a deeper bug in the
costing code. I'm still trying to figure out the best way to clean that up.
jeff
