* Re: [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV
2023-05-26 0:52 [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV Die Li
@ 2023-05-26 2:43 ` Kito Cheng
2023-05-26 4:01 ` Jeff Law
2023-05-26 8:20 ` Philipp Tomsich
2023-05-29 17:14 ` Jeff Law
2 siblings, 1 reply; 5+ messages in thread
From: Kito Cheng @ 2023-05-26 2:43 UTC (permalink / raw)
To: Die Li
Cc: GCC Patches, Palmer Dabbelt, Jeff Law, Christoph Müllner,
Philipp Tomsich
[-- Attachment #1: Type: text/plain, Size: 11929 bytes --]
I would defer this to vrull or t-head folks :)
Die Li <lidie@eswincomputing.com> 於 2023年5月26日 週五 08:53 寫道:
> 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 <lidie@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.
> ---
> gcc/config/riscv/riscv.cc | 44 +++--------------
> .../riscv/xtheadcondmov-indirect-rv32.c | 48 +++++++------------
> .../riscv/xtheadcondmov-indirect-rv64.c | 48 +++++++------------
> 3 files changed, 42 insertions(+), 98 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 09fc9e5d95e..8b8ac9181ba 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3442,37 +3442,6 @@ riscv_expand_conditional_branch (rtx label,
> rtx_code code, rtx op0, rtx op1)
> emit_jump_insn (gen_condjump (condition, label));
> }
>
> -/* Helper to emit two one-sided conditional moves for the movecc. */
> -
> -static void
> -riscv_expand_conditional_move_onesided (rtx dest, rtx cons, rtx alt,
> - rtx_code code, rtx op0, rtx op1)
> -{
> - machine_mode mode = GET_MODE (dest);
> -
> - gcc_assert (GET_MODE_CLASS (mode) == MODE_INT);
> - gcc_assert (reg_or_0_operand (cons, mode));
> - gcc_assert (reg_or_0_operand (alt, mode));
> -
> - riscv_emit_int_compare (&code, &op0, &op1, true);
> - rtx cond = gen_rtx_fmt_ee (code, mode, op0, op1);
> -
> - rtx tmp1 = gen_reg_rtx (mode);
> - rtx tmp2 = gen_reg_rtx (mode);
> -
> - emit_insn (gen_rtx_SET (tmp1, gen_rtx_IF_THEN_ELSE (mode, cond,
> - cons, const0_rtx)));
> -
> - /* We need to expand a sequence for both blocks and we do that such,
> - that the second conditional move will use the inverted condition.
> - We use temporaries that are or'd to the dest register. */
> - cond = gen_rtx_fmt_ee ((code == EQ) ? NE : EQ, mode, op0, op1);
> - emit_insn (gen_rtx_SET (tmp2, gen_rtx_IF_THEN_ELSE (mode, cond,
> - alt, const0_rtx)));
> -
> - emit_insn (gen_rtx_SET (dest, gen_rtx_IOR (mode, tmp1, tmp2)));
> - }
> -
> /* Emit a cond move: If OP holds, move CONS to DEST; else move ALT to
> DEST.
> Return 0 if expansion failed. */
>
> @@ -3483,6 +3452,7 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx
> cons, rtx alt)
> rtx_code code = GET_CODE (op);
> rtx op0 = XEXP (op, 0);
> rtx op1 = XEXP (op, 1);
> + bool need_eq_ne_p = false;
>
> if (TARGET_XTHEADCONDMOV
> && GET_MODE_CLASS (mode) == MODE_INT
> @@ -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;
> +
> + if (need_eq_ne_p || (TARGET_SFB_ALU
> + && GET_MODE (op0) == word_mode))
> {
> - riscv_expand_conditional_move_onesided (dest, cons, alt, code, op0,
> op1);
> - return true;
> - }
> - else if (TARGET_SFB_ALU
> - && GET_MODE (op0) == word_mode)
> - {
> - riscv_emit_int_compare (&code, &op0, &op1);
> + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
> rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
>
> /* The expander allows (const_int 0) for CONS for the benefit of
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> index 9afdc2eabfd..e2b135f3d00 100644
> --- a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> @@ -1,15 +1,13 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -march=rv32gc_xtheadcondmov -mabi=ilp32
> -mriscv-attribute" } */
> -/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" } } */
> +/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-Os" "-Og" "-O3" "-Oz"
> "-flto"} } */
> /* { dg-final { check-function-bodies "**" "" } } */
>
> /*
> **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
> +** th.mvnez a0,a1,a5
> ** ret
> */
> int ConEmv_imm_imm_reg(int x, int y){
> @@ -20,9 +18,8 @@ int ConEmv_imm_imm_reg(int x, int y){
> /*
> **ConEmv_imm_reg_reg:
> ** addi a5,a0,-1000
> -** th.mvnez a1,zero,a5
> -** th.mveqz a2,zero,a5
> -** or a0,a1,a2
> +** th.mveqz a2,a1,a5
> +** mv a0,a2
> ** ret
> */
> int ConEmv_imm_reg_reg(int x, int y, int z){
> @@ -34,9 +31,7 @@ int ConEmv_imm_reg_reg(int x, int y, int z){
> **ConEmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mvnez a0,zero,a1
> -** th.mveqz a2,zero,a1
> -** or a0,a0,a2
> +** th.mvnez a0,a2,a1
> ** ret
> */
> int ConEmv_reg_imm_reg(int x, int y, int z){
> @@ -47,9 +42,8 @@ int ConEmv_reg_imm_reg(int x, int y, int z){
> /*
> **ConEmv_reg_reg_reg:
> ** sub a1,a0,a1
> -** th.mvnez a2,zero,a1
> -** th.mveqz a3,zero,a1
> -** or a0,a2,a3
> +** th.mveqz a3,a2,a1
> +** mv a0,a3
> ** ret
> */
> int ConEmv_reg_reg_reg(int x, int y, int z, int n){
> @@ -59,12 +53,10 @@ int ConEmv_reg_reg_reg(int x, int y, int z, int n){
>
> /*
> **ConNmv_imm_imm_reg:
> -** li a5,9998336
> -** addi a4,a0,-1000
> -** addi a5,a5,1664
> -** th.mvnez a1,zero,a4
> -** th.mveqz a5,zero,a4
> -** or a0,a1,a5
> +** addi a5,a0,-1000
> +** li a0,9998336
> +** addi a0,a0,1664
> +** th.mveqz a0,a1,a5
> ** ret
> */
> int ConNmv_imm_imm_reg(int x, int y){
> @@ -74,10 +66,9 @@ int ConNmv_imm_imm_reg(int x, int y){
>
> /*
> **ConNmv_imm_reg_reg:
> -** addi a5,a0,-1000
> -** th.mveqz a1,zero,a5
> -** th.mvnez a2,zero,a5
> -** or a0,a1,a2
> +** addi a0,a0,-1000
> +** th.mvnez a2,a1,a0
> +** mv a0,a2
> ** ret
> */
> int ConNmv_imm_reg_reg(int x, int y, int z){
> @@ -89,9 +80,7 @@ int ConNmv_imm_reg_reg(int x, int y, int z){
> **ConNmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mveqz a0,zero,a1
> -** th.mvnez a2,zero,a1
> -** or a0,a0,a2
> +** th.mveqz a0,a2,a1
> ** ret
> */
> int ConNmv_reg_imm_reg(int x, int y, int z){
> @@ -101,10 +90,9 @@ int ConNmv_reg_imm_reg(int x, int y, int z){
>
> /*
> **ConNmv_reg_reg_reg:
> -** sub a1,a0,a1
> -** th.mveqz a2,zero,a1
> -** th.mvnez a3,zero,a1
> -** or a0,a2,a3
> +** sub a0,a0,a1
> +** th.mvnez a3,a2,a0
> +** mv a0,a3
> ** ret
> */
> int ConNmv_reg_reg_reg(int x, int y, int z, int n){
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> index a1982fd90bd..99956f8496c 100644
> --- a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> @@ -1,15 +1,13 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -march=rv64gc_xtheadcondmov -mabi=lp64d
> -mriscv-attribute" } */
> -/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" } } */
> +/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-Os" "-Og" "-O3" "-Oz"
> "-flto"} } */
> /* { dg-final { check-function-bodies "**" "" } } */
>
> /*
> **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
> +** th.mvnez a0,a1,a5
> ** ret
> */
> int ConEmv_imm_imm_reg(int x, int y){
> @@ -19,10 +17,9 @@ int ConEmv_imm_imm_reg(int x, int y){
>
> /*
> **ConEmv_imm_reg_reg:
> -** addi a5,a0,-1000
> -** th.mvnez a1,zero,a5
> -** th.mveqz a2,zero,a5
> -** or a0,a1,a2
> +** addi a0,a0,-1000
> +** th.mveqz a2,a1,a5
> +** mv a0,a2
> ** ret
> */
> int ConEmv_imm_reg_reg(int x, int y, int z){
> @@ -34,9 +31,7 @@ int ConEmv_imm_reg_reg(int x, int y, int z){
> **ConEmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mvnez a0,zero,a1
> -** th.mveqz a2,zero,a1
> -** or a0,a0,a2
> +** th.mvnez a0,a2,a1
> ** ret
> */
> int ConEmv_reg_imm_reg(int x, int y, int z){
> @@ -47,9 +42,8 @@ int ConEmv_reg_imm_reg(int x, int y, int z){
> /*
> **ConEmv_reg_reg_reg:
> ** sub a1,a0,a1
> -** th.mvnez a2,zero,a1
> -** th.mveqz a3,zero,a1
> -** or a0,a2,a3
> +** th.mveqz a3,a2,a1
> +** mv a0,a3
> ** ret
> */
> int ConEmv_reg_reg_reg(int x, int y, int z, int n){
> @@ -59,12 +53,10 @@ int ConEmv_reg_reg_reg(int x, int y, int z, int n){
>
> /*
> **ConNmv_imm_imm_reg:
> -** li a5,9998336
> -** addi a4,a0,-1000
> -** addi a5,a5,1664
> -** th.mvnez a1,zero,a4
> -** th.mveqz a5,zero,a4
> -** or a0,a1,a5
> +** addi a5,a0,-1000
> +** li a0,9998336
> +** addi a0,a0,1664
> +** th.mveqz a0,a1,a5
> ** ret
> */
> int ConNmv_imm_imm_reg(int x, int y){
> @@ -75,9 +67,8 @@ int ConNmv_imm_imm_reg(int x, int y){
> /*
> **ConNmv_imm_reg_reg:
> ** addi a5,a0,-1000
> -** th.mveqz a1,zero,a5
> -** th.mvnez a2,zero,a5
> -** or a0,a1,a2
> +** th.mvnez a2,a1,a0
> +** mv a0,a2
> ** ret
> */
> int ConNmv_imm_reg_reg(int x, int y, int z){
> @@ -89,9 +80,7 @@ int ConNmv_imm_reg_reg(int x, int y, int z){
> **ConNmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mveqz a0,zero,a1
> -** th.mvnez a2,zero,a1
> -** or a0,a0,a2
> +** th.mveqz a0,a2,a1
> ** ret
> */
> int ConNmv_reg_imm_reg(int x, int y, int z){
> @@ -101,10 +90,9 @@ int ConNmv_reg_imm_reg(int x, int y, int z){
>
> /*
> **ConNmv_reg_reg_reg:
> -** sub a1,a0,a1
> -** th.mveqz a2,zero,a1
> -** th.mvnez a3,zero,a1
> -** or a0,a2,a3
> +** sub a0,a0,a1
> +** th.mvnez a3,a2,a0
> +** mv a0,a3
> ** ret
> */
> int ConNmv_reg_reg_reg(int x, int y, int z, int n){
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV
2023-05-26 2:43 ` Kito Cheng
@ 2023-05-26 4:01 ` Jeff Law
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2023-05-26 4:01 UTC (permalink / raw)
To: Kito Cheng, Die Li
Cc: GCC Patches, Palmer Dabbelt, Christoph Müllner, Philipp Tomsich
On 5/25/23 20:43, Kito Cheng wrote:
> I would defer this to vrull or t-head folks :)
Given the overlap between where this is going and how I think we should
be handling Zicondops, I'll take it. It overlaps with work I've had
Raphael doing recently.
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV
2023-05-26 0:52 [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV Die Li
2023-05-26 2:43 ` Kito Cheng
@ 2023-05-26 8:20 ` Philipp Tomsich
2023-05-29 17:14 ` Jeff Law
2 siblings, 0 replies; 5+ messages in thread
From: Philipp Tomsich @ 2023-05-26 8:20 UTC (permalink / raw)
To: Die Li; +Cc: gcc-patches, kito.cheng, palmer, jeffreyalaw
LGTM. Happy to move this forward, once it receives an OK from one of you.
--Philipp.
On Fri, 26 May 2023 at 02:53, Die Li <lidie@eswincomputing.com> 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 <lidie@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.
> ---
> gcc/config/riscv/riscv.cc | 44 +++--------------
> .../riscv/xtheadcondmov-indirect-rv32.c | 48 +++++++------------
> .../riscv/xtheadcondmov-indirect-rv64.c | 48 +++++++------------
> 3 files changed, 42 insertions(+), 98 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 09fc9e5d95e..8b8ac9181ba 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3442,37 +3442,6 @@ riscv_expand_conditional_branch (rtx label, rtx_code code, rtx op0, rtx op1)
> emit_jump_insn (gen_condjump (condition, label));
> }
>
> -/* Helper to emit two one-sided conditional moves for the movecc. */
> -
> -static void
> -riscv_expand_conditional_move_onesided (rtx dest, rtx cons, rtx alt,
> - rtx_code code, rtx op0, rtx op1)
> -{
> - machine_mode mode = GET_MODE (dest);
> -
> - gcc_assert (GET_MODE_CLASS (mode) == MODE_INT);
> - gcc_assert (reg_or_0_operand (cons, mode));
> - gcc_assert (reg_or_0_operand (alt, mode));
> -
> - riscv_emit_int_compare (&code, &op0, &op1, true);
> - rtx cond = gen_rtx_fmt_ee (code, mode, op0, op1);
> -
> - rtx tmp1 = gen_reg_rtx (mode);
> - rtx tmp2 = gen_reg_rtx (mode);
> -
> - emit_insn (gen_rtx_SET (tmp1, gen_rtx_IF_THEN_ELSE (mode, cond,
> - cons, const0_rtx)));
> -
> - /* We need to expand a sequence for both blocks and we do that such,
> - that the second conditional move will use the inverted condition.
> - We use temporaries that are or'd to the dest register. */
> - cond = gen_rtx_fmt_ee ((code == EQ) ? NE : EQ, mode, op0, op1);
> - emit_insn (gen_rtx_SET (tmp2, gen_rtx_IF_THEN_ELSE (mode, cond,
> - alt, const0_rtx)));
> -
> - emit_insn (gen_rtx_SET (dest, gen_rtx_IOR (mode, tmp1, tmp2)));
> - }
> -
> /* Emit a cond move: If OP holds, move CONS to DEST; else move ALT to DEST.
> Return 0 if expansion failed. */
>
> @@ -3483,6 +3452,7 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx cons, rtx alt)
> rtx_code code = GET_CODE (op);
> rtx op0 = XEXP (op, 0);
> rtx op1 = XEXP (op, 1);
> + bool need_eq_ne_p = false;
>
> if (TARGET_XTHEADCONDMOV
> && GET_MODE_CLASS (mode) == MODE_INT
> @@ -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;
> +
> + if (need_eq_ne_p || (TARGET_SFB_ALU
> + && GET_MODE (op0) == word_mode))
> {
> - riscv_expand_conditional_move_onesided (dest, cons, alt, code, op0, op1);
> - return true;
> - }
> - else if (TARGET_SFB_ALU
> - && GET_MODE (op0) == word_mode)
> - {
> - riscv_emit_int_compare (&code, &op0, &op1);
> + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
> rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
>
> /* The expander allows (const_int 0) for CONS for the benefit of
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> index 9afdc2eabfd..e2b135f3d00 100644
> --- a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv32.c
> @@ -1,15 +1,13 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -march=rv32gc_xtheadcondmov -mabi=ilp32 -mriscv-attribute" } */
> -/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" } } */
> +/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-Os" "-Og" "-O3" "-Oz" "-flto"} } */
> /* { dg-final { check-function-bodies "**" "" } } */
>
> /*
> **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
> +** th.mvnez a0,a1,a5
> ** ret
> */
> int ConEmv_imm_imm_reg(int x, int y){
> @@ -20,9 +18,8 @@ int ConEmv_imm_imm_reg(int x, int y){
> /*
> **ConEmv_imm_reg_reg:
> ** addi a5,a0,-1000
> -** th.mvnez a1,zero,a5
> -** th.mveqz a2,zero,a5
> -** or a0,a1,a2
> +** th.mveqz a2,a1,a5
> +** mv a0,a2
> ** ret
> */
> int ConEmv_imm_reg_reg(int x, int y, int z){
> @@ -34,9 +31,7 @@ int ConEmv_imm_reg_reg(int x, int y, int z){
> **ConEmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mvnez a0,zero,a1
> -** th.mveqz a2,zero,a1
> -** or a0,a0,a2
> +** th.mvnez a0,a2,a1
> ** ret
> */
> int ConEmv_reg_imm_reg(int x, int y, int z){
> @@ -47,9 +42,8 @@ int ConEmv_reg_imm_reg(int x, int y, int z){
> /*
> **ConEmv_reg_reg_reg:
> ** sub a1,a0,a1
> -** th.mvnez a2,zero,a1
> -** th.mveqz a3,zero,a1
> -** or a0,a2,a3
> +** th.mveqz a3,a2,a1
> +** mv a0,a3
> ** ret
> */
> int ConEmv_reg_reg_reg(int x, int y, int z, int n){
> @@ -59,12 +53,10 @@ int ConEmv_reg_reg_reg(int x, int y, int z, int n){
>
> /*
> **ConNmv_imm_imm_reg:
> -** li a5,9998336
> -** addi a4,a0,-1000
> -** addi a5,a5,1664
> -** th.mvnez a1,zero,a4
> -** th.mveqz a5,zero,a4
> -** or a0,a1,a5
> +** addi a5,a0,-1000
> +** li a0,9998336
> +** addi a0,a0,1664
> +** th.mveqz a0,a1,a5
> ** ret
> */
> int ConNmv_imm_imm_reg(int x, int y){
> @@ -74,10 +66,9 @@ int ConNmv_imm_imm_reg(int x, int y){
>
> /*
> **ConNmv_imm_reg_reg:
> -** addi a5,a0,-1000
> -** th.mveqz a1,zero,a5
> -** th.mvnez a2,zero,a5
> -** or a0,a1,a2
> +** addi a0,a0,-1000
> +** th.mvnez a2,a1,a0
> +** mv a0,a2
> ** ret
> */
> int ConNmv_imm_reg_reg(int x, int y, int z){
> @@ -89,9 +80,7 @@ int ConNmv_imm_reg_reg(int x, int y, int z){
> **ConNmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mveqz a0,zero,a1
> -** th.mvnez a2,zero,a1
> -** or a0,a0,a2
> +** th.mveqz a0,a2,a1
> ** ret
> */
> int ConNmv_reg_imm_reg(int x, int y, int z){
> @@ -101,10 +90,9 @@ int ConNmv_reg_imm_reg(int x, int y, int z){
>
> /*
> **ConNmv_reg_reg_reg:
> -** sub a1,a0,a1
> -** th.mveqz a2,zero,a1
> -** th.mvnez a3,zero,a1
> -** or a0,a2,a3
> +** sub a0,a0,a1
> +** th.mvnez a3,a2,a0
> +** mv a0,a3
> ** ret
> */
> int ConNmv_reg_reg_reg(int x, int y, int z, int n){
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> index a1982fd90bd..99956f8496c 100644
> --- a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-indirect-rv64.c
> @@ -1,15 +1,13 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -march=rv64gc_xtheadcondmov -mabi=lp64d -mriscv-attribute" } */
> -/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" } } */
> +/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-Os" "-Og" "-O3" "-Oz" "-flto"} } */
> /* { dg-final { check-function-bodies "**" "" } } */
>
> /*
> **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
> +** th.mvnez a0,a1,a5
> ** ret
> */
> int ConEmv_imm_imm_reg(int x, int y){
> @@ -19,10 +17,9 @@ int ConEmv_imm_imm_reg(int x, int y){
>
> /*
> **ConEmv_imm_reg_reg:
> -** addi a5,a0,-1000
> -** th.mvnez a1,zero,a5
> -** th.mveqz a2,zero,a5
> -** or a0,a1,a2
> +** addi a0,a0,-1000
> +** th.mveqz a2,a1,a5
> +** mv a0,a2
> ** ret
> */
> int ConEmv_imm_reg_reg(int x, int y, int z){
> @@ -34,9 +31,7 @@ int ConEmv_imm_reg_reg(int x, int y, int z){
> **ConEmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mvnez a0,zero,a1
> -** th.mveqz a2,zero,a1
> -** or a0,a0,a2
> +** th.mvnez a0,a2,a1
> ** ret
> */
> int ConEmv_reg_imm_reg(int x, int y, int z){
> @@ -47,9 +42,8 @@ int ConEmv_reg_imm_reg(int x, int y, int z){
> /*
> **ConEmv_reg_reg_reg:
> ** sub a1,a0,a1
> -** th.mvnez a2,zero,a1
> -** th.mveqz a3,zero,a1
> -** or a0,a2,a3
> +** th.mveqz a3,a2,a1
> +** mv a0,a3
> ** ret
> */
> int ConEmv_reg_reg_reg(int x, int y, int z, int n){
> @@ -59,12 +53,10 @@ int ConEmv_reg_reg_reg(int x, int y, int z, int n){
>
> /*
> **ConNmv_imm_imm_reg:
> -** li a5,9998336
> -** addi a4,a0,-1000
> -** addi a5,a5,1664
> -** th.mvnez a1,zero,a4
> -** th.mveqz a5,zero,a4
> -** or a0,a1,a5
> +** addi a5,a0,-1000
> +** li a0,9998336
> +** addi a0,a0,1664
> +** th.mveqz a0,a1,a5
> ** ret
> */
> int ConNmv_imm_imm_reg(int x, int y){
> @@ -75,9 +67,8 @@ int ConNmv_imm_imm_reg(int x, int y){
> /*
> **ConNmv_imm_reg_reg:
> ** addi a5,a0,-1000
> -** th.mveqz a1,zero,a5
> -** th.mvnez a2,zero,a5
> -** or a0,a1,a2
> +** th.mvnez a2,a1,a0
> +** mv a0,a2
> ** ret
> */
> int ConNmv_imm_reg_reg(int x, int y, int z){
> @@ -89,9 +80,7 @@ int ConNmv_imm_reg_reg(int x, int y, int z){
> **ConNmv_reg_imm_reg:
> ** sub a1,a0,a1
> ** li a0,10
> -** th.mveqz a0,zero,a1
> -** th.mvnez a2,zero,a1
> -** or a0,a0,a2
> +** th.mveqz a0,a2,a1
> ** ret
> */
> int ConNmv_reg_imm_reg(int x, int y, int z){
> @@ -101,10 +90,9 @@ int ConNmv_reg_imm_reg(int x, int y, int z){
>
> /*
> **ConNmv_reg_reg_reg:
> -** sub a1,a0,a1
> -** th.mveqz a2,zero,a1
> -** th.mvnez a3,zero,a1
> -** or a0,a2,a3
> +** sub a0,a0,a1
> +** th.mvnez a3,a2,a0
> +** mv a0,a3
> ** ret
> */
> int ConNmv_reg_reg_reg(int x, int y, int z, int n){
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV
2023-05-26 0:52 [PATCH] RISC-V: Optimize TARGET_XTHEADCONDMOV Die Li
2023-05-26 2:43 ` Kito Cheng
2023-05-26 8:20 ` Philipp Tomsich
@ 2023-05-29 17:14 ` Jeff Law
2 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2023-05-29 17:14 UTC (permalink / raw)
To: Die Li, gcc-patches; +Cc: kito.cheng, palmer
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 <lidie@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
^ permalink raw reply [flat|nested] 5+ messages in thread