diff mbox series

RISC-V: Disallow RVV mode address for any load/store[PR112535]

Message ID 20231115071508.3273813-1-juzhe.zhong@rivai.ai
State Accepted
Headers
Series RISC-V: Disallow RVV mode address for any load/store[PR112535] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

juzhe.zhong@rivai.ai Nov. 15, 2023, 7:15 a.m. UTC 
  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

Comments

Kito Cheng Nov. 15, 2023, 7:24 a.m. UTC | #1 
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 <juzhe.zhong@rivai.ai> 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 (&addr, 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 00000000000..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 = &d;
> +void g() {
> +  c = 3;
> +  for (; c; c--)
> +    if (c < 8) {
> +      f = 0;
> +      ***e = a;
> +    }
> +  if (b)
> +    ***d = 0;
> +}
> --
> 2.36.3
>
>
juzhe.zhong@rivai.ai Nov. 15, 2023, 7:28 a.m. UTC | #2 
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.zhong@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 <juzhe.zhong@rivai.ai> 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 (&addr, 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 00000000000..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 = &d;
+void g() {
+  c = 3;
+  for (; c; c--)
+    if (c < 8) {
+      f = 0;
+      ***e = a;
+    }
+  if (b)
+    ***d = 0;
+}
-- 
2.36.3
Kito Cheng Nov. 15, 2023, 7:35 a.m. UTC | #3 
LGTM, and yeah, I agree that's a code model issue.

On Wed, Nov 15, 2023 at 3:29 PM juzhe.zhong@rivai.ai
<juzhe.zhong@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.zhong@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 <juzhe.zhong@rivai.ai> 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 (&addr, 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 00000000000..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 = &d;
>> +void g() {
>> +  c = 3;
>> +  for (; c; c--)
>> +    if (c < 8) {
>> +      f = 0;
>> +      ***e = a;
>> +    }
>> +  if (b)
>> +    ***d = 0;
>> +}
>> --
>> 2.36.3
>>
Li, Pan2 Nov. 15, 2023, 7:41 a.m. UTC | #4 
Committed, thanks Kito.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Wednesday, November 15, 2023 3:36 PM
To: juzhe.zhong@rivai.ai
Cc: Kito.cheng <kito.cheng@sifive.com>; gcc-patches <gcc-patches@gcc.gnu.org>; jeffreyalaw <jeffreyalaw@gmail.com>; Robin Dapp <rdapp.gcc@gmail.com>
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.zhong@rivai.ai
<juzhe.zhong@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.zhong@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 <juzhe.zhong@rivai.ai> 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 (&addr, 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 00000000000..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 = &d;
>> +void g() {
>> +  c = 3;
>> +  for (; c; c--)
>> +    if (c < 8) {
>> +      f = 0;
>> +      ***e = a;
>> +    }
>> +  if (b)
>> +    ***d = 0;
>> +}
>> --
>> 2.36.3
>>
diff mbox series

Patch

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 (&addr, 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 00000000000..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 = &d;
+void g() {
+  c = 3;
+  for (; c; c--)
+    if (c < 8) {
+      f = 0;
+      ***e = a;
+    }
+  if (b)
+    ***d = 0;
+}