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

2023-08-18 Thread Jeff Law via Gcc-patches 




On 8/18/23 13:37, Robin Dapp 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.

OK
jeff

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

2023-08-18 Thread Jeff Law via Gcc-patches 




On 8/18/23 13:56, Palmer Dabbelt wrote:

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?
I think it's been out of date on every port I've ever worked on! Folks 
start with good intentions, but nobody seems to keep this up-to-date.


So +1 to removing it if someone wants to. I'll even pre-approve such a 
patch.  I'll also pre-approve a patch which brings it up-to-date if 
that's the direction we want to go.


jeff

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

2023-08-18 Thread 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 000..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 

Thanks!

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

2023-08-18 Thread Robin Dapp via Gcc-patches 
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)
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 000..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;
+}
+}
-- 
2.41.0