public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Allow immediates 17-31 for vector shift.
@ 2023-08-18 19:37 Robin Dapp
  2023-08-18 19:56 ` Palmer Dabbelt
  2023-08-18 23:14 ` Jeff Law
  0 siblings, 2 replies; 4+ messages in thread
From: Robin Dapp @ 2023-08-18 19:37 UTC (permalink / raw)
  To: gcc-patches, palmer, Kito Cheng, jeffreyalaw, juzhe.zhong; +Cc: rdapp.gcc

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 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;
+    }
+}
-- 
2.41.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] RISC-V: Allow immediates 17-31 for vector shift.
  2023-08-18 19:37 [PATCH] RISC-V: Allow immediates 17-31 for vector shift Robin Dapp
@ 2023-08-18 19:56 ` Palmer Dabbelt
  2023-08-18 23:13   ` Jeff Law
  2023-08-18 23:14 ` Jeff Law
  1 sibling, 1 reply; 4+ messages in thread
From: Palmer Dabbelt @ 2023-08-18 19:56 UTC (permalink / raw)
  To: rdapp.gcc; +Cc: gcc-patches, Kito Cheng, jeffreyalaw, juzhe.zhong, rdapp.gcc

On Fri, 18 Aug 2023 12:37:06 PDT (-0700), rdapp.gcc@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 <palmer@rivosinc.com>

Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] RISC-V: Allow immediates 17-31 for vector shift.
  2023-08-18 19:56 ` Palmer Dabbelt
@ 2023-08-18 23:13   ` Jeff Law
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2023-08-18 23:13 UTC (permalink / raw)
  To: Palmer Dabbelt, rdapp.gcc; +Cc: gcc-patches, Kito Cheng, juzhe.zhong



On 8/18/23 13:56, Palmer Dabbelt wrote:
> On Fri, 18 Aug 2023 12:37:06 PDT (-0700), rdapp.gcc@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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] RISC-V: Allow immediates 17-31 for vector shift.
  2023-08-18 19:37 [PATCH] RISC-V: Allow immediates 17-31 for vector shift Robin Dapp
  2023-08-18 19:56 ` Palmer Dabbelt
@ 2023-08-18 23:14 ` Jeff Law
  1 sibling, 0 replies; 4+ messages in thread
From: Jeff Law @ 2023-08-18 23:14 UTC (permalink / raw)
  To: Robin Dapp, gcc-patches, palmer, Kito Cheng, juzhe.zhong



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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-08-18 23:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-18 19:37 [PATCH] RISC-V: Allow immediates 17-31 for vector shift Robin Dapp
2023-08-18 19:56 ` Palmer Dabbelt
2023-08-18 23:13   ` Jeff Law
2023-08-18 23:14 ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).