Re: [PATCH] RISC-V: Allow immediates 17-31 for vector shift.

[Palmer Dabbelt]

On Fri, 18 Aug 2023 12:37:06 PDT (-0700), rdapp....@gmail.com wrote:
Hi,
this patch adds a missing constraint check in order to be able to
print (and not ICE) vector immediates 17-31 for vector shifts.

Regards
 Robin

gcc/ChangeLog:

        * config/riscv/riscv.cc (riscv_print_operand):

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/rvv/autovec/binop/shift-immediate.c: New test.
---
 gcc/config/riscv/riscv.cc                        |  3 ++-
 .../riscv/rvv/autovec/binop/shift-immediate.c    | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 
gcc/testsuite/gcc.target/riscv/rvv/autovec/binop/shift-immediate.c

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 49062bef9fc..0f60ffe5f60 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -4954,7 +4954,8 @@ riscv_print_operand (FILE *file, rtx op, int letter)

Looks like the comment at the top of riscv_print_operand() is way out of 
date.  Maybe we should just toss it?

            else if (satisfies_constraint_Wc0 (op))
              asm_fprintf (file, "0");
            else if (satisfies_constraint_vi (op)
-                    || satisfies_constraint_vj (op))
+                    || satisfies_constraint_vj (op)
+                    || satisfies_constraint_vk (op))
              asm_fprintf (file, "%wd", INTVAL (elt));
            else
              output_operand_lossage ("invalid vector constant");
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/binop/shift-immediate.c 
b/gcc/testsuite/gcc.target/riscv/rvv/autovec/binop/shift-immediate.c
new file mode 100644
index 00000000000..a2e1c33f4fa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/binop/shift-immediate.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-std=c99 -march=rv32gcv -mabi=ilp32d -O2 
--param=riscv-autovec-preference=scalable" } */
+
+#define uint8_t unsigned char
+
+void foo1 (uint8_t *a)
+{
+    uint8_t b = a[0];
+    int val = 0;
+
+    for (int i = 0; i < 4; i++)
+    {
+        a[i] = (val & 1) ? (-val) >> 17 : val;
+        val += b;
+    }
+}


Unless I'm missing something it looks like we're missing at least Wc1 as 
well, and maybe a few others?

Either way

Reviewed-by: Palmer Dabbelt <pal...@rivosinc.com>

Thanks!