RE: Re: [PATCH] RISC-V: Disallow RVV mode address for any load/store[PR112535]
Committed, thanks Kito.
Pan
-Original Message-
From: Kito Cheng
Sent: Wednesday, November 15, 2023 3:36 PM
To: juzhe.zh...@rivai.ai
Cc: Kito.cheng ; gcc-patches ;
jeffreyalaw ; Robin Dapp
Subject: Re: Re: [PATCH] RISC-V: Disallow RVV mode address for any
load/store[PR112535]
LGTM, and yeah, I agree that's a code model issue.
On Wed, Nov 15, 2023 at 3:29 PM juzhe.zh...@rivai.ai
wrote:
>
> Yes.
> (set (reg:DI) (subreg:DI (reg:V1DI 155) 0)) become vmv.x.s
>
> So, you will see:
> vsetivli zero,1,e64,m1,ta,ma
> sb zero,%lo(c)(a2)
> vle64.v v1,0(a5)
> lbu a5,%lo(b)(a4)
> vse64.v v1,0(a3)
> beq a5,zero,.L6
> vmv.x.s a5,v1
> sw zero,0(a5)
>
> I think the codegen is not good. It should be using scalar load/store.
> I think it should be COST MODEL issue.
>
>
> juzhe.zh...@rivai.ai
>
>
> From: Kito Cheng
> Date: 2023-11-15 15:24
> To: Juzhe-Zhong
> CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> Subject: Re: [PATCH] RISC-V: Disallow RVV mode address for any
> load/store[PR112535]
> Curious about the code gen impact, does it make IRA/LRA insert one more move
> like (set (reg:DI) (subreg:DI (reg:V1DI 155) 0)) and then (set (mem:SI
> (reg:DI)) (const_int 0))?
>
> On Wed, Nov 15, 2023 at 3:15 PM Juzhe-Zhong wrote:
>>
>> This patch is quite obvious patch which disallow for load/store address
>> register
>> with RVV mode.
>>
>> PR target/112535
>>
>> gcc/ChangeLog:
>>
>> * config/riscv/riscv.cc (riscv_legitimate_address_p): Disallow RVV
>> modes base address.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/rvv/autovec/pr112535.c: New test.
>>
>> ---
>> gcc/config/riscv/riscv.cc | 4
>> .../gcc.target/riscv/rvv/autovec/pr112535.c | 17 +
>> 2 files changed, 21 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>>
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index ecee7eb4727..e919850fc6c 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -1427,6 +1427,10 @@ static bool
>> riscv_legitimate_address_p (machine_mode mode, rtx x, bool strict_p,
>> code_helper = ERROR_MARK)
>> {
>> + /* Disallow RVV modes base address.
>> + E.g. (mem:SI (subreg:DI (reg:V1DI 155) 0). */
>> + if (SUBREG_P (x) && riscv_v_ext_mode_p (GET_MODE (SUBREG_REG (x
>> +return false;
>>struct riscv_address_info addr;
>>
>>return riscv_classify_address (, x, mode, strict_p);
>> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>> b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>> new file mode 100644
>> index 000..95799aab8d2
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>> @@ -0,0 +1,17 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
>> +
>> +int *a, *f;
>> +char b, c;
>> +int ***d;
>> +static int e =
>> +void g() {
>> + c = 3;
>> + for (; c; c--)
>> +if (c < 8) {
>> + f = 0;
>> + ***e = a;
>> +}
>> + if (b)
>> +***d = 0;
>> +}
>> --
>> 2.36.3
>>
Re: Re: [PATCH] RISC-V: Disallow RVV mode address for any load/store[PR112535]
LGTM, and yeah, I agree that's a code model issue.
On Wed, Nov 15, 2023 at 3:29 PM juzhe.zh...@rivai.ai
wrote:
>
> Yes.
> (set (reg:DI) (subreg:DI (reg:V1DI 155) 0)) become vmv.x.s
>
> So, you will see:
> vsetivli zero,1,e64,m1,ta,ma
> sb zero,%lo(c)(a2)
> vle64.v v1,0(a5)
> lbu a5,%lo(b)(a4)
> vse64.v v1,0(a3)
> beq a5,zero,.L6
> vmv.x.s a5,v1
> sw zero,0(a5)
>
> I think the codegen is not good. It should be using scalar load/store.
> I think it should be COST MODEL issue.
>
>
> juzhe.zh...@rivai.ai
>
>
> From: Kito Cheng
> Date: 2023-11-15 15:24
> To: Juzhe-Zhong
> CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> Subject: Re: [PATCH] RISC-V: Disallow RVV mode address for any
> load/store[PR112535]
> Curious about the code gen impact, does it make IRA/LRA insert one more move
> like (set (reg:DI) (subreg:DI (reg:V1DI 155) 0)) and then (set (mem:SI
> (reg:DI)) (const_int 0))?
>
> On Wed, Nov 15, 2023 at 3:15 PM Juzhe-Zhong wrote:
>>
>> This patch is quite obvious patch which disallow for load/store address
>> register
>> with RVV mode.
>>
>> PR target/112535
>>
>> gcc/ChangeLog:
>>
>> * config/riscv/riscv.cc (riscv_legitimate_address_p): Disallow RVV
>> modes base address.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/rvv/autovec/pr112535.c: New test.
>>
>> ---
>> gcc/config/riscv/riscv.cc | 4
>> .../gcc.target/riscv/rvv/autovec/pr112535.c | 17 +
>> 2 files changed, 21 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>>
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index ecee7eb4727..e919850fc6c 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -1427,6 +1427,10 @@ static bool
>> riscv_legitimate_address_p (machine_mode mode, rtx x, bool strict_p,
>> code_helper = ERROR_MARK)
>> {
>> + /* Disallow RVV modes base address.
>> + E.g. (mem:SI (subreg:DI (reg:V1DI 155) 0). */
>> + if (SUBREG_P (x) && riscv_v_ext_mode_p (GET_MODE (SUBREG_REG (x
>> +return false;
>>struct riscv_address_info addr;
>>
>>return riscv_classify_address (, x, mode, strict_p);
>> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>> b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>> new file mode 100644
>> index 000..95799aab8d2
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
>> @@ -0,0 +1,17 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
>> +
>> +int *a, *f;
>> +char b, c;
>> +int ***d;
>> +static int e =
>> +void g() {
>> + c = 3;
>> + for (; c; c--)
>> +if (c < 8) {
>> + f = 0;
>> + ***e = a;
>> +}
>> + if (b)
>> +***d = 0;
>> +}
>> --
>> 2.36.3
>>
Re: Re: [PATCH] RISC-V: Disallow RVV mode address for any load/store[PR112535]
Yes.
(set (reg:DI) (subreg:DI (reg:V1DI 155) 0)) become vmv.x.s
So, you will see:
vsetivli zero,1,e64,m1,ta,ma
sb zero,%lo(c)(a2)
vle64.v v1,0(a5)
lbu a5,%lo(b)(a4)
vse64.v v1,0(a3)
beq a5,zero,.L6
vmv.x.s a5,v1
sw zero,0(a5)
I think the codegen is not good. It should be using scalar load/store.
I think it should be COST MODEL issue.
juzhe.zh...@rivai.ai
From: Kito Cheng
Date: 2023-11-15 15:24
To: Juzhe-Zhong
CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
Subject: Re: [PATCH] RISC-V: Disallow RVV mode address for any
load/store[PR112535]
Curious about the code gen impact, does it make IRA/LRA insert one more move
like (set (reg:DI) (subreg:DI (reg:V1DI 155) 0)) and then (set (mem:SI
(reg:DI)) (const_int 0))?
On Wed, Nov 15, 2023 at 3:15 PM Juzhe-Zhong wrote:
This patch is quite obvious patch which disallow for load/store address register
with RVV mode.
PR target/112535
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_legitimate_address_p): Disallow RVV
modes base address.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/autovec/pr112535.c: New test.
---
gcc/config/riscv/riscv.cc | 4
.../gcc.target/riscv/rvv/autovec/pr112535.c | 17 +
2 files changed, 21 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index ecee7eb4727..e919850fc6c 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -1427,6 +1427,10 @@ static bool
riscv_legitimate_address_p (machine_mode mode, rtx x, bool strict_p,
code_helper = ERROR_MARK)
{
+ /* Disallow RVV modes base address.
+ E.g. (mem:SI (subreg:DI (reg:V1DI 155) 0). */
+ if (SUBREG_P (x) && riscv_v_ext_mode_p (GET_MODE (SUBREG_REG (x
+return false;
struct riscv_address_info addr;
return riscv_classify_address (, x, mode, strict_p);
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
new file mode 100644
index 000..95799aab8d2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112535.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
+
+int *a, *f;
+char b, c;
+int ***d;
+static int e =
+void g() {
+ c = 3;
+ for (; c; c--)
+if (c < 8) {
+ f = 0;
+ ***e = a;
+}
+ if (b)
+***d = 0;
+}
--
2.36.3
