RISC-V: Fix rawmemchr implementation.

Message ID d9eef014-3fc1-45d2-88bf-6aa4bb0b2fe8@gmail.com
State Committed
Commit cdb34bf5dd10df967b7f72a502a48cc34e284ef0
Headers
Series RISC-V: Fix rawmemchr implementation. |

Checks

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

Commit Message

Robin Dapp Dec. 1, 2023, 3:20 p.m. UTC
  Hi,

this fixes a bug in the rawmemchr implementation by incrementing the
source address by vl * element_size instead of just vl.

This is normally harmless as we will just scan the same region more than
once but, in combination with an older qemu version, would lead to
an execution failure in SPEC2017.

Regards
 Robin


gcc/ChangeLog:

	* config/riscv/riscv-string.cc (expand_rawmemchr): Increment
	source address by vl * element_size.
---
 gcc/config/riscv/riscv-string.cc | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
  

Comments

钟居哲 Dec. 1, 2023, 10:57 p.m. UTC | #1
LGTM。



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-12-01 23:20
To: gcc-patches; palmer; Kito Cheng; jeffreyalaw; juzhe.zhong@rivai.ai
CC: rdapp.gcc
Subject: [PATCH] RISC-V: Fix rawmemchr implementation.
Hi,
 
this fixes a bug in the rawmemchr implementation by incrementing the
source address by vl * element_size instead of just vl.
 
This is normally harmless as we will just scan the same region more than
once but, in combination with an older qemu version, would lead to
an execution failure in SPEC2017.
 
Regards
Robin
 
 
gcc/ChangeLog:
 
* config/riscv/riscv-string.cc (expand_rawmemchr): Increment
source address by vl * element_size.
---
gcc/config/riscv/riscv-string.cc | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
 
diff --git a/gcc/config/riscv/riscv-string.cc b/gcc/config/riscv/riscv-string.cc
index f3a4d3ddd47..594ff49fc5a 100644
--- a/gcc/config/riscv/riscv-string.cc
+++ b/gcc/config/riscv/riscv-string.cc
@@ -1017,6 +1017,8 @@ expand_rawmemchr (machine_mode mode, rtx dst, rtx src, rtx pat)
   machine_mode mask_mode = riscv_vector::get_mask_mode (vmode);
   rtx cnt = gen_reg_rtx (Pmode);
+  emit_move_insn (cnt, CONST0_RTX (Pmode));
+
   rtx end = gen_reg_rtx (Pmode);
   rtx vec = gen_reg_rtx (vmode);
   rtx mask = gen_reg_rtx (mask_mode);
@@ -1033,6 +1035,11 @@ expand_rawmemchr (machine_mode mode, rtx dst, rtx src, rtx pat)
   rtx vsrc = change_address (src, vmode, src_addr);
+  /* Bump the pointer.  */
+  rtx step = gen_reg_rtx (Pmode);
+  emit_insn (gen_rtx_SET (step, gen_rtx_ASHIFT (Pmode, cnt, GEN_INT (shift))));
+  emit_insn (gen_rtx_SET (src_addr, gen_rtx_PLUS (Pmode, src_addr, step)));
+
   /* Emit a first-fault load.  */
   rtx vlops[] = {vec, vsrc};
   emit_vlmax_insn (code_for_pred_fault_load (vmode),
@@ -1055,16 +1062,10 @@ expand_rawmemchr (machine_mode mode, rtx dst, rtx src, rtx pat)
   emit_nonvlmax_insn (code_for_pred_ffs (mask_mode, Pmode),
      riscv_vector::CPOP_OP, vfops, cnt);
-  /* Bump the pointer.  */
-  emit_insn (gen_rtx_SET (src_addr, gen_rtx_PLUS (Pmode, src_addr, cnt)));
-
   /* Emit the loop condition.  */
   rtx test = gen_rtx_LT (VOIDmode, end, const0_rtx);
   emit_jump_insn (gen_cbranch4 (Pmode, test, end, const0_rtx, loop));
-  /*  We overran by CNT, subtract it.  */
-  emit_insn (gen_rtx_SET (src_addr, gen_rtx_MINUS (Pmode, src_addr, cnt)));
-
   /*  We found something at SRC + END * [1,2,4,8].  */
   emit_insn (gen_rtx_SET (end, gen_rtx_ASHIFT (Pmode, end, GEN_INT (shift))));
   emit_insn (gen_rtx_SET (dst, gen_rtx_PLUS (Pmode, src_addr, end)));
-- 
2.43.0
  

Patch

diff --git a/gcc/config/riscv/riscv-string.cc b/gcc/config/riscv/riscv-string.cc
index f3a4d3ddd47..594ff49fc5a 100644
--- a/gcc/config/riscv/riscv-string.cc
+++ b/gcc/config/riscv/riscv-string.cc
@@ -1017,6 +1017,8 @@  expand_rawmemchr (machine_mode mode, rtx dst, rtx src, rtx pat)
   machine_mode mask_mode = riscv_vector::get_mask_mode (vmode);
 
   rtx cnt = gen_reg_rtx (Pmode);
+  emit_move_insn (cnt, CONST0_RTX (Pmode));
+
   rtx end = gen_reg_rtx (Pmode);
   rtx vec = gen_reg_rtx (vmode);
   rtx mask = gen_reg_rtx (mask_mode);
@@ -1033,6 +1035,11 @@  expand_rawmemchr (machine_mode mode, rtx dst, rtx src, rtx pat)
 
   rtx vsrc = change_address (src, vmode, src_addr);
 
+  /* Bump the pointer.  */
+  rtx step = gen_reg_rtx (Pmode);
+  emit_insn (gen_rtx_SET (step, gen_rtx_ASHIFT (Pmode, cnt, GEN_INT (shift))));
+  emit_insn (gen_rtx_SET (src_addr, gen_rtx_PLUS (Pmode, src_addr, step)));
+
   /* Emit a first-fault load.  */
   rtx vlops[] = {vec, vsrc};
   emit_vlmax_insn (code_for_pred_fault_load (vmode),
@@ -1055,16 +1062,10 @@  expand_rawmemchr (machine_mode mode, rtx dst, rtx src, rtx pat)
   emit_nonvlmax_insn (code_for_pred_ffs (mask_mode, Pmode),
 		      riscv_vector::CPOP_OP, vfops, cnt);
 
-  /* Bump the pointer.  */
-  emit_insn (gen_rtx_SET (src_addr, gen_rtx_PLUS (Pmode, src_addr, cnt)));
-
   /* Emit the loop condition.  */
   rtx test = gen_rtx_LT (VOIDmode, end, const0_rtx);
   emit_jump_insn (gen_cbranch4 (Pmode, test, end, const0_rtx, loop));
 
-  /*  We overran by CNT, subtract it.  */
-  emit_insn (gen_rtx_SET (src_addr, gen_rtx_MINUS (Pmode, src_addr, cnt)));
-
   /*  We found something at SRC + END * [1,2,4,8].  */
   emit_insn (gen_rtx_SET (end, gen_rtx_ASHIFT (Pmode, end, GEN_INT (shift))));
   emit_insn (gen_rtx_SET (dst, gen_rtx_PLUS (Pmode, src_addr, end)));