[PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move

[PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move

[pan2.li]

From: Pan Li <pan2.li@intel.com>

When we build with isl, there will be a ICE for graphite in both
the c/c++ and fortran.  The legitimize move cannot take care of
below rtl.

(set (subreg:DI (reg:TI 237) 8) (subreg:DI (const_poly_int:TI [4, 2]) 8))

Then we will have ice similar to below:

internal compiler error: in extract_insn, at recog.cc:2812.

This patch would like to take care of the above rtl.  Given the value of
const_poly_int can hardly excceed the max of int64,  we can simply
consider the highest 8 bytes of TImode is zero and then set the dest
to (const_int 0).

The below test cases are fixed by this PATCH.

C:
FAIL: gcc.dg/graphite/pr111878.c (internal compiler error: in
extract_insn, at recog.cc:2812)
FAIL: gcc.dg/graphite/pr111878.c (test for excess errors)

Fortran:
FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (internal compiler
error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
compiler error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (test for excess
errors)
FAIL: gfortran.dg/graphite/pr14741.f90   -O  (test for excess errors)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (test for excess
errors)
FAIL: gfortran.dg/graphite/id-27.f90   -O  (internal compiler error: in
extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (internal compiler
error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (test for excess
errors)
FAIL: gfortran.dg/graphite/id-27.f90   -O  (test for excess errors)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
compiler error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr14741.f90   -O  (internal compiler error:
in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (internal compiler
error: in extract_insn, at recog.cc:2812)

The below test suites are passed for this patch:
* The rv64gcv fully regression test.
* The rv64gc fully regression test.

Try to write some RTL code for test but not works well according to
existing test cases.  Thus, take above as test cases.  Please note
graphite require the gcc build with isl.

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_legitimize_subreg_const_poly_move): New
	func impl to take care of (const_int_poly:TI 8).
	(riscv_legitimize_move): Handle subreg is const_int_poly,

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/config/riscv/riscv.cc | 43 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 0519e0679ed..bad23ea487f 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2786,6 +2786,44 @@ riscv_v_adjust_scalable_frame (rtx target, poly_int64 offset, bool epilogue)
   REG_NOTES (insn) = dwarf;
 }
 
+/* Take care below subreg const_poly_int move:
+
+   1. (set (subreg:DI (reg:TI 237) 8)
+	   (subreg:DI (const_poly_int:TI [4, 2]) 8))
+      =>
+      (set (subreg:DI (reg:TI 237) 8)
+	   (const_int 0)) */
+
+static bool
+riscv_legitimize_subreg_const_poly_move (machine_mode mode, rtx dest, rtx src)
+{
+  gcc_assert (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src)));
+  gcc_assert (SUBREG_BYTE (src).is_constant ());
+
+  int byte_offset = SUBREG_BYTE (src).to_constant ();
+  rtx const_poly = SUBREG_REG (src);
+  machine_mode subreg_mode = GET_MODE (const_poly);
+
+  if (subreg_mode != TImode) /* Only TImode is needed for now.  */
+    return false;
+
+  if (byte_offset == 8)
+    { /* The const_poly_int cannot exceed int64, just set zero here.  */
+      emit_move_insn (dest, CONST0_RTX (mode));
+      return true;
+    }
+
+  /* The below transform will be covered in somewhere else.
+     Thus, ignore this here.
+   1. (set (subreg:DI (reg:TI 237) 0)
+	   (subreg:DI (const_poly_int:TI [4, 2]) 0))
+      =>
+      (set (subreg:DI (reg:TI 237) 0)
+	   (const_poly_int:DI [4, 2])) */
+
+  return false;
+}
+
 /* If (set DEST SRC) is not a valid move instruction, emit an equivalent
    sequence that is valid.  */
 
@@ -2839,6 +2877,11 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
 	}
       return true;
     }
+
+  if (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src))
+    && riscv_legitimize_subreg_const_poly_move (mode, dest, src))
+    return true;
+
   /* Expand
        (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
      Expand this data movement instead of simply forbid it since
-- 
2.34.1

Re: [PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move

[juzhe.zhong]

[-- Attachment #1: Type: text/plain, Size: 5368 bytes --]

LGTM from my side. But give kito more time chime in.



juzhe.zhong@rivai.ai
 
From: pan2.li
Date: 2024-04-28 11:53
To: gcc-patches
CC: juzhe.zhong; kito.cheng; Pan Li
Subject: [PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move
From: Pan Li <pan2.li@intel.com>
 
When we build with isl, there will be a ICE for graphite in both
the c/c++ and fortran.  The legitimize move cannot take care of
below rtl.
 
(set (subreg:DI (reg:TI 237) 8) (subreg:DI (const_poly_int:TI [4, 2]) 8))
 
Then we will have ice similar to below:
 
internal compiler error: in extract_insn, at recog.cc:2812.
 
This patch would like to take care of the above rtl.  Given the value of
const_poly_int can hardly excceed the max of int64,  we can simply
consider the highest 8 bytes of TImode is zero and then set the dest
to (const_int 0).
 
The below test cases are fixed by this PATCH.
 
C:
FAIL: gcc.dg/graphite/pr111878.c (internal compiler error: in
extract_insn, at recog.cc:2812)
FAIL: gcc.dg/graphite/pr111878.c (test for excess errors)
 
Fortran:
FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (internal compiler
error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
compiler error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (test for excess
errors)
FAIL: gfortran.dg/graphite/pr14741.f90   -O  (test for excess errors)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (test for excess
errors)
FAIL: gfortran.dg/graphite/id-27.f90   -O  (internal compiler error: in
extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (internal compiler
error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (test for excess
errors)
FAIL: gfortran.dg/graphite/id-27.f90   -O  (test for excess errors)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
compiler error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr14741.f90   -O  (internal compiler error:
in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (internal compiler
error: in extract_insn, at recog.cc:2812)
 
The below test suites are passed for this patch:
* The rv64gcv fully regression test.
* The rv64gc fully regression test.
 
Try to write some RTL code for test but not works well according to
existing test cases.  Thus, take above as test cases.  Please note
graphite require the gcc build with isl.
 
gcc/ChangeLog:
 
* config/riscv/riscv.cc (riscv_legitimize_subreg_const_poly_move): New
func impl to take care of (const_int_poly:TI 8).
(riscv_legitimize_move): Handle subreg is const_int_poly,
 
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/config/riscv/riscv.cc | 43 +++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 0519e0679ed..bad23ea487f 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2786,6 +2786,44 @@ riscv_v_adjust_scalable_frame (rtx target, poly_int64 offset, bool epilogue)
   REG_NOTES (insn) = dwarf;
}
+/* Take care below subreg const_poly_int move:
+
+   1. (set (subreg:DI (reg:TI 237) 8)
+    (subreg:DI (const_poly_int:TI [4, 2]) 8))
+      =>
+      (set (subreg:DI (reg:TI 237) 8)
+    (const_int 0)) */
+
+static bool
+riscv_legitimize_subreg_const_poly_move (machine_mode mode, rtx dest, rtx src)
+{
+  gcc_assert (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src)));
+  gcc_assert (SUBREG_BYTE (src).is_constant ());
+
+  int byte_offset = SUBREG_BYTE (src).to_constant ();
+  rtx const_poly = SUBREG_REG (src);
+  machine_mode subreg_mode = GET_MODE (const_poly);
+
+  if (subreg_mode != TImode) /* Only TImode is needed for now.  */
+    return false;
+
+  if (byte_offset == 8)
+    { /* The const_poly_int cannot exceed int64, just set zero here.  */
+      emit_move_insn (dest, CONST0_RTX (mode));
+      return true;
+    }
+
+  /* The below transform will be covered in somewhere else.
+     Thus, ignore this here.
+   1. (set (subreg:DI (reg:TI 237) 0)
+    (subreg:DI (const_poly_int:TI [4, 2]) 0))
+      =>
+      (set (subreg:DI (reg:TI 237) 0)
+    (const_poly_int:DI [4, 2])) */
+
+  return false;
+}
+
/* If (set DEST SRC) is not a valid move instruction, emit an equivalent
    sequence that is valid.  */
@@ -2839,6 +2877,11 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
}
       return true;
     }
+
+  if (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src))
+    && riscv_legitimize_subreg_const_poly_move (mode, dest, src))
+    return true;
+
   /* Expand
        (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
      Expand this data movement instead of simply forbid it since
-- 
2.34.1
 
 

Re: [PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move

[Kito Cheng]

> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 0519e0679ed..bad23ea487f 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -2786,6 +2786,44 @@ riscv_v_adjust_scalable_frame (rtx target, poly_int64 offset, bool epilogue)
>    REG_NOTES (insn) = dwarf;
> }
> +/* Take care below subreg const_poly_int move:
> +
> +   1. (set (subreg:DI (reg:TI 237) 8)
> +    (subreg:DI (const_poly_int:TI [4, 2]) 8))
> +      =>
> +      (set (subreg:DI (reg:TI 237) 8)
> +    (const_int 0)) */
> +
> +static bool
> +riscv_legitimize_subreg_const_poly_move (machine_mode mode, rtx dest, rtx src)
> +{
> +  gcc_assert (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src)));
> +  gcc_assert (SUBREG_BYTE (src).is_constant ());
> +
> +  int byte_offset = SUBREG_BYTE (src).to_constant ();
> +  rtx const_poly = SUBREG_REG (src);
> +  machine_mode subreg_mode = GET_MODE (const_poly);
> +
> +  if (subreg_mode != TImode) /* Only TImode is needed for now.  */
> +    return false;
> +
> +  if (byte_offset == 8)
> +    { /* The const_poly_int cannot exceed int64, just set zero here.  */

{
 /* The const_poly_int cannot exceed int64, just set zero here.  */

New line for the comment.

> +      emit_move_insn (dest, CONST0_RTX (mode));
> +      return true;
> +    }
> +
> +  /* The below transform will be covered in somewhere else.
> +     Thus, ignore this here.
> +   1. (set (subreg:DI (reg:TI 237) 0)
> +    (subreg:DI (const_poly_int:TI [4, 2]) 0))
> +      =>
> +      (set (subreg:DI (reg:TI 237) 0)
> +    (const_poly_int:DI [4, 2])) */
> +
> +  return false;
> +}
> +
> /* If (set DEST SRC) is not a valid move instruction, emit an equivalent
>     sequence that is valid.  */
> @@ -2839,6 +2877,11 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
> }
>        return true;
>      }
> +
> +  if (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src))
> +    && riscv_legitimize_subreg_const_poly_move (mode, dest, src))
> +    return true;
> +
>    /* Expand
>         (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
>       Expand this data movement instead of simply forbid it since
> --
> 2.34.1
>
>

[PATCH v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885]

[pan2.li]

From: Pan Li <pan2.li@intel.com>

When we build with isl, there will be a ICE for graphite in both
the c/c++ and fortran.  The legitimize move cannot take care of
below rtl.

(set (subreg:DI (reg:TI 237) 8) (subreg:DI (const_poly_int:TI [4, 2]) 8))

Then we will have ice similar to below:

internal compiler error: in extract_insn, at recog.cc:2812.

This patch would like to take care of the above rtl.  Given the value of
const_poly_int can hardly excceed the max of int64,  we can simply
consider the highest 8 bytes of TImode is zero and then set the dest
to (const_int 0).

The below test cases are fixed by this PATCH.

C:
FAIL: gcc.dg/graphite/pr111878.c (internal compiler error: in
extract_insn, at recog.cc:2812)
FAIL: gcc.dg/graphite/pr111878.c (test for excess errors)

Fortran:
FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (internal compiler
error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
compiler error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (test for excess
errors)
FAIL: gfortran.dg/graphite/pr14741.f90   -O  (test for excess errors)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (test for excess
errors)
FAIL: gfortran.dg/graphite/id-27.f90   -O  (internal compiler error: in
extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (internal compiler
error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (test for excess
errors)
FAIL: gfortran.dg/graphite/id-27.f90   -O  (test for excess errors)
FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
compiler error: in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr14741.f90   -O  (internal compiler error:
in extract_insn, at recog.cc:2812)
FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (internal compiler
error: in extract_insn, at recog.cc:2812)

The below test suites are passed for this patch:
* The rv64gcv fully regression test.
* The rv64gc fully regression test.

Try to write some RTL code for test but not works well according to
existing test cases.  Thus, take above as test cases.  Please note
graphite require the gcc build with isl.

	PR target/114885

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_legitimize_subreg_const_poly_move): New
	func impl to take care of (const_int_poly:TI 8).
	(riscv_legitimize_move): Handle subreg is const_int_poly,

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/config/riscv/riscv.cc | 44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 0519e0679ed..0f62b295b96 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2786,6 +2786,45 @@ riscv_v_adjust_scalable_frame (rtx target, poly_int64 offset, bool epilogue)
   REG_NOTES (insn) = dwarf;
 }
 
+/* Take care below subreg const_poly_int move:
+
+   1. (set (subreg:DI (reg:TI 237) 8)
+	   (subreg:DI (const_poly_int:TI [4, 2]) 8))
+      =>
+      (set (subreg:DI (reg:TI 237) 8)
+	   (const_int 0)) */
+
+static bool
+riscv_legitimize_subreg_const_poly_move (machine_mode mode, rtx dest, rtx src)
+{
+  gcc_assert (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src)));
+  gcc_assert (SUBREG_BYTE (src).is_constant ());
+
+  int byte_offset = SUBREG_BYTE (src).to_constant ();
+  rtx const_poly = SUBREG_REG (src);
+  machine_mode subreg_mode = GET_MODE (const_poly);
+
+  if (subreg_mode != TImode) /* Only TImode is needed for now.  */
+    return false;
+
+  if (byte_offset == 8)
+    {
+      /* The const_poly_int cannot exceed int64, just set zero here.  */
+      emit_move_insn (dest, CONST0_RTX (mode));
+      return true;
+    }
+
+  /* The below transform will be covered in somewhere else.
+     Thus, ignore this here.
+     (set (subreg:DI (reg:TI 237) 0)
+	  (subreg:DI (const_poly_int:TI [4, 2]) 0))
+     =>
+     (set (subreg:DI (reg:TI 237) 0)
+	  (const_poly_int:DI [4, 2])) */
+
+  return false;
+}
+
 /* If (set DEST SRC) is not a valid move instruction, emit an equivalent
    sequence that is valid.  */
 
@@ -2839,6 +2878,11 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
 	}
       return true;
     }
+
+  if (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src))
+    && riscv_legitimize_subreg_const_poly_move (mode, dest, src))
+    return true;
+
   /* Expand
        (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
      Expand this data movement instead of simply forbid it since
-- 
2.34.1

RE: [PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move

[Li, Pan2]

Thanks Kito, sent v2 for addressing comments.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Monday, April 29, 2024 2:37 PM
To: juzhe.zhong@rivai.ai
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH v1] RISC-V: Fix ICE for legitimize move on subreg const_poly_move

> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 0519e0679ed..bad23ea487f 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -2786,6 +2786,44 @@ riscv_v_adjust_scalable_frame (rtx target, poly_int64 offset, bool epilogue)
>    REG_NOTES (insn) = dwarf;
> }
> +/* Take care below subreg const_poly_int move:
> +
> +   1. (set (subreg:DI (reg:TI 237) 8)
> +    (subreg:DI (const_poly_int:TI [4, 2]) 8))
> +      =>
> +      (set (subreg:DI (reg:TI 237) 8)
> +    (const_int 0)) */
> +
> +static bool
> +riscv_legitimize_subreg_const_poly_move (machine_mode mode, rtx dest, rtx src)
> +{
> +  gcc_assert (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src)));
> +  gcc_assert (SUBREG_BYTE (src).is_constant ());
> +
> +  int byte_offset = SUBREG_BYTE (src).to_constant ();
> +  rtx const_poly = SUBREG_REG (src);
> +  machine_mode subreg_mode = GET_MODE (const_poly);
> +
> +  if (subreg_mode != TImode) /* Only TImode is needed for now.  */
> +    return false;
> +
> +  if (byte_offset == 8)
> +    { /* The const_poly_int cannot exceed int64, just set zero here.  */

{
 /* The const_poly_int cannot exceed int64, just set zero here.  */

New line for the comment.

> +      emit_move_insn (dest, CONST0_RTX (mode));
> +      return true;
> +    }
> +
> +  /* The below transform will be covered in somewhere else.
> +     Thus, ignore this here.
> +   1. (set (subreg:DI (reg:TI 237) 0)
> +    (subreg:DI (const_poly_int:TI [4, 2]) 0))
> +      =>
> +      (set (subreg:DI (reg:TI 237) 0)
> +    (const_poly_int:DI [4, 2])) */
> +
> +  return false;
> +}
> +
> /* If (set DEST SRC) is not a valid move instruction, emit an equivalent
>     sequence that is valid.  */
> @@ -2839,6 +2877,11 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
> }
>        return true;
>      }
> +
> +  if (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src))
> +    && riscv_legitimize_subreg_const_poly_move (mode, dest, src))
> +    return true;
> +
>    /* Expand
>         (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
>       Expand this data movement instead of simply forbid it since
> --
> 2.34.1
>
>

Re: [PATCH v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885]

[Kito Cheng]

Hi Pan:

LGTM.

Hi Jakub:

Is this OK for GCC 14 branch? it's fix ICE on valid code, thanks :)

On Mon, Apr 29, 2024 at 3:40 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> When we build with isl, there will be a ICE for graphite in both
> the c/c++ and fortran.  The legitimize move cannot take care of
> below rtl.
>
> (set (subreg:DI (reg:TI 237) 8) (subreg:DI (const_poly_int:TI [4, 2]) 8))
>
> Then we will have ice similar to below:
>
> internal compiler error: in extract_insn, at recog.cc:2812.
>
> This patch would like to take care of the above rtl.  Given the value of
> const_poly_int can hardly excceed the max of int64,  we can simply
> consider the highest 8 bytes of TImode is zero and then set the dest
> to (const_int 0).
>
> The below test cases are fixed by this PATCH.
>
> C:
> FAIL: gcc.dg/graphite/pr111878.c (internal compiler error: in
> extract_insn, at recog.cc:2812)
> FAIL: gcc.dg/graphite/pr111878.c (test for excess errors)
>
> Fortran:
> FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (internal compiler
> error: in extract_insn, at recog.cc:2812)
> FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
> compiler error: in extract_insn, at recog.cc:2812)
> FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (test for excess
> errors)
> FAIL: gfortran.dg/graphite/pr14741.f90   -O  (test for excess errors)
> FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
> excess errors)
> FAIL: gfortran.dg/graphite/vect-pr40979.f90   -O  (test for excess
> errors)
> FAIL: gfortran.dg/graphite/id-27.f90   -O  (internal compiler error: in
> extract_insn, at recog.cc:2812)
> FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (internal compiler
> error: in extract_insn, at recog.cc:2812)
> FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -g  (test for excess
> errors)
> FAIL: gfortran.dg/graphite/id-27.f90   -O  (test for excess errors)
> FAIL: gfortran.dg/graphite/pr29832.f90   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
> excess errors)
> FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (internal
> compiler error: in extract_insn, at recog.cc:2812)
> FAIL: gfortran.dg/graphite/pr14741.f90   -O  (internal compiler error:
> in extract_insn, at recog.cc:2812)
> FAIL: gfortran.dg/graphite/pr29581.f90   -O3 -g  (internal compiler
> error: in extract_insn, at recog.cc:2812)
>
> The below test suites are passed for this patch:
> * The rv64gcv fully regression test.
> * The rv64gc fully regression test.
>
> Try to write some RTL code for test but not works well according to
> existing test cases.  Thus, take above as test cases.  Please note
> graphite require the gcc build with isl.
>
>         PR target/114885
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_legitimize_subreg_const_poly_move): New
>         func impl to take care of (const_int_poly:TI 8).
>         (riscv_legitimize_move): Handle subreg is const_int_poly,
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/config/riscv/riscv.cc | 44 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 0519e0679ed..0f62b295b96 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -2786,6 +2786,45 @@ riscv_v_adjust_scalable_frame (rtx target, poly_int64 offset, bool epilogue)
>    REG_NOTES (insn) = dwarf;
>  }
>
> +/* Take care below subreg const_poly_int move:
> +
> +   1. (set (subreg:DI (reg:TI 237) 8)
> +          (subreg:DI (const_poly_int:TI [4, 2]) 8))
> +      =>
> +      (set (subreg:DI (reg:TI 237) 8)
> +          (const_int 0)) */
> +
> +static bool
> +riscv_legitimize_subreg_const_poly_move (machine_mode mode, rtx dest, rtx src)
> +{
> +  gcc_assert (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src)));
> +  gcc_assert (SUBREG_BYTE (src).is_constant ());
> +
> +  int byte_offset = SUBREG_BYTE (src).to_constant ();
> +  rtx const_poly = SUBREG_REG (src);
> +  machine_mode subreg_mode = GET_MODE (const_poly);
> +
> +  if (subreg_mode != TImode) /* Only TImode is needed for now.  */
> +    return false;
> +
> +  if (byte_offset == 8)
> +    {
> +      /* The const_poly_int cannot exceed int64, just set zero here.  */
> +      emit_move_insn (dest, CONST0_RTX (mode));
> +      return true;
> +    }
> +
> +  /* The below transform will be covered in somewhere else.
> +     Thus, ignore this here.
> +     (set (subreg:DI (reg:TI 237) 0)
> +         (subreg:DI (const_poly_int:TI [4, 2]) 0))
> +     =>
> +     (set (subreg:DI (reg:TI 237) 0)
> +         (const_poly_int:DI [4, 2])) */
> +
> +  return false;
> +}
> +
>  /* If (set DEST SRC) is not a valid move instruction, emit an equivalent
>     sequence that is valid.  */
>
> @@ -2839,6 +2878,11 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
>         }
>        return true;
>      }
> +
> +  if (SUBREG_P (src) && CONST_POLY_INT_P (SUBREG_REG (src))
> +    && riscv_legitimize_subreg_const_poly_move (mode, dest, src))
> +    return true;
> +
>    /* Expand
>         (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
>       Expand this data movement instead of simply forbid it since
> --
> 2.34.1
>

Re: [PATCH v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885]

[Jakub Jelinek]

On Mon, Apr 29, 2024 at 03:47:05PM +0800, Kito Cheng wrote:
> Hi Jakub:
> 
> Is this OK for GCC 14 branch? it's fix ICE on valid code, thanks :)

Ok.

	Jakub

RE: [PATCH v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885]

[Li, Pan2]

Committed to trunk, thanks Kito.

Backported to releases/gcc-14, thanks Jakub.

Pan

-----Original Message-----
From: Jakub Jelinek <jakub@redhat.com> 
Sent: Monday, April 29, 2024 3:53 PM
To: Kito Cheng <kito.cheng@gmail.com>
Cc: Li, Pan2 <pan2.li@intel.com>; Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai
Subject: Re: [PATCH v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885]

On Mon, Apr 29, 2024 at 03:47:05PM +0800, Kito Cheng wrote:
> Hi Jakub:
> 
> Is this OK for GCC 14 branch? it's fix ICE on valid code, thanks :)

Ok.

	Jakub