diff mbox series

[v1] RISC-V: Fix one ICE for vect test vect-multitypes-5

Message ID 20230829104921.4117031-1-pan2.li@intel.com
State Accepted
Headers
Series [v1] RISC-V: Fix one ICE for vect test vect-multitypes-5 |

Checks

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

Commit Message

Li, Pan2 via Gcc-patches Aug. 29, 2023, 10:49 a.m. UTC 
  From: Pan Li <pan2.li@intel.com>

There will be one ICE when build vect-multitypes-5.c similar as below:

riscv64-unknown-elf-gcc -O3 \
  -march=rv64imafdcv -mabi=lp64d -mcmodel=medlow \
  -fdiagnostics-plain-output -flto -ffat-lto-objects \
  --param riscv-autovec-preference=scalable -Wno-psabi \
  -ftree-vectorize -fno-tree-loop-distribute-patterns \
  -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details \
  gcc/testsuite/gcc.dg/vect/vect-multitypes-5.c -o test.elf -lm

The below RTL is not well handled in riscv_legitimize_const_move, and
then fall through to the default pass. Then the
default force_const_mem will NULL_RTX, and will have ICE when operating
one the NULL_RTX.

(const:DI
  (plus:DI
    (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>)
    (const_poly_int:DI [16, 16])))

This patch would like to take care of this rtl in riscv_legitimize_const_move.

Signed-off-by: Pan Li <pan2.li@intel.com>
Co-Authored-By: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_legitimize_poly_move): New declaration.
	(riscv_legitimize_const_move): Handle ref plus const poly.
---
 gcc/config/riscv/riscv.cc | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Kito Cheng Aug. 29, 2023, 1:46 p.m. UTC | #1 
LGTM, thanks :)

On Tue, Aug 29, 2023 at 6:50 PM Pan Li via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> There will be one ICE when build vect-multitypes-5.c similar as below:
>
> riscv64-unknown-elf-gcc -O3 \
>   -march=rv64imafdcv -mabi=lp64d -mcmodel=medlow \
>   -fdiagnostics-plain-output -flto -ffat-lto-objects \
>   --param riscv-autovec-preference=scalable -Wno-psabi \
>   -ftree-vectorize -fno-tree-loop-distribute-patterns \
>   -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details \
>   gcc/testsuite/gcc.dg/vect/vect-multitypes-5.c -o test.elf -lm
>
> The below RTL is not well handled in riscv_legitimize_const_move, and
> then fall through to the default pass. Then the
> default force_const_mem will NULL_RTX, and will have ICE when operating
> one the NULL_RTX.
>
> (const:DI
>   (plus:DI
>     (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>)
>     (const_poly_int:DI [16, 16])))
>
> This patch would like to take care of this rtl in riscv_legitimize_const_move.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_legitimize_poly_move): New declaration.
>         (riscv_legitimize_const_move): Handle ref plus const poly.
> ---
>  gcc/config/riscv/riscv.cc | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 1d6e278ea90..bab6ed70b2d 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -366,6 +366,7 @@ static const struct riscv_tune_param optimize_size_tune_info = {
>
>  static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
>  static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
> +static void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
>
>  /* Defining target-specific uses of __attribute__.  */
>  static const struct attribute_spec riscv_attribute_table[] =
> @@ -2118,6 +2119,28 @@ riscv_legitimize_const_move (machine_mode mode, rtx dest, rtx src)
>        return;
>      }
>
> +  /* Handle below format.
> +     (const:DI
> +       (plus:DI
> +        (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>) <- op_0
> +        (const_poly_int:DI [16, 16]) // <- op_1
> +     ))
> +   */
> +  rtx src_op_0 = XEXP (src, 0);
> +
> +  if (GET_CODE (src) == CONST && GET_CODE (src_op_0) == PLUS
> +    && CONST_POLY_INT_P (XEXP (src_op_0, 1)))
> +    {
> +      rtx dest_tmp = gen_reg_rtx (mode);
> +      rtx tmp = gen_reg_rtx (mode);
> +
> +      riscv_emit_move (dest, XEXP (src_op_0, 0));
> +      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (src_op_0, 1));
> +
> +      emit_insn (gen_rtx_SET (dest, gen_rtx_PLUS (mode, dest, dest_tmp)));
> +      return;
> +    }
> +
>    src = force_const_mem (mode, src);
>
>    /* When using explicit relocs, constant pool references are sometimes
> --
> 2.34.1
>
Li, Pan2 via Gcc-patches Aug. 29, 2023, 2:40 p.m. UTC | #2 
Committed, thanks Kito.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Tuesday, August 29, 2023 9:46 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; Wang, Yanzhang <yanzhang.wang@intel.com>; juzhe.zhong@rivai.ai
Subject: Re: [PATCH v1] RISC-V: Fix one ICE for vect test vect-multitypes-5

LGTM, thanks :)

On Tue, Aug 29, 2023 at 6:50 PM Pan Li via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> There will be one ICE when build vect-multitypes-5.c similar as below:
>
> riscv64-unknown-elf-gcc -O3 \
>   -march=rv64imafdcv -mabi=lp64d -mcmodel=medlow \
>   -fdiagnostics-plain-output -flto -ffat-lto-objects \
>   --param riscv-autovec-preference=scalable -Wno-psabi \
>   -ftree-vectorize -fno-tree-loop-distribute-patterns \
>   -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details \
>   gcc/testsuite/gcc.dg/vect/vect-multitypes-5.c -o test.elf -lm
>
> The below RTL is not well handled in riscv_legitimize_const_move, and
> then fall through to the default pass. Then the
> default force_const_mem will NULL_RTX, and will have ICE when operating
> one the NULL_RTX.
>
> (const:DI
>   (plus:DI
>     (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>)
>     (const_poly_int:DI [16, 16])))
>
> This patch would like to take care of this rtl in riscv_legitimize_const_move.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored-By: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_legitimize_poly_move): New declaration.
>         (riscv_legitimize_const_move): Handle ref plus const poly.
> ---
>  gcc/config/riscv/riscv.cc | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 1d6e278ea90..bab6ed70b2d 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -366,6 +366,7 @@ static const struct riscv_tune_param optimize_size_tune_info = {
>
>  static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
>  static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
> +static void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
>
>  /* Defining target-specific uses of __attribute__.  */
>  static const struct attribute_spec riscv_attribute_table[] =
> @@ -2118,6 +2119,28 @@ riscv_legitimize_const_move (machine_mode mode, rtx dest, rtx src)
>        return;
>      }
>
> +  /* Handle below format.
> +     (const:DI
> +       (plus:DI
> +        (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>) <- op_0
> +        (const_poly_int:DI [16, 16]) // <- op_1
> +     ))
> +   */
> +  rtx src_op_0 = XEXP (src, 0);
> +
> +  if (GET_CODE (src) == CONST && GET_CODE (src_op_0) == PLUS
> +    && CONST_POLY_INT_P (XEXP (src_op_0, 1)))
> +    {
> +      rtx dest_tmp = gen_reg_rtx (mode);
> +      rtx tmp = gen_reg_rtx (mode);
> +
> +      riscv_emit_move (dest, XEXP (src_op_0, 0));
> +      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (src_op_0, 1));
> +
> +      emit_insn (gen_rtx_SET (dest, gen_rtx_PLUS (mode, dest, dest_tmp)));
> +      return;
> +    }
> +
>    src = force_const_mem (mode, src);
>
>    /* When using explicit relocs, constant pool references are sometimes
> --
> 2.34.1
>
Patrick O'Neill Sept. 18, 2023, 5:38 p.m. UTC | #3 
Hi,

After this patch, there is now an ICE when bootstrapping with
--enable-checking=rtl on rv32gc.

More details:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111461

Thanks,
Patrick

On 8/29/23 07:40, Li, Pan2 via Gcc-patches wrote:
> Committed, thanks Kito.
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Tuesday, August 29, 2023 9:46 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: gcc-patches@gcc.gnu.org; Wang, Yanzhang <yanzhang.wang@intel.com>; juzhe.zhong@rivai.ai
> Subject: Re: [PATCH v1] RISC-V: Fix one ICE for vect test vect-multitypes-5
>
> LGTM, thanks :)
>
> On Tue, Aug 29, 2023 at 6:50 PM Pan Li via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> From: Pan Li <pan2.li@intel.com>
>>
>> There will be one ICE when build vect-multitypes-5.c similar as below:
>>
>> riscv64-unknown-elf-gcc -O3 \
>>    -march=rv64imafdcv -mabi=lp64d -mcmodel=medlow \
>>    -fdiagnostics-plain-output -flto -ffat-lto-objects \
>>    --param riscv-autovec-preference=scalable -Wno-psabi \
>>    -ftree-vectorize -fno-tree-loop-distribute-patterns \
>>    -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details \
>>    gcc/testsuite/gcc.dg/vect/vect-multitypes-5.c -o test.elf -lm
>>
>> The below RTL is not well handled in riscv_legitimize_const_move, and
>> then fall through to the default pass. Then the
>> default force_const_mem will NULL_RTX, and will have ICE when operating
>> one the NULL_RTX.
>>
>> (const:DI
>>    (plus:DI
>>      (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>)
>>      (const_poly_int:DI [16, 16])))
>>
>> This patch would like to take care of this rtl in riscv_legitimize_const_move.
>>
>> Signed-off-by: Pan Li <pan2.li@intel.com>
>> Co-Authored-By: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>>
>> gcc/ChangeLog:
>>
>>          * config/riscv/riscv.cc (riscv_legitimize_poly_move): New declaration.
>>          (riscv_legitimize_const_move): Handle ref plus const poly.
>> ---
>>   gcc/config/riscv/riscv.cc | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index 1d6e278ea90..bab6ed70b2d 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -366,6 +366,7 @@ static const struct riscv_tune_param optimize_size_tune_info = {
>>
>>   static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
>>   static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
>> +static void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
>>
>>   /* Defining target-specific uses of __attribute__.  */
>>   static const struct attribute_spec riscv_attribute_table[] =
>> @@ -2118,6 +2119,28 @@ riscv_legitimize_const_move (machine_mode mode, rtx dest, rtx src)
>>         return;
>>       }
>>
>> +  /* Handle below format.
>> +     (const:DI
>> +       (plus:DI
>> +        (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>) <- op_0
>> +        (const_poly_int:DI [16, 16]) // <- op_1
>> +     ))
>> +   */
>> +  rtx src_op_0 = XEXP (src, 0);
>> +
>> +  if (GET_CODE (src) == CONST && GET_CODE (src_op_0) == PLUS
>> +    && CONST_POLY_INT_P (XEXP (src_op_0, 1)))
>> +    {
>> +      rtx dest_tmp = gen_reg_rtx (mode);
>> +      rtx tmp = gen_reg_rtx (mode);
>> +
>> +      riscv_emit_move (dest, XEXP (src_op_0, 0));
>> +      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (src_op_0, 1));
>> +
>> +      emit_insn (gen_rtx_SET (dest, gen_rtx_PLUS (mode, dest, dest_tmp)));
>> +      return;
>> +    }
>> +
>>     src = force_const_mem (mode, src);
>>
>>     /* When using explicit relocs, constant pool references are sometimes
>> --
>> 2.34.1
>>
juzhe.zhong@rivai.ai Sept. 19, 2023, 7:54 a.m. UTC | #4 
Thanks for reporting it.

Could you try this and verify for me?

-  rtx src_op_0 = XEXP (src, 0);
-
-  if (GET_CODE (src) == CONST && GET_CODE (src_op_0) == PLUS
-    && CONST_POLY_INT_P (XEXP (src_op_0, 1)))
+  if (GET_CODE (src) == CONST && GET_CODE (XEXP (src, 0)) == PLUS
+    && CONST_POLY_INT_P (XEXP (XEXP (src, 0), 1)))
     {
       rtx dest_tmp = gen_reg_rtx (mode);
       rtx tmp = gen_reg_rtx (mode);

-      riscv_emit_move (dest, XEXP (src_op_0, 0));
-      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (src_op_0, 1));
+      riscv_emit_move (dest, XEXP (XEXP (src, 0), 0));
+      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (XEXP (src, 0), 1));

If it can fix your issue, plz send a patch and commit it.

Thanks.



juzhe.zhong@rivai.ai
 
From: Patrick O'Neill
Date: 2023-09-19 01:38
To: Li, Pan2; Kito Cheng
CC: gcc-patches@gcc.gnu.org; Wang, Yanzhang; juzhe.zhong@rivai.ai; Palmer Dabbelt
Subject: Re: [PATCH v1] RISC-V: Fix one ICE for vect test vect-multitypes-5
Hi,
 
After this patch, there is now an ICE when bootstrapping with
--enable-checking=rtl on rv32gc.
 
More details:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111461
 
Thanks,
Patrick
 
On 8/29/23 07:40, Li, Pan2 via Gcc-patches wrote:
> Committed, thanks Kito.
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Tuesday, August 29, 2023 9:46 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: gcc-patches@gcc.gnu.org; Wang, Yanzhang <yanzhang.wang@intel.com>; juzhe.zhong@rivai.ai
> Subject: Re: [PATCH v1] RISC-V: Fix one ICE for vect test vect-multitypes-5
>
> LGTM, thanks :)
>
> On Tue, Aug 29, 2023 at 6:50 PM Pan Li via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> From: Pan Li <pan2.li@intel.com>
>>
>> There will be one ICE when build vect-multitypes-5.c similar as below:
>>
>> riscv64-unknown-elf-gcc -O3 \
>>    -march=rv64imafdcv -mabi=lp64d -mcmodel=medlow \
>>    -fdiagnostics-plain-output -flto -ffat-lto-objects \
>>    --param riscv-autovec-preference=scalable -Wno-psabi \
>>    -ftree-vectorize -fno-tree-loop-distribute-patterns \
>>    -fno-vect-cost-model -fno-common -O2 -fdump-tree-vect-details \
>>    gcc/testsuite/gcc.dg/vect/vect-multitypes-5.c -o test.elf -lm
>>
>> The below RTL is not well handled in riscv_legitimize_const_move, and
>> then fall through to the default pass. Then the
>> default force_const_mem will NULL_RTX, and will have ICE when operating
>> one the NULL_RTX.
>>
>> (const:DI
>>    (plus:DI
>>      (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>)
>>      (const_poly_int:DI [16, 16])))
>>
>> This patch would like to take care of this rtl in riscv_legitimize_const_move.
>>
>> Signed-off-by: Pan Li <pan2.li@intel.com>
>> Co-Authored-By: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>>
>> gcc/ChangeLog:
>>
>>          * config/riscv/riscv.cc (riscv_legitimize_poly_move): New declaration.
>>          (riscv_legitimize_const_move): Handle ref plus const poly.
>> ---
>>   gcc/config/riscv/riscv.cc | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index 1d6e278ea90..bab6ed70b2d 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -366,6 +366,7 @@ static const struct riscv_tune_param optimize_size_tune_info = {
>>
>>   static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
>>   static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
>> +static void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
>>
>>   /* Defining target-specific uses of __attribute__.  */
>>   static const struct attribute_spec riscv_attribute_table[] =
>> @@ -2118,6 +2119,28 @@ riscv_legitimize_const_move (machine_mode mode, rtx dest, rtx src)
>>         return;
>>       }
>>
>> +  /* Handle below format.
>> +     (const:DI
>> +       (plus:DI
>> +        (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>) <- op_0
>> +        (const_poly_int:DI [16, 16]) // <- op_1
>> +     ))
>> +   */
>> +  rtx src_op_0 = XEXP (src, 0);
>> +
>> +  if (GET_CODE (src) == CONST && GET_CODE (src_op_0) == PLUS
>> +    && CONST_POLY_INT_P (XEXP (src_op_0, 1)))
>> +    {
>> +      rtx dest_tmp = gen_reg_rtx (mode);
>> +      rtx tmp = gen_reg_rtx (mode);
>> +
>> +      riscv_emit_move (dest, XEXP (src_op_0, 0));
>> +      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (src_op_0, 1));
>> +
>> +      emit_insn (gen_rtx_SET (dest, gen_rtx_PLUS (mode, dest, dest_tmp)));
>> +      return;
>> +    }
>> +
>>     src = force_const_mem (mode, src);
>>
>>     /* When using explicit relocs, constant pool references are sometimes
>> --
>> 2.34.1
>>
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 1d6e278ea90..bab6ed70b2d 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -366,6 +366,7 @@  static const struct riscv_tune_param optimize_size_tune_info = {
 
 static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
 static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
+static void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
 
 /* Defining target-specific uses of __attribute__.  */
 static const struct attribute_spec riscv_attribute_table[] =
@@ -2118,6 +2119,28 @@  riscv_legitimize_const_move (machine_mode mode, rtx dest, rtx src)
       return;
     }
 
+  /* Handle below format.
+     (const:DI
+       (plus:DI
+	 (symbol_ref:DI ("ic") [flags 0x2] <var_decl 0x7fe57740be10 ic>) <- op_0
+	 (const_poly_int:DI [16, 16]) // <- op_1
+     ))
+   */
+  rtx src_op_0 = XEXP (src, 0);
+
+  if (GET_CODE (src) == CONST && GET_CODE (src_op_0) == PLUS
+    && CONST_POLY_INT_P (XEXP (src_op_0, 1)))
+    {
+      rtx dest_tmp = gen_reg_rtx (mode);
+      rtx tmp = gen_reg_rtx (mode);
+
+      riscv_emit_move (dest, XEXP (src_op_0, 0));
+      riscv_legitimize_poly_move (mode, dest_tmp, tmp, XEXP (src_op_0, 1));
+
+      emit_insn (gen_rtx_SET (dest, gen_rtx_PLUS (mode, dest, dest_tmp)));
+      return;
+    }
+
   src = force_const_mem (mode, src);
 
   /* When using explicit relocs, constant pool references are sometimes