Re: [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV

[Jeff Law via Gcc-patches]

On 5/25/23 18:52, Die Li wrote:
This patch allows less instructions to be used when TARGET_XTHEADCONDMOV is 
enabled.

Provide an example from the existing testcases.

Testcase:
int ConEmv_imm_imm_reg(int x, int y){
   if (x == 1000) return 10;
   return y;
}

Cflags:
-O2 -march=rv64gc_xtheadcondmov -mabi=lp64d

before patch:
ConEmv_imm_imm_reg:
        addi    a5,a0,-1000
        li      a0,10
        th.mvnez        a0,zero,a5
        th.mveqz        a1,zero,a5
        or      a0,a0,a1
        ret

after patch:
ConEmv_imm_imm_reg:
        addi    a5,a0,-1000
        li      a0,10
        th.mvnez        a0,a1,a5
        ret

Signed-off-by: Die Li <li...@eswincomputing.com>

gcc/ChangeLog:

         * config/riscv/riscv.cc (riscv_expand_conditional_move_onesided): 
Delete.
         (riscv_expand_conditional_move):  Reuse the TARGET_SFB_ALU expand 
process for TARGET_XTHEADCONDMOV

gcc/testsuite/ChangeLog:

         * gcc.target/riscv/xtheadcondmov-indirect-rv32.c: Update the output.
         * gcc.target/riscv/xtheadcondmov-indirect-rv64.c: Likewise.
I've made minor formatting adjustments and pushed this to the trunk.


@@ -3492,14 +3462,12 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx 
cons, rtx alt)
        && GET_MODE (op0) == mode
        && GET_MODE (op1) == mode
        && (code == EQ || code == NE))
+        need_eq_ne_p = true;
So the need_eq_ne_p should have indented 2 spaces in from the IF statement.

+
+  if (need_eq_ne_p || (TARGET_SFB_ALU
+          && GET_MODE (op0) == word_mode))This should have been:
  if (need_eq_ne_p
      || (TARGET_SFB_ALU && GET_MODE (op0) == word_mode))

I've fixed this as well.

I would recommend you review the GCC coding guidelines.  While I fixed 
the problems this time, I'll likely ask you to do so in the future.

jeff