diff mbox series

genrecog: Add SUBREG_BYTE.to_constant check to the genrecog

Message ID 20230814094218.3286920-1-juzhe.zhong@rivai.ai
State Accepted
Headers
Series genrecog: Add SUBREG_BYTE.to_constant check to the genrecog |

Checks

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

Commit Message

juzhe.zhong@rivai.ai Aug. 14, 2023, 9:42 a.m. UTC 
  Hi, there is genrecog issue happens in RISC-V backend.

This is the ICE info:

0xfa3ba4 poly_int_pod<2u, unsigned short>::to_constant() const
        ../../../riscv-gcc/gcc/poly-int.h:504
0x28eaa91 recog_5
        ../../../riscv-gcc/gcc/config/riscv/bitmanip.md:314
0x28ec5b4 recog_7
        ../../../riscv-gcc/gcc/config/riscv/iterators.md:81
0x2a2e740 recog_436
        ../../../riscv-gcc/gcc/config/riscv/thead.md:265
0x2a729ef recog_475
        ../../../riscv-gcc/gcc/config/riscv/sync.md:509
0x2a75aec recog(rtx_def*, rtx_insn*, int*)
        ../../../riscv-gcc/gcc/config/riscv/iterators.md:55
0x2b3e39e recog_for_combine_1
        ../../../riscv-gcc/gcc/combine.cc:11382
0x2b3f457 recog_for_combine
        ../../../riscv-gcc/gcc/combine.cc:11652
0x2b25a15 try_combine
        ../../../riscv-gcc/gcc/combine.cc:4054
0x2b1d3f1 combine_instructions
        ../../../riscv-gcc/gcc/combine.cc:1266
0x2b48cfc rest_of_handle_combine
        ../../../riscv-gcc/gcc/combine.cc:15063
0x2b48db8 execute
        ../../../riscv-gcc/gcc/combine.cc:15107

This is because the genrecog code here cause ICE for scalable vector in RISC-V:

Before this patch:

static int
recog_5 (rtx x1 ATTRIBUTE_UNUSED,
        rtx_insn *insn ATTRIBUTE_UNUSED,
        int *pnum_clobbers ATTRIBUTE_UNUSED)
{
  rtx * const operands ATTRIBUTE_UNUSED = &recog_data.operand[0];
  rtx x2, x3, x4;
  int res ATTRIBUTE_UNUSED;
  if (pnum_clobbers == NULL)
    return -1;
  x2 = XEXP (x1, 1);
  x3 = XEXP (x2, 0);
  if (maybe_ne (SUBREG_BYTE (x3).to_constant (), 0) ---> this code cause ICE.
      || GET_MODE (x3) != E_SImode
      || !register_operand (operands[0], E_DImode)
      || GET_MODE (x2) != E_DImode)
    return -1;
...

This ICE happens since we have following RTL IR:

(insn 27 26 29 4 (set (reg:RVVM1HI 155 [ vect__12.23 ])
        (sign_extend:RVVM1HI (subreg:RVVMF2QI (reg:RVVMF2x2QI 146 [ vect_array.19 ]) [8, 8]))) "auto.c":29:1 discrim 2 12570 {extendrvvmf2qirvvm1hi2}
     (expr_list:REG_DEAD (reg:RVVMF2x2QI 146 [ vect_array.19 ])
        (nil)))

This is the scalable vector with SUBREG_BYTE = poly (8, 8)

After this patch:

static int
recog_5 (rtx x1 ATTRIBUTE_UNUSED,
        rtx_insn *insn ATTRIBUTE_UNUSED,
        int *pnum_clobbers ATTRIBUTE_UNUSED)
{
  rtx * const operands ATTRIBUTE_UNUSED = &recog_data.operand[0];
  rtx x2, x3, x4;
  int res ATTRIBUTE_UNUSED;
  if (pnum_clobbers == NULL)
    return -1;
  x2 = XEXP (x1, 1);
  x3 = XEXP (x2, 0);
  if ((SUBREG_BYTE (x3).is_constant () && maybe_ne (SUBREG_BYTE (x3).to_constant (), 0))   ----> change here and fix ICE.
      || GET_MODE (x3) != E_SImode
      || !register_operand (operands[0], E_DImode)
      || GET_MODE (x2) != E_DImode)
    return -1;

Does it reasonable ?

Thanks.

gcc/ChangeLog:

        * genrecog.cc (print_test): Add SUBREG_BYTE.to_constant () check.

---
 gcc/genrecog.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Richard Sandiford Aug. 14, 2023, 10 a.m. UTC | #1 
Juzhe-Zhong <juzhe.zhong@rivai.ai> writes:
> Hi, there is genrecog issue happens in RISC-V backend.
>
> This is the ICE info:
>
> 0xfa3ba4 poly_int_pod<2u, unsigned short>::to_constant() const
>         ../../../riscv-gcc/gcc/poly-int.h:504
> 0x28eaa91 recog_5
>         ../../../riscv-gcc/gcc/config/riscv/bitmanip.md:314
> 0x28ec5b4 recog_7
>         ../../../riscv-gcc/gcc/config/riscv/iterators.md:81
> 0x2a2e740 recog_436
>         ../../../riscv-gcc/gcc/config/riscv/thead.md:265
> 0x2a729ef recog_475
>         ../../../riscv-gcc/gcc/config/riscv/sync.md:509
> 0x2a75aec recog(rtx_def*, rtx_insn*, int*)
>         ../../../riscv-gcc/gcc/config/riscv/iterators.md:55
> 0x2b3e39e recog_for_combine_1
>         ../../../riscv-gcc/gcc/combine.cc:11382
> 0x2b3f457 recog_for_combine
>         ../../../riscv-gcc/gcc/combine.cc:11652
> 0x2b25a15 try_combine
>         ../../../riscv-gcc/gcc/combine.cc:4054
> 0x2b1d3f1 combine_instructions
>         ../../../riscv-gcc/gcc/combine.cc:1266
> 0x2b48cfc rest_of_handle_combine
>         ../../../riscv-gcc/gcc/combine.cc:15063
> 0x2b48db8 execute
>         ../../../riscv-gcc/gcc/combine.cc:15107
>
> This is because the genrecog code here cause ICE for scalable vector in RISC-V:
>
> Before this patch:
>
> static int
> recog_5 (rtx x1 ATTRIBUTE_UNUSED,
>         rtx_insn *insn ATTRIBUTE_UNUSED,
>         int *pnum_clobbers ATTRIBUTE_UNUSED)
> {
>   rtx * const operands ATTRIBUTE_UNUSED = &recog_data.operand[0];
>   rtx x2, x3, x4;
>   int res ATTRIBUTE_UNUSED;
>   if (pnum_clobbers == NULL)
>     return -1;
>   x2 = XEXP (x1, 1);
>   x3 = XEXP (x2, 0);
>   if (maybe_ne (SUBREG_BYTE (x3).to_constant (), 0) ---> this code cause ICE.
>       || GET_MODE (x3) != E_SImode
>       || !register_operand (operands[0], E_DImode)
>       || GET_MODE (x2) != E_DImode)
>     return -1;
> ...
>
> This ICE happens since we have following RTL IR:
>
> (insn 27 26 29 4 (set (reg:RVVM1HI 155 [ vect__12.23 ])
>         (sign_extend:RVVM1HI (subreg:RVVMF2QI (reg:RVVMF2x2QI 146 [ vect_array.19 ]) [8, 8]))) "auto.c":29:1 discrim 2 12570 {extendrvvmf2qirvvm1hi2}
>      (expr_list:REG_DEAD (reg:RVVMF2x2QI 146 [ vect_array.19 ])
>         (nil)))
>
> This is the scalable vector with SUBREG_BYTE = poly (8, 8)
>
> After this patch:
>
> static int
> recog_5 (rtx x1 ATTRIBUTE_UNUSED,
>         rtx_insn *insn ATTRIBUTE_UNUSED,
>         int *pnum_clobbers ATTRIBUTE_UNUSED)
> {
>   rtx * const operands ATTRIBUTE_UNUSED = &recog_data.operand[0];
>   rtx x2, x3, x4;
>   int res ATTRIBUTE_UNUSED;
>   if (pnum_clobbers == NULL)
>     return -1;
>   x2 = XEXP (x1, 1);
>   x3 = XEXP (x2, 0);
>   if ((SUBREG_BYTE (x3).is_constant () && maybe_ne (SUBREG_BYTE (x3).to_constant (), 0))   ----> change here and fix ICE.
>       || GET_MODE (x3) != E_SImode
>       || !register_operand (operands[0], E_DImode)
>       || GET_MODE (x2) != E_DImode)
>     return -1;
>
> Does it reasonable ?
>
> Thanks.
>
> gcc/ChangeLog:
>
>         * genrecog.cc (print_test): Add SUBREG_BYTE.to_constant () check.

I think instead we should revert the addition of to_constant.
See: https://inbox.sourceware.org/gcc-patches/mptedn4bwf7.fsf@arm.com/
(and earlier messages in that thread).

Thanks,
Richard

>
> ---
>  gcc/genrecog.cc | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/genrecog.cc b/gcc/genrecog.cc
> index 04a5533ca4b..28884ab3985 100644
> --- a/gcc/genrecog.cc
> +++ b/gcc/genrecog.cc
> @@ -4705,11 +4705,14 @@ print_test (output_state *os, const rtx_test &test, bool is_param,
>        break;
>  
>      case rtx_test::SUBREG_FIELD:
> +      printf ("(SUBREG_BYTE (");
> +      print_test_rtx (os, test);
> +      printf (").is_constant () && ");
>        printf ("%s (", invert_p ? "maybe_ne" : "known_eq");
>        print_nonbool_test (os, test);
>        printf (", ");
>        print_label_value (test, is_param, value);
> -      printf (")");
> +      printf ("))");
>        break;
>  
>      case rtx_test::SAVED_CONST_INT:
juzhe.zhong@rivai.ai Aug. 14, 2023, 10:22 a.m. UTC | #2 
Thanks Richard.

It can fix my issue and reverted to the trunk.

Thanks.


juzhe.zhong@rivai.ai
 
From: Richard Sandiford
Date: 2023-08-14 18:00
To: Juzhe-Zhong
CC: gcc-patches; rguenther; jeffreyalaw
Subject: Re: [PATCH] genrecog: Add SUBREG_BYTE.to_constant check to the genrecog
Juzhe-Zhong <juzhe.zhong@rivai.ai> writes:
> Hi, there is genrecog issue happens in RISC-V backend.
>
> This is the ICE info:
>
> 0xfa3ba4 poly_int_pod<2u, unsigned short>::to_constant() const
>         ../../../riscv-gcc/gcc/poly-int.h:504
> 0x28eaa91 recog_5
>         ../../../riscv-gcc/gcc/config/riscv/bitmanip.md:314
> 0x28ec5b4 recog_7
>         ../../../riscv-gcc/gcc/config/riscv/iterators.md:81
> 0x2a2e740 recog_436
>         ../../../riscv-gcc/gcc/config/riscv/thead.md:265
> 0x2a729ef recog_475
>         ../../../riscv-gcc/gcc/config/riscv/sync.md:509
> 0x2a75aec recog(rtx_def*, rtx_insn*, int*)
>         ../../../riscv-gcc/gcc/config/riscv/iterators.md:55
> 0x2b3e39e recog_for_combine_1
>         ../../../riscv-gcc/gcc/combine.cc:11382
> 0x2b3f457 recog_for_combine
>         ../../../riscv-gcc/gcc/combine.cc:11652
> 0x2b25a15 try_combine
>         ../../../riscv-gcc/gcc/combine.cc:4054
> 0x2b1d3f1 combine_instructions
>         ../../../riscv-gcc/gcc/combine.cc:1266
> 0x2b48cfc rest_of_handle_combine
>         ../../../riscv-gcc/gcc/combine.cc:15063
> 0x2b48db8 execute
>         ../../../riscv-gcc/gcc/combine.cc:15107
>
> This is because the genrecog code here cause ICE for scalable vector in RISC-V:
>
> Before this patch:
>
> static int
> recog_5 (rtx x1 ATTRIBUTE_UNUSED,
>         rtx_insn *insn ATTRIBUTE_UNUSED,
>         int *pnum_clobbers ATTRIBUTE_UNUSED)
> {
>   rtx * const operands ATTRIBUTE_UNUSED = &recog_data.operand[0];
>   rtx x2, x3, x4;
>   int res ATTRIBUTE_UNUSED;
>   if (pnum_clobbers == NULL)
>     return -1;
>   x2 = XEXP (x1, 1);
>   x3 = XEXP (x2, 0);
>   if (maybe_ne (SUBREG_BYTE (x3).to_constant (), 0) ---> this code cause ICE.
>       || GET_MODE (x3) != E_SImode
>       || !register_operand (operands[0], E_DImode)
>       || GET_MODE (x2) != E_DImode)
>     return -1;
> ...
>
> This ICE happens since we have following RTL IR:
>
> (insn 27 26 29 4 (set (reg:RVVM1HI 155 [ vect__12.23 ])
>         (sign_extend:RVVM1HI (subreg:RVVMF2QI (reg:RVVMF2x2QI 146 [ vect_array.19 ]) [8, 8]))) "auto.c":29:1 discrim 2 12570 {extendrvvmf2qirvvm1hi2}
>      (expr_list:REG_DEAD (reg:RVVMF2x2QI 146 [ vect_array.19 ])
>         (nil)))
>
> This is the scalable vector with SUBREG_BYTE = poly (8, 8)
>
> After this patch:
>
> static int
> recog_5 (rtx x1 ATTRIBUTE_UNUSED,
>         rtx_insn *insn ATTRIBUTE_UNUSED,
>         int *pnum_clobbers ATTRIBUTE_UNUSED)
> {
>   rtx * const operands ATTRIBUTE_UNUSED = &recog_data.operand[0];
>   rtx x2, x3, x4;
>   int res ATTRIBUTE_UNUSED;
>   if (pnum_clobbers == NULL)
>     return -1;
>   x2 = XEXP (x1, 1);
>   x3 = XEXP (x2, 0);
>   if ((SUBREG_BYTE (x3).is_constant () && maybe_ne (SUBREG_BYTE (x3).to_constant (), 0))   ----> change here and fix ICE.
>       || GET_MODE (x3) != E_SImode
>       || !register_operand (operands[0], E_DImode)
>       || GET_MODE (x2) != E_DImode)
>     return -1;
>
> Does it reasonable ?
>
> Thanks.
>
> gcc/ChangeLog:
>
>         * genrecog.cc (print_test): Add SUBREG_BYTE.to_constant () check.
 
I think instead we should revert the addition of to_constant.
See: https://inbox.sourceware.org/gcc-patches/mptedn4bwf7.fsf@arm.com/
(and earlier messages in that thread).
 
Thanks,
Richard
 
>
> ---
>  gcc/genrecog.cc | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/genrecog.cc b/gcc/genrecog.cc
> index 04a5533ca4b..28884ab3985 100644
> --- a/gcc/genrecog.cc
> +++ b/gcc/genrecog.cc
> @@ -4705,11 +4705,14 @@ print_test (output_state *os, const rtx_test &test, bool is_param,
>        break;
>  
>      case rtx_test::SUBREG_FIELD:
> +      printf ("(SUBREG_BYTE (");
> +      print_test_rtx (os, test);
> +      printf (").is_constant () && ");
>        printf ("%s (", invert_p ? "maybe_ne" : "known_eq");
>        print_nonbool_test (os, test);
>        printf (", ");
>        print_label_value (test, is_param, value);
> -      printf (")");
> +      printf ("))");
>        break;
>  
>      case rtx_test::SAVED_CONST_INT:
diff mbox series

Patch

diff --git a/gcc/genrecog.cc b/gcc/genrecog.cc
index 04a5533ca4b..28884ab3985 100644
--- a/gcc/genrecog.cc
+++ b/gcc/genrecog.cc
@@ -4705,11 +4705,14 @@  print_test (output_state *os, const rtx_test &test, bool is_param,
       break;
 
     case rtx_test::SUBREG_FIELD:
+      printf ("(SUBREG_BYTE (");
+      print_test_rtx (os, test);
+      printf (").is_constant () && ");
       printf ("%s (", invert_p ? "maybe_ne" : "known_eq");
       print_nonbool_test (os, test);
       printf (", ");
       print_label_value (test, is_param, value);
-      printf (")");
+      printf ("))");
       break;
 
     case rtx_test::SAVED_CONST_INT: