Re: [PATCH][GCC 13] RISC-V: Fix vsetvli local eliminate [PR114747]

2024-05-06 Thread Kito Cheng
Committed to gcc 13 branch, thanks:)

On Tue, May 7, 2024 at 9:20 AM juzhe.zh...@rivai.ai
 wrote:
>
> LGTM。
>
> 
> juzhe.zh...@rivai.ai
>
>
> From: Kito Cheng
> Date: 2024-05-07 09:17
> To: gcc-patches; kito.cheng; palmer; jeffreyalaw; rdapp; juzhe.zhong; pan2.li
> CC: Kito Cheng
> Subject: [PATCH][GCC 13] RISC-V: Fix vsetvli local eliminate [PR114747]
> vsetvli local eliminate is only consider the current demand instead of
> full demand, and it will use that incomplete info to remove vsetvli.
>
> Give following example from PR114747:
>
> vsetvli a5,a1,e8,m4,ta,mu   # 57, ratio=2, sew=8, lmul=4
> vsetvli zero,a5,e16,m8,ta,ma# 58, ratio=2, sew=16, lmul=8
> vle8.v  v8,0(a0)# 13, demand ratio=2
> vzext.vf2   v24,v8  # 14, demand sew=16 and lmul=8
>
> Insn #58 will removed because #57 has satisfied demand of #13, but it's
> not consider #14.
>
> It should doing more demand analyze, but this bug only present in GCC 13
> branch, and we should not change too much on this release branch, so the best
> way is make the check more conservative - remove only if the target
> vsetvl_discard_result having same SEW and LMUL as the source vsetvli.
>
> gcc/ChangeLog:
>
> PR target/114747
> * config/riscv/riscv-vsetvl.cc (local_eliminate_vsetvl_insn):
> Check target vsetvl_discard_result and source vsetvli has same
> SEW and LMUL.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/vsetvl/pr114747.c: New.
> ---
> gcc/config/riscv/riscv-vsetvl.cc   | 10 ++
> .../gcc.target/riscv/rvv/vsetvl/pr114747.c | 18 ++
> 2 files changed, 28 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
>
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc 
> b/gcc/config/riscv/riscv-vsetvl.cc
> index 587c6975a70..e6606b1e4de 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -1106,6 +1106,16 @@ local_eliminate_vsetvl_insn (const vector_insn_info 
> )
>   if (!new_info.skip_avl_compatible_p (dem))
> return;
> +   /* Be more conservative here since we don't really get full
> + demand info for following instructions, also that instruction
> + isn't exist in RTL-SSA yet so we need parse that by low level
> + API rather than vector_insn_info::parse_insn, see PR114747.  */
> +   unsigned last_vsetvli_sew = ::get_sew (PREV_INSN (i->rtl ()));
> +   unsigned last_vsetvli_lmul = ::get_vlmul (PREV_INSN (i->rtl ()));
> +   if (new_info.get_sew() != last_vsetvli_sew ||
> +   new_info.get_vlmul() != last_vsetvli_lmul)
> + return;
> +
>   new_info.set_avl_info (dem.get_avl_info ());
>   new_info = dem.merge (new_info, LOCAL_MERGE);
>   change_vsetvl_insn (insn, new_info);
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c 
> b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
> new file mode 100644
> index 000..c478405e8d6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv32gcv -mabi=ilp32 -fno-tree-vectorize 
> -fno-schedule-insns -fno-schedule-insns2" } */
> +
> +#include "riscv_vector.h"
> +
> +typedef unsigned short char16_t;
> +
> +size_t convert_latin1_to_utf16le(const char *src, size_t len, char16_t *dst) 
> {
> +  char16_t *beg = dst;
> +  for (size_t vl; len > 0; len -= vl, src += vl, dst += vl) {
> +vl = __riscv_vsetvl_e8m4(len);
> +vuint8m4_t v = __riscv_vle8_v_u8m4((uint8_t*)src, vl);
> +__riscv_vse16_v_u16m8((uint16_t*)dst, __riscv_vzext_vf2_u16m8(v, vl), 
> vl);
> +  }
> +  return dst - beg;
> +}
> +
> +/* { dg-final { scan-assembler 
> {vsetvli\s+[a-z0-9]+,\s*[a-x0-9]+,\s*e16,\s*m8,\s*t[au],\s*m[au]} } } */
> --
> 2.34.1
>
>


Re: [PATCH][GCC 13] RISC-V: Fix vsetvli local eliminate [PR114747]

2024-05-06 Thread juzhe.zh...@rivai.ai
LGTM。



juzhe.zh...@rivai.ai
 
From: Kito Cheng
Date: 2024-05-07 09:17
To: gcc-patches; kito.cheng; palmer; jeffreyalaw; rdapp; juzhe.zhong; pan2.li
CC: Kito Cheng
Subject: [PATCH][GCC 13] RISC-V: Fix vsetvli local eliminate [PR114747]
vsetvli local eliminate is only consider the current demand instead of
full demand, and it will use that incomplete info to remove vsetvli.
 
Give following example from PR114747:
 
vsetvli a5,a1,e8,m4,ta,mu   # 57, ratio=2, sew=8, lmul=4
vsetvli zero,a5,e16,m8,ta,ma# 58, ratio=2, sew=16, lmul=8
vle8.v  v8,0(a0)# 13, demand ratio=2
vzext.vf2   v24,v8  # 14, demand sew=16 and lmul=8
 
Insn #58 will removed because #57 has satisfied demand of #13, but it's
not consider #14.
 
It should doing more demand analyze, but this bug only present in GCC 13
branch, and we should not change too much on this release branch, so the best
way is make the check more conservative - remove only if the target
vsetvl_discard_result having same SEW and LMUL as the source vsetvli.
 
gcc/ChangeLog:
 
PR target/114747
* config/riscv/riscv-vsetvl.cc (local_eliminate_vsetvl_insn):
Check target vsetvl_discard_result and source vsetvli has same
SEW and LMUL.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/vsetvl/pr114747.c: New.
---
gcc/config/riscv/riscv-vsetvl.cc   | 10 ++
.../gcc.target/riscv/rvv/vsetvl/pr114747.c | 18 ++
2 files changed, 28 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
 
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 587c6975a70..e6606b1e4de 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -1106,6 +1106,16 @@ local_eliminate_vsetvl_insn (const vector_insn_info )
  if (!new_info.skip_avl_compatible_p (dem))
return;
+   /* Be more conservative here since we don't really get full
+ demand info for following instructions, also that instruction
+ isn't exist in RTL-SSA yet so we need parse that by low level
+ API rather than vector_insn_info::parse_insn, see PR114747.  */
+   unsigned last_vsetvli_sew = ::get_sew (PREV_INSN (i->rtl ()));
+   unsigned last_vsetvli_lmul = ::get_vlmul (PREV_INSN (i->rtl ()));
+   if (new_info.get_sew() != last_vsetvli_sew ||
+   new_info.get_vlmul() != last_vsetvli_lmul)
+ return;
+
  new_info.set_avl_info (dem.get_avl_info ());
  new_info = dem.merge (new_info, LOCAL_MERGE);
  change_vsetvl_insn (insn, new_info);
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c 
b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
new file mode 100644
index 000..c478405e8d6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv32gcv -mabi=ilp32 -fno-tree-vectorize 
-fno-schedule-insns -fno-schedule-insns2" } */
+
+#include "riscv_vector.h"
+
+typedef unsigned short char16_t;
+
+size_t convert_latin1_to_utf16le(const char *src, size_t len, char16_t *dst) {
+  char16_t *beg = dst;
+  for (size_t vl; len > 0; len -= vl, src += vl, dst += vl) {
+vl = __riscv_vsetvl_e8m4(len);
+vuint8m4_t v = __riscv_vle8_v_u8m4((uint8_t*)src, vl);
+__riscv_vse16_v_u16m8((uint16_t*)dst, __riscv_vzext_vf2_u16m8(v, vl), vl);
+  }
+  return dst - beg;
+}
+
+/* { dg-final { scan-assembler 
{vsetvli\s+[a-z0-9]+,\s*[a-x0-9]+,\s*e16,\s*m8,\s*t[au],\s*m[au]} } } */
-- 
2.34.1
 
 


[PATCH][GCC 13] RISC-V: Fix vsetvli local eliminate [PR114747]

2024-05-06 Thread Kito Cheng
vsetvli local eliminate is only consider the current demand instead of
full demand, and it will use that incomplete info to remove vsetvli.

Give following example from PR114747:

vsetvli a5,a1,e8,m4,ta,mu   # 57, ratio=2, sew=8, lmul=4
vsetvli zero,a5,e16,m8,ta,ma# 58, ratio=2, sew=16, lmul=8
vle8.v  v8,0(a0)# 13, demand ratio=2
vzext.vf2   v24,v8  # 14, demand sew=16 and lmul=8

Insn #58 will removed because #57 has satisfied demand of #13, but it's
not consider #14.

It should doing more demand analyze, but this bug only present in GCC 13
branch, and we should not change too much on this release branch, so the best
way is make the check more conservative - remove only if the target
vsetvl_discard_result having same SEW and LMUL as the source vsetvli.

gcc/ChangeLog:

PR target/114747
* config/riscv/riscv-vsetvl.cc (local_eliminate_vsetvl_insn):
Check target vsetvl_discard_result and source vsetvli has same
SEW and LMUL.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/vsetvl/pr114747.c: New.
---
 gcc/config/riscv/riscv-vsetvl.cc   | 10 ++
 .../gcc.target/riscv/rvv/vsetvl/pr114747.c | 18 ++
 2 files changed, 28 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 587c6975a70..e6606b1e4de 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -1106,6 +1106,16 @@ local_eliminate_vsetvl_insn (const vector_insn_info )
  if (!new_info.skip_avl_compatible_p (dem))
return;
 
+ /* Be more conservative here since we don't really get full
+demand info for following instructions, also that instruction
+isn't exist in RTL-SSA yet so we need parse that by low level
+API rather than vector_insn_info::parse_insn, see PR114747.  */
+ unsigned last_vsetvli_sew = ::get_sew (PREV_INSN (i->rtl ()));
+ unsigned last_vsetvli_lmul = ::get_vlmul (PREV_INSN (i->rtl ()));
+ if (new_info.get_sew() != last_vsetvli_sew ||
+ new_info.get_vlmul() != last_vsetvli_lmul)
+   return;
+
  new_info.set_avl_info (dem.get_avl_info ());
  new_info = dem.merge (new_info, LOCAL_MERGE);
  change_vsetvl_insn (insn, new_info);
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c 
b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
new file mode 100644
index 000..c478405e8d6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr114747.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv32gcv -mabi=ilp32 -fno-tree-vectorize 
-fno-schedule-insns -fno-schedule-insns2" } */
+
+#include "riscv_vector.h"
+
+typedef unsigned short char16_t;
+
+size_t convert_latin1_to_utf16le(const char *src, size_t len, char16_t *dst) {
+  char16_t *beg = dst;
+  for (size_t vl; len > 0; len -= vl, src += vl, dst += vl) {
+vl = __riscv_vsetvl_e8m4(len);
+vuint8m4_t v = __riscv_vle8_v_u8m4((uint8_t*)src, vl);
+__riscv_vse16_v_u16m8((uint16_t*)dst, __riscv_vzext_vf2_u16m8(v, vl), vl);
+  }
+  return dst - beg;
+}
+
+/* { dg-final { scan-assembler 
{vsetvli\s+[a-z0-9]+,\s*[a-x0-9]+,\s*e16,\s*m8,\s*t[au],\s*m[au]} } } */
-- 
2.34.1