Re: [PATCH 3/5] [RISC-V] Generate Zicond instruction for select pattern with condition eq or neq to 0

[Jeff Law via Gcc-patches]

On 7/19/23 04:11, Xiao Zeng wrote:
This patch completes the recognition of Zicond when the select pattern
with condition eq or neq to 0 (using equality 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_rtx_costs): IF_THEN_ELSE costs in 
Zicond.
         (riscv_expand_conditional_move): Recognize Zicond.
         * config/riscv/riscv.md: Zicond patterns.

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                     | 125 ++++++++++++++++++
  gcc/config/riscv/riscv.md                     |   2 +-
  .../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, 394 insertions(+), 1 deletion(-)
  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 38d8eb2fcf5..7e6b24bd232 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2448,6 +2448,17 @@ riscv_rtx_costs (rtx x, machine_mode mode, int 
outer_code, int opno ATTRIBUTE_UN
          *total = COSTS_N_INSNS (1);
          return true;
        }
+      else if (TARGET_ZICOND && outer_code == SET &&
+               ((GET_CODE (XEXP (x, 1)) == REG && XEXP (x, 2) == const0_rtx) ||
+               (GET_CODE (XEXP (x, 2)) == REG && XEXP (x, 1) == const0_rtx) ||
+               (GET_CODE (XEXP (x, 1)) == REG && GET_CODE (XEXP (x, 2)) &&
+                XEXP (x, 1) == XEXP (XEXP (x, 0), 0)) ||
+               (GET_CODE (XEXP (x, 1)) == REG && GET_CODE (XEXP (x, 2)) &&
+                XEXP (x, 2) == XEXP (XEXP (x, 0), 0))))
+        {
+          *total = 0;
+          return true;
+        }
So why *total = 0.  I would have expected *total = COSTS_N_INSNS (1).


I'm not entirely sure the changes to riscv_expand_conditional_move are 
desirable -- these are likely better done in the generic if-conversion 
pass.


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"
  {
    if (riscv_expand_conditional_move (operands[0], operands[1],
                                     operands[2], operands[3]))
We had to do more than just slap on a TARGET_ZICOND.  I'm a bit 
surprised this worked as-is.  Though we also have bits to handle 
conditions other than eq/ne by first emitting an sCC style insn which 
might be adding complication or cases you hadn't encountered.


Jeff