Re: Re: [PATCH] RISC-V: Fix V_WHOLE && V_FRACT iterator requirement

[钟居哲 <juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 7722 bytes --]

Ok. If you have done this plz go ahead.
I think it shouldn't be with vec_set patch.
Instead, it obviously should be the separate patch.



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-06-09 22:37
To: juzhe.zhong; gcc-patches
CC: rdapp.gcc; kito.cheng; palmer; jeffreyalaw
Subject: Re: [PATCH] RISC-V: Fix V_WHOLE && V_FRACT iterator requirement
On 6/9/23 16:32, juzhe.zhong@rivai.ai wrote:
> From: Juzhe-Zhong <juzhe.zhong@rivai.ai>
> 
> This patch fixes the requirement of V_WHOLE and V_FRACT.
> E.g. VNx8QI in V_WHOLE has no requirement which is incorrect.
>      Actually, VNx8QI should be whole(full) mode when TARGET_MIN_VLEN < 128
>      since when TARGET_MIN_VLEN == 128, VNx8QI is e8mf2 which is fractional
>      vector.
> 
> gcc/ChangeLog:
> 
>         * config/riscv/vector-iterators.md: Fix requirement.
 
I actually have the attached already on my local tree (as well as a test),
and wanted to post it with the vec_set patch.  I think the alignment helps
a bit with readability.
 
From 147a459dfbf1fe9d5dd93148f475f42dee3bd94b Mon Sep 17 00:00:00 2001
From: Robin Dapp <rdapp@ventanamicro.com>
Date: Tue, 6 Jun 2023 17:29:26 +0200
Subject: [PATCH] RISC-V: Change V_WHOLE iterator to properly match
instruction.
 
Currently we emit e.g. an vl1r.v even when loading a mode whose size is
smaller than the hardware vector size.  This can happen when reload
decides to switch to another alternative.
 
This patch fixes the iterator and adds a testcase for the problem.
 
gcc/ChangeLog:
 
* config/riscv/vector-iterators.md: Add guards for modes smaller
than the hardware vector size.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c: New test.
---
gcc/config/riscv/vector-iterators.md          | 65 ++++++++++++++-----
.../rvv/autovec/vls-vlmax/full-vec-move1.c    | 23 +++++++
2 files changed, 72 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c
 
diff --git a/gcc/config/riscv/vector-iterators.md b/gcc/config/riscv/vector-iterators.md
index 90743ed76c5..0587325e82c 100644
--- a/gcc/config/riscv/vector-iterators.md
+++ b/gcc/config/riscv/vector-iterators.md
@@ -430,32 +430,65 @@ (define_mode_iterator VNX64_QHI [
   VNx64QI (VNx64HI "TARGET_MIN_VLEN >= 128")
])
+;; This iterator describes which modes can be moved/loaded/stored by
+;; full-register move instructions (e.g. vl1r.v).
+;; For now we support a maximum vector length of 1024 that can
+;; also be reached by combining multiple hardware registers (mf1, mf2, ...).
+;; This means that e.g. VNx64HI (with a size of 128 bytes) requires
+;; at least a minimum vector length of 128 bits = 16 bytes in order
+;; to be loadable by vl8r.v (mf8).
+;; Apart from that we must make sure that modes smaller than the
+;; vector size are properly guarded so that e.g. VNx4HI is not loaded
+;; by vl1r.v when VL == 128.
(define_mode_iterator V_WHOLE [
-  (VNx4QI "TARGET_MIN_VLEN == 32") VNx8QI VNx16QI VNx32QI (VNx64QI "TARGET_MIN_VLEN > 32") (VNx128QI "TARGET_MIN_VLEN >= 128")
-  (VNx2HI "TARGET_MIN_VLEN == 32") VNx4HI VNx8HI VNx16HI (VNx32HI "TARGET_MIN_VLEN > 32") (VNx64HI "TARGET_MIN_VLEN >= 128")
-  (VNx1SI "TARGET_MIN_VLEN == 32") VNx2SI VNx4SI VNx8SI (VNx16SI "TARGET_MIN_VLEN > 32") (VNx32SI "TARGET_MIN_VLEN >= 128")
-  (VNx1DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN < 128") (VNx2DI "TARGET_VECTOR_ELEN_64")
-  (VNx4DI "TARGET_VECTOR_ELEN_64") (VNx8DI "TARGET_VECTOR_ELEN_64") (VNx16DI "TARGET_MIN_VLEN >= 128")
+  (VNx4QI "TARGET_MIN_VLEN == 32")
+  (VNx8QI "TARGET_MIN_VLEN <= 64")
+  (VNx16QI "TARGET_MIN_VLEN <= 128")
+  (VNx32QI "TARGET_MIN_VLEN <= 256")
+  (VNx64QI "TARGET_MIN_VLEN >= 64 && TARGET_MIN_VLEN <= 512")
+  (VNx128QI "TARGET_MIN_VLEN >= 128 && TARGET_MIN_VLEN <= 1024")
+  (VNx2HI "TARGET_MIN_VLEN == 32")
+  (VNx4HI "TARGET_MIN_VLEN <= 64")
+  (VNx8HI "TARGET_MIN_VLEN <= 128")
+  (VNx16HI "TARGET_MIN_VLEN <= 256")
+  (VNx32HI "TARGET_MIN_VLEN >= 64 && TARGET_MIN_VLEN <= 512")
+  (VNx64HI "TARGET_MIN_VLEN >= 128 && TARGET_MIN_VLEN <= 1024")
+  (VNx1SI "TARGET_MIN_VLEN == 32")
+  (VNx2SI "TARGET_MIN_VLEN <= 64")
+  (VNx4SI "TARGET_MIN_VLEN <= 128")
+  (VNx8SI "TARGET_MIN_VLEN <= 256")
+  (VNx16SI "TARGET_MIN_VLEN >= 64 && TARGET_MIN_VLEN <= 512")
+  (VNx32SI "TARGET_MIN_VLEN >= 128 && TARGET_MIN_VLEN <= 1024")
+  (VNx1DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN == 64")
+  (VNx2DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN <= 128")
+  (VNx4DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN <= 256")
+  (VNx8DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN <= 512")
+  (VNx16DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN >= 128
+            && TARGET_MIN_VLEN <= 1024")
   (VNx1HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN < 128")
   (VNx2HF "TARGET_VECTOR_ELEN_FP_16")
   (VNx4HF "TARGET_VECTOR_ELEN_FP_16")
   (VNx8HF "TARGET_VECTOR_ELEN_FP_16")
   (VNx16HF "TARGET_VECTOR_ELEN_FP_16")
-  (VNx32HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN > 32")
+  (VNx32HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN >= 64")
   (VNx64HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN >= 128")
   (VNx1SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN == 32")
-  (VNx2SF "TARGET_VECTOR_ELEN_FP_32")
-  (VNx4SF "TARGET_VECTOR_ELEN_FP_32")
-  (VNx8SF "TARGET_VECTOR_ELEN_FP_32")
-  (VNx16SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN > 32")
-  (VNx32SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN >= 128")
-  (VNx1DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN < 128")
-  (VNx2DF "TARGET_VECTOR_ELEN_FP_64")
-  (VNx4DF "TARGET_VECTOR_ELEN_FP_64")
-  (VNx8DF "TARGET_VECTOR_ELEN_FP_64")
-  (VNx16DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN >= 128")
+  (VNx2SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN <= 64")
+  (VNx4SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN <= 128")
+  (VNx8SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN <= 256")
+  (VNx16SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN >= 64
+            && TARGET_MIN_VLEN <= 512")
+  (VNx32SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN >= 128
+            && TARGET_MIN_VLEN <= 1024")
+  (VNx1DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN <= 64")
+  (VNx2DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN <= 128")
+  (VNx4DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN <= 256")
+  (VNx8DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN >= 64
+           && TARGET_MIN_VLEN <= 512")
+  (VNx16DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN >= 128
+            && TARGET_MIN_VLEN <= 1024")
])
(define_mode_iterator V_FRACT [
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c
new file mode 100644
index 00000000000..d63f219a1e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c
@@ -0,0 +1,23 @@
+/* { dg-do run { target { riscv_vector } } } */
+/* { dg-additional-options "-std=c99 -O3 -march=rv64gcv_zvl128b -fno-vect-cost-model --param=riscv-autovec-preference=fixed-vlmax" } */
+
+#include <stdint-gcc.h>
+#include <assert.h>
+
+/* This would cause us to emit a vl1r.v for VNx4HImode even when
+   the hardware vector size vl > 64.  */
+
+typedef int16_t V __attribute__((vector_size (128)));
+
+int main ()
+{
+  V v;
+  for (int i = 0; i < sizeof (v) / sizeof (v[0]); i++)
+    (v)[i] = i;
+  V res = v;
+  for (int i = 0; i < sizeof (v) / sizeof (v[0]); i++)
+    assert (res[i] == i);
+}
+
+/* { dg-final { scan-assembler-not {vl[1248]r.v} } }  */
+/* { dg-final { scan-assembler-times {vl[1248]re16.v} 1 } }  */
-- 
2.40.1