Message ID | 20240429074014.1576154-1-pan2.li@intel.com |
---|---|
State | New |
Headers | show |
Series | [v2] RISC-V: Fix ICE for legitimize move on subreg const_poly_int [PR114885] | expand |
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 >
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
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
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