diff mbox series

[v2] RISC-V: Correct the mode that is causing the program to fail for XTheadCondMov

Message ID 20250120055610.1867-1-jinma@linux.alibaba.com
State Committed
Commit 9d869296f095a02c37d3721f546ce99663e5417c
Delegated to: Jeff Law
Headers
Series [v2] RISC-V: Correct the mode that is causing the program to fail for XTheadCondMov |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-lint success Lint passed
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
rivoscibot/toolchain-ci-rivos-test success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Jin Ma Jan. 20, 2025, 5:56 a.m. UTC 
  For XTheadCondMov, the bit width of rs2 should always be XLEN-sized, otherwise
the program logic will be wrong.

Reference form
https://github.com/XUANTIE-RV/thead-extension-spec/releases/download/2.3.0/xthead-2023-11-10-2.3.0.pdf

Synopsis
Move if equal zero.

Mnemonic
th.mveqz rd, rs1, rs2

Description
This instruction moves the content of register rs1 into rd if the content of rs2 is 0x0.
Otherwise, the value of rd does not change.

Operation
if (reg[rs2] == 0x0)
  reg[rd] := reg[rs1]

gcc/ChangeLog:

	* config/riscv/thead.md (*th_cond_mov<GPR:mode><GPR2:mode>):
	Change GPR2 to X.
	(*th_cond_mov<GPR:mode>): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/xtheadcondmov-bug.c: New test.
---
 gcc/config/riscv/thead.md                          |  4 ++--
 gcc/testsuite/gcc.target/riscv/xtheadcondmov-bug.c | 12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadcondmov-bug.c

Comments

Jeff Law Jan. 20, 2025, 4:37 p.m. UTC | #1 
On 1/19/25 10:56 PM, Jin Ma wrote:
> For XTheadCondMov, the bit width of rs2 should always be XLEN-sized, otherwise
> the program logic will be wrong.
> 
> Reference form
> https://github.com/XUANTIE-RV/thead-extension-spec/releases/download/2.3.0/xthead-2023-11-10-2.3.0.pdf
> 
> Synopsis
> Move if equal zero.
> 
> Mnemonic
> th.mveqz rd, rs1, rs2
> 
> Description
> This instruction moves the content of register rs1 into rd if the content of rs2 is 0x0.
> Otherwise, the value of rd does not change.
> 
> Operation
> if (reg[rs2] == 0x0)
>    reg[rd] := reg[rs1]
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/thead.md (*th_cond_mov<GPR:mode><GPR2:mode>):
> 	Change GPR2 to X.
> 	(*th_cond_mov<GPR:mode>): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/xtheadcondmov-bug.c: New test.
Thanks.  I've pushed this to the trunk.

Interestingly enough I was wandering around a bit in this code over the 
last week as part of the work on pr114277 (and its related bzs).  We 
need to improve riscv_expand_conditional_move to handle sub-word sizes 
better -- which we've known for a while.

First for the comparison if we start using riscv_extend_operands we'll 
be able to handle the sub-word comparison case at expansion time.

Second we probably need to support sub-word source/destinations.  I'm 
still pondering the best way to handle these -- it's a known weakness 
that we don't handle the sub-word cases well.

Anyway, point is we probably want to do more work in the expander and 
your fix will be critical once we start handling more cases in the expander.

Thanks again,
jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
index 54b9737b4308..d816f3b86dde 100644
--- a/gcc/config/riscv/thead.md
+++ b/gcc/config/riscv/thead.md
@@ -154,11 +154,11 @@  (define_insn "*th_tst<mode>3"
 
 ;; XTheadCondMov
 
-(define_insn "*th_cond_mov<GPR:mode><GPR2:mode>"
+(define_insn "*th_cond_mov<GPR:mode>"
   [(set (match_operand:GPR 0 "register_operand" "=r,r")
 	(if_then_else:GPR
 	 (match_operator 4 "equality_operator"
-		[(match_operand:GPR2 1 "register_operand" "r,r")
+		[(match_operand:X 1 "register_operand" "r,r")
 		 (const_int 0)])
 	 (match_operand:GPR 2 "reg_or_0_operand" "rJ,0")
 	 (match_operand:GPR 3 "reg_or_0_operand" "0,rJ")))]
diff --git a/gcc/testsuite/gcc.target/riscv/xtheadcondmov-bug.c b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-bug.c
new file mode 100644
index 000000000000..01cec6292919
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadcondmov-bug.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile  { target { rv64 } } } */
+/* { dg-options "-march=rv64gc_xtheadcondmov -mabi=lp64d -O2" } */
+
+long long int
+foo (long long int x, long long int y)
+{
+  if (((int) x | (int) y) != 0)
+    return 6;
+  return x + y;
+}
+
+/* { dg-final { scan-assembler-times {\msext\.w\M} 1 } } */