diff mbox series

RISC-V: Set the natural size of constant vector mask modes to one RVV data vector.

Message ID 20230620060705.22235-1-xuli1@eswincomputing.com
State Accepted
Headers
Series RISC-V: Set the natural size of constant vector mask modes to one RVV data vector. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Li Xu June 20, 2023, 6:07 a.m. UTC 
  If reinterpret vnx2bi as vnx16bi, vnx16bi must occupy no more of the underlying
registers than vnx2bi.

Consider this following case:
void test_vreinterpret_v_b64_i8m1 (uint8_t *in, int8_t *out)
{
  vbool64_t vmask = __riscv_vlm_v_b64 (in, 2);
  vint8m1_t vout = __riscv_vreinterpret_v_b64_i8m1 (vmask);
  __riscv_vse8_v_i8m1(out, vout, 16);
}

compiler parameters: -march=rv64gcv -mabi=lp64d --param=riscv-autovec-preference=fixed-vlmax -O3
Compilation fails with:
test_vreinterpret_v_b64_i8m1during RTL pass: expand

test.c: In function 'test_vreinterpret_v_b64_i8m1':
test.c:11:22: internal compiler error: in gen_lowpart_general, at rtlhooks.cc:57
   11 |     vint8m1_t vout = __riscv_vreinterpret_v_b64_i8m1(src);
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0xf11876 gen_lowpart_general(machine_mode, rtx_def*)
        ../.././riscv-gcc/gcc/rtlhooks.cc:57
0x191435e gen_vreinterpretvnx16qi(rtx_def*, rtx_def*)
        ../.././riscv-gcc/gcc/config/riscv/vector.md:486
0xe08858 maybe_expand_insn(insn_code, unsigned int, expand_operand*)
        ../.././riscv-gcc/gcc/optabs.cc:8213
0x1471209 riscv_vector::function_expander::generate_insn(insn_code)
        ../.././riscv-gcc/gcc/config/riscv/riscv-vector-builtins.cc:3813
0x147629c riscv_vector::function_expander::expand()
        ../.././riscv-gcc/gcc/config/riscv/riscv-vector-builtins.h:520
0x147629c riscv_vector::expand_builtin(unsigned int, tree_node*, rtx_def*)
        ../.././riscv-gcc/gcc/config/riscv/riscv-vector-builtins.cc:4103
0x9868f9 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        ../.././riscv-gcc/gcc/builtins.cc:7342

gcc/ChangeLog:

        * config/riscv/riscv.cc (riscv_regmode_natural_size): set the natural size of vector mask mode to one rvv register.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c: New test.
---
 gcc/config/riscv/riscv.cc                             |  4 ++++
 .../gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c  | 11 +++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c

Comments

juzhe.zhong@rivai.ai June 20, 2023, 6:32 a.m. UTC | #1 
Good catch !
Thanks for fixing this. 

Some nit coments:

Could you add some comments above. like:

/* RVV mask modes always consume a single register.  */
if (GET_MODE_CLASS (mode) == MODE_VECTOR_BOOL)
    return BYTES_PER_RISCV_VECTOR;

If reinterpret vnx2bi as vnx16bi, vnx16bi must occupy no more of the underlying
registers than vnx2bi.

And adjust commit log "vnx16bi" into "vnx16qi".

Otherwise, LGTM.

Thanks.


juzhe.zhong@rivai.ai
 
From: Li Xu
Date: 2023-06-20 14:07
To: gcc-patches
CC: kito.cheng; palmer; juzhe.zhong; Li Xu
Subject: [PATCH] RISC-V: Set the natural size of constant vector mask modes to one RVV data vector.
If reinterpret vnx2bi as vnx16bi, vnx16bi must occupy no more of the underlying
registers than vnx2bi.
 
Consider this following case:
void test_vreinterpret_v_b64_i8m1 (uint8_t *in, int8_t *out)
{
  vbool64_t vmask = __riscv_vlm_v_b64 (in, 2);
  vint8m1_t vout = __riscv_vreinterpret_v_b64_i8m1 (vmask);
  __riscv_vse8_v_i8m1(out, vout, 16);
}
 
compiler parameters: -march=rv64gcv -mabi=lp64d --param=riscv-autovec-preference=fixed-vlmax -O3
Compilation fails with:
test_vreinterpret_v_b64_i8m1during RTL pass: expand
 
test.c: In function 'test_vreinterpret_v_b64_i8m1':
test.c:11:22: internal compiler error: in gen_lowpart_general, at rtlhooks.cc:57
   11 |     vint8m1_t vout = __riscv_vreinterpret_v_b64_i8m1(src);
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0xf11876 gen_lowpart_general(machine_mode, rtx_def*)
        ../.././riscv-gcc/gcc/rtlhooks.cc:57
0x191435e gen_vreinterpretvnx16qi(rtx_def*, rtx_def*)
        ../.././riscv-gcc/gcc/config/riscv/vector.md:486
0xe08858 maybe_expand_insn(insn_code, unsigned int, expand_operand*)
        ../.././riscv-gcc/gcc/optabs.cc:8213
0x1471209 riscv_vector::function_expander::generate_insn(insn_code)
        ../.././riscv-gcc/gcc/config/riscv/riscv-vector-builtins.cc:3813
0x147629c riscv_vector::function_expander::expand()
        ../.././riscv-gcc/gcc/config/riscv/riscv-vector-builtins.h:520
0x147629c riscv_vector::expand_builtin(unsigned int, tree_node*, rtx_def*)
        ../.././riscv-gcc/gcc/config/riscv/riscv-vector-builtins.cc:4103
0x9868f9 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        ../.././riscv-gcc/gcc/builtins.cc:7342
 
gcc/ChangeLog:
 
        * config/riscv/riscv.cc (riscv_regmode_natural_size): set the natural size of vector mask mode to one rvv register.
 
gcc/testsuite/ChangeLog:
 
        * gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c: New test.
---
gcc/config/riscv/riscv.cc                             |  4 ++++
.../gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c  | 11 +++++++++++
2 files changed, 15 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 6eb63a9d4de..73454f65086 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7362,6 +7362,10 @@ riscv_regmode_natural_size (machine_mode mode)
      anything smaller than that.  */
   /* ??? For now, only do this for variable-width RVV registers.
      Doing it for constant-sized registers breaks lower-subreg.c.  */
+
+  if (GET_MODE_CLASS (mode) == MODE_VECTOR_BOOL)
+    return BYTES_PER_RISCV_VECTOR;
+
   if (!riscv_vector_chunks.is_constant () && riscv_v_ext_mode_p (mode))
     {
       if (riscv_v_ext_tuple_mode_p (mode))
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c
new file mode 100644
index 00000000000..534d5fe0f0a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d --param=riscv-autovec-preference=fixed-vlmax -O3" } */
+
+#include "riscv_vector.h"
+
+void test_vreinterpret_v_b64_i8m1 (uint8_t *in, int8_t *out)
+{
+  vbool64_t vmask = __riscv_vlm_v_b64 (in, 2);
+  vint8m1_t vout = __riscv_vreinterpret_v_b64_i8m1 (vmask);
+  __riscv_vse8_v_i8m1(out, vout, 16);
+}
-- 
2.17.1
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 6eb63a9d4de..73454f65086 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7362,6 +7362,10 @@  riscv_regmode_natural_size (machine_mode mode)
      anything smaller than that.  */
   /* ??? For now, only do this for variable-width RVV registers.
      Doing it for constant-sized registers breaks lower-subreg.c.  */
+
+  if (GET_MODE_CLASS (mode) == MODE_VECTOR_BOOL)
+    return BYTES_PER_RISCV_VECTOR;
+
   if (!riscv_vector_chunks.is_constant () && riscv_v_ext_mode_p (mode))
     {
       if (riscv_v_ext_tuple_mode_p (mode))
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c
new file mode 100644
index 00000000000..534d5fe0f0a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vreinterpet-fixed.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d --param=riscv-autovec-preference=fixed-vlmax -O3" } */
+
+#include "riscv_vector.h"
+
+void test_vreinterpret_v_b64_i8m1 (uint8_t *in, int8_t *out)
+{
+  vbool64_t vmask = __riscv_vlm_v_b64 (in, 2);
+  vint8m1_t vout = __riscv_vreinterpret_v_b64_i8m1 (vmask);
+  __riscv_vse8_v_i8m1(out, vout, 16);
+}