| Message ID | 20230829023642.154907-1-juzhe.zhong@rivai.ai |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Fix AVL/VL get ICE[VSETVL PASS] | expand |
Assuming prev is vsetvli instruction is kind of a strong assumption, but it is guarded with gcc_assert, so it is a reasonable fix to me, LGTM :) On Tue, Aug 29, 2023 at 10:37 AM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote: > > Fix bunch of ICE in "vect" testsuite: > FAIL: gcc.dg/vect/vect-alias-check-16.c (internal compiler error: Segmentation fault) > FAIL: gcc.dg/vect/vect-alias-check-16.c (test for excess errors) > FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault) > FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (test for excess errors) > FAIL: gcc.dg/vect/vect-alias-check-20.c (internal compiler error: Segmentation fault) > FAIL: gcc.dg/vect/vect-alias-check-20.c (test for excess errors) > FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault) > FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (test for excess errors) > > gcc/ChangeLog: > > * config/riscv/riscv-vsetvl.cc (vector_insn_info::get_avl_or_vl_reg): New function. > (pass_vsetvl::compute_local_properties): Fix bug. > (pass_vsetvl::commit_vsetvls): Ditto. > * config/riscv/riscv-vsetvl.h: New function. > > --- > gcc/config/riscv/riscv-vsetvl.cc | 46 +++++++++++++++++++++----------- > gcc/config/riscv/riscv-vsetvl.h | 1 + > 2 files changed, 31 insertions(+), 16 deletions(-) > > diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc > index f7ae6c16bee..73d672b083b 100644 > --- a/gcc/config/riscv/riscv-vsetvl.cc > +++ b/gcc/config/riscv/riscv-vsetvl.cc > @@ -2290,6 +2290,32 @@ vector_insn_info::global_merge (const vector_insn_info &merge_info, > return new_info; > } > > +/* Wrapper helps to return the AVL or VL operand for the > + vector_insn_info. Return AVL if the AVL is not VLMAX. > + Otherwise, return the VL operand. */ > +rtx > +vector_insn_info::get_avl_or_vl_reg (void) const > +{ > + gcc_assert (has_avl_reg ()); > + if (!vlmax_avl_p (get_avl ())) > + return get_avl (); > + > + if (has_vl_op (get_insn ()->rtl ()) || vsetvl_insn_p (get_insn ()->rtl ())) > + return ::get_vl (get_insn ()->rtl ()); > + > + if (get_avl_source ()) > + return get_avl_reg_rtx (); > + > + /* A DIRTY (polluted EMPTY) block if: > + - get_insn is scalar move (no AVL or VL operand). > + - get_avl_source is null (no def in the current DIRTY block). > + Then we trace the previous insn which must be the insn > + already inserted in Phase 2 to get the VL operand for VLMAX. */ > + rtx_insn *prev_rinsn = PREV_INSN (get_insn ()->rtl ()); > + gcc_assert (prev_rinsn && vsetvl_insn_p (prev_rinsn)); > + return ::get_vl (prev_rinsn); > +} > + > bool > vector_insn_info::update_fault_first_load_avl (insn_info *insn) > { > @@ -3166,19 +3192,17 @@ pass_vsetvl::compute_local_properties (void) > bitmap_clear_bit (m_vector_manager->vector_transp[curr_bb_idx], i); > else if (expr->has_avl_reg ()) > { > - rtx avl = vlmax_avl_p (expr->get_avl ()) > - ? get_vl (expr->get_insn ()->rtl ()) > - : expr->get_avl (); > + rtx reg = expr->get_avl_or_vl_reg (); > for (const insn_info *insn : bb->real_nondebug_insns ()) > { > - if (find_access (insn->defs (), REGNO (avl))) > + if (find_access (insn->defs (), REGNO (reg))) > { > bitmap_clear_bit ( > m_vector_manager->vector_transp[curr_bb_idx], i); > break; > } > else if (vlmax_avl_p (expr->get_avl ()) > - && find_access (insn->uses (), REGNO (avl))) > + && find_access (insn->uses (), REGNO (reg))) > { > bitmap_clear_bit ( > m_vector_manager->vector_transp[curr_bb_idx], i); > @@ -3649,17 +3673,7 @@ pass_vsetvl::commit_vsetvls (void) > = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, reaching_out, NULL_RTX); > else if (vlmax_avl_p (reaching_out.get_avl ())) > { > - rtx vl = NULL_RTX; > - /* For user VSETVL VL, AVL. We need to use VL operand here, so we > - don't directly use get_avl_reg_rtx (). Instead, we use the VL > - of the INSN->RTL (). */ > - if (!reaching_out.get_avl_source ()) > - { > - gcc_assert (vsetvl_insn_p (reaching_out.get_insn ()->rtl ())); > - vl = get_vl (reaching_out.get_insn ()->rtl ()); > - } > - else > - vl = reaching_out.get_avl_reg_rtx (); > + rtx vl = reaching_out.get_avl_or_vl_reg (); > new_pat = gen_vsetvl_pat (VSETVL_NORMAL, reaching_out, vl); > } > else > diff --git a/gcc/config/riscv/riscv-vsetvl.h b/gcc/config/riscv/riscv-vsetvl.h > index 4b5825d7f6b..2a315e45f31 100644 > --- a/gcc/config/riscv/riscv-vsetvl.h > +++ b/gcc/config/riscv/riscv-vsetvl.h > @@ -335,6 +335,7 @@ public: > > rtl_ssa::insn_info *get_insn () const { return m_insn; } > const bool *get_demands (void) const { return m_demands; } > + rtx get_avl_or_vl_reg (void) const; > rtx get_avl_reg_rtx (void) const > { > return gen_rtx_REG (Pmode, get_avl_source ()->regno ()); > -- > 2.36.3 >
Committed, thanks Kito. On 2023/8/29 10:46, Kito Cheng via Gcc-patches wrote: > Assuming prev is vsetvli instruction is kind of a strong assumption, > but it is guarded with gcc_assert, so it is a reasonable fix to me, > LGTM :) > > On Tue, Aug 29, 2023 at 10:37 AM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote: >> >> Fix bunch of ICE in "vect" testsuite: >> FAIL: gcc.dg/vect/vect-alias-check-16.c (internal compiler error: Segmentation fault) >> FAIL: gcc.dg/vect/vect-alias-check-16.c (test for excess errors) >> FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault) >> FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (test for excess errors) >> FAIL: gcc.dg/vect/vect-alias-check-20.c (internal compiler error: Segmentation fault) >> FAIL: gcc.dg/vect/vect-alias-check-20.c (test for excess errors) >> FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault) >> FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (test for excess errors) >> >> gcc/ChangeLog: >> >> * config/riscv/riscv-vsetvl.cc (vector_insn_info::get_avl_or_vl_reg): New function. >> (pass_vsetvl::compute_local_properties): Fix bug. >> (pass_vsetvl::commit_vsetvls): Ditto. >> * config/riscv/riscv-vsetvl.h: New function. >> >> --- >> gcc/config/riscv/riscv-vsetvl.cc | 46 +++++++++++++++++++++----------- >> gcc/config/riscv/riscv-vsetvl.h | 1 + >> 2 files changed, 31 insertions(+), 16 deletions(-) >> >> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc >> index f7ae6c16bee..73d672b083b 100644 >> --- a/gcc/config/riscv/riscv-vsetvl.cc >> +++ b/gcc/config/riscv/riscv-vsetvl.cc >> @@ -2290,6 +2290,32 @@ vector_insn_info::global_merge (const vector_insn_info &merge_info, >> return new_info; >> } >> >> +/* Wrapper helps to return the AVL or VL operand for the >> + vector_insn_info. Return AVL if the AVL is not VLMAX. >> + Otherwise, return the VL operand. */ >> +rtx >> +vector_insn_info::get_avl_or_vl_reg (void) const >> +{ >> + gcc_assert (has_avl_reg ()); >> + if (!vlmax_avl_p (get_avl ())) >> + return get_avl (); >> + >> + if (has_vl_op (get_insn ()->rtl ()) || vsetvl_insn_p (get_insn ()->rtl ())) >> + return ::get_vl (get_insn ()->rtl ()); >> + >> + if (get_avl_source ()) >> + return get_avl_reg_rtx (); >> + >> + /* A DIRTY (polluted EMPTY) block if: >> + - get_insn is scalar move (no AVL or VL operand). >> + - get_avl_source is null (no def in the current DIRTY block). >> + Then we trace the previous insn which must be the insn >> + already inserted in Phase 2 to get the VL operand for VLMAX. */ >> + rtx_insn *prev_rinsn = PREV_INSN (get_insn ()->rtl ()); >> + gcc_assert (prev_rinsn && vsetvl_insn_p (prev_rinsn)); >> + return ::get_vl (prev_rinsn); >> +} >> + >> bool >> vector_insn_info::update_fault_first_load_avl (insn_info *insn) >> { >> @@ -3166,19 +3192,17 @@ pass_vsetvl::compute_local_properties (void) >> bitmap_clear_bit (m_vector_manager->vector_transp[curr_bb_idx], i); >> else if (expr->has_avl_reg ()) >> { >> - rtx avl = vlmax_avl_p (expr->get_avl ()) >> - ? get_vl (expr->get_insn ()->rtl ()) >> - : expr->get_avl (); >> + rtx reg = expr->get_avl_or_vl_reg (); >> for (const insn_info *insn : bb->real_nondebug_insns ()) >> { >> - if (find_access (insn->defs (), REGNO (avl))) >> + if (find_access (insn->defs (), REGNO (reg))) >> { >> bitmap_clear_bit ( >> m_vector_manager->vector_transp[curr_bb_idx], i); >> break; >> } >> else if (vlmax_avl_p (expr->get_avl ()) >> - && find_access (insn->uses (), REGNO (avl))) >> + && find_access (insn->uses (), REGNO (reg))) >> { >> bitmap_clear_bit ( >> m_vector_manager->vector_transp[curr_bb_idx], i); >> @@ -3649,17 +3673,7 @@ pass_vsetvl::commit_vsetvls (void) >> = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, reaching_out, NULL_RTX); >> else if (vlmax_avl_p (reaching_out.get_avl ())) >> { >> - rtx vl = NULL_RTX; >> - /* For user VSETVL VL, AVL. We need to use VL operand here, so we >> - don't directly use get_avl_reg_rtx (). Instead, we use the VL >> - of the INSN->RTL (). */ >> - if (!reaching_out.get_avl_source ()) >> - { >> - gcc_assert (vsetvl_insn_p (reaching_out.get_insn ()->rtl ())); >> - vl = get_vl (reaching_out.get_insn ()->rtl ()); >> - } >> - else >> - vl = reaching_out.get_avl_reg_rtx (); >> + rtx vl = reaching_out.get_avl_or_vl_reg (); >> new_pat = gen_vsetvl_pat (VSETVL_NORMAL, reaching_out, vl); >> } >> else >> diff --git a/gcc/config/riscv/riscv-vsetvl.h b/gcc/config/riscv/riscv-vsetvl.h >> index 4b5825d7f6b..2a315e45f31 100644 >> --- a/gcc/config/riscv/riscv-vsetvl.h >> +++ b/gcc/config/riscv/riscv-vsetvl.h >> @@ -335,6 +335,7 @@ public: >> >> rtl_ssa::insn_info *get_insn () const { return m_insn; } >> const bool *get_demands (void) const { return m_demands; } >> + rtx get_avl_or_vl_reg (void) const; >> rtx get_avl_reg_rtx (void) const >> { >> return gen_rtx_REG (Pmode, get_avl_source ()->regno ()); >> -- >> 2.36.3 >>
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc index f7ae6c16bee..73d672b083b 100644 --- a/gcc/config/riscv/riscv-vsetvl.cc +++ b/gcc/config/riscv/riscv-vsetvl.cc @@ -2290,6 +2290,32 @@ vector_insn_info::global_merge (const vector_insn_info &merge_info, return new_info; } +/* Wrapper helps to return the AVL or VL operand for the + vector_insn_info. Return AVL if the AVL is not VLMAX. + Otherwise, return the VL operand. */ +rtx +vector_insn_info::get_avl_or_vl_reg (void) const +{ + gcc_assert (has_avl_reg ()); + if (!vlmax_avl_p (get_avl ())) + return get_avl (); + + if (has_vl_op (get_insn ()->rtl ()) || vsetvl_insn_p (get_insn ()->rtl ())) + return ::get_vl (get_insn ()->rtl ()); + + if (get_avl_source ()) + return get_avl_reg_rtx (); + + /* A DIRTY (polluted EMPTY) block if: + - get_insn is scalar move (no AVL or VL operand). + - get_avl_source is null (no def in the current DIRTY block). + Then we trace the previous insn which must be the insn + already inserted in Phase 2 to get the VL operand for VLMAX. */ + rtx_insn *prev_rinsn = PREV_INSN (get_insn ()->rtl ()); + gcc_assert (prev_rinsn && vsetvl_insn_p (prev_rinsn)); + return ::get_vl (prev_rinsn); +} + bool vector_insn_info::update_fault_first_load_avl (insn_info *insn) { @@ -3166,19 +3192,17 @@ pass_vsetvl::compute_local_properties (void) bitmap_clear_bit (m_vector_manager->vector_transp[curr_bb_idx], i); else if (expr->has_avl_reg ()) { - rtx avl = vlmax_avl_p (expr->get_avl ()) - ? get_vl (expr->get_insn ()->rtl ()) - : expr->get_avl (); + rtx reg = expr->get_avl_or_vl_reg (); for (const insn_info *insn : bb->real_nondebug_insns ()) { - if (find_access (insn->defs (), REGNO (avl))) + if (find_access (insn->defs (), REGNO (reg))) { bitmap_clear_bit ( m_vector_manager->vector_transp[curr_bb_idx], i); break; } else if (vlmax_avl_p (expr->get_avl ()) - && find_access (insn->uses (), REGNO (avl))) + && find_access (insn->uses (), REGNO (reg))) { bitmap_clear_bit ( m_vector_manager->vector_transp[curr_bb_idx], i); @@ -3649,17 +3673,7 @@ pass_vsetvl::commit_vsetvls (void) = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, reaching_out, NULL_RTX); else if (vlmax_avl_p (reaching_out.get_avl ())) { - rtx vl = NULL_RTX; - /* For user VSETVL VL, AVL. We need to use VL operand here, so we - don't directly use get_avl_reg_rtx (). Instead, we use the VL - of the INSN->RTL (). */ - if (!reaching_out.get_avl_source ()) - { - gcc_assert (vsetvl_insn_p (reaching_out.get_insn ()->rtl ())); - vl = get_vl (reaching_out.get_insn ()->rtl ()); - } - else - vl = reaching_out.get_avl_reg_rtx (); + rtx vl = reaching_out.get_avl_or_vl_reg (); new_pat = gen_vsetvl_pat (VSETVL_NORMAL, reaching_out, vl); } else diff --git a/gcc/config/riscv/riscv-vsetvl.h b/gcc/config/riscv/riscv-vsetvl.h index 4b5825d7f6b..2a315e45f31 100644 --- a/gcc/config/riscv/riscv-vsetvl.h +++ b/gcc/config/riscv/riscv-vsetvl.h @@ -335,6 +335,7 @@ public: rtl_ssa::insn_info *get_insn () const { return m_insn; } const bool *get_demands (void) const { return m_demands; } + rtx get_avl_or_vl_reg (void) const; rtx get_avl_reg_rtx (void) const { return gen_rtx_REG (Pmode, get_avl_source ()->regno ());
Fix bunch of ICE in "vect" testsuite: FAIL: gcc.dg/vect/vect-alias-check-16.c (internal compiler error: Segmentation fault) FAIL: gcc.dg/vect/vect-alias-check-16.c (test for excess errors) FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault) FAIL: gcc.dg/vect/vect-alias-check-16.c -flto -ffat-lto-objects (test for excess errors) FAIL: gcc.dg/vect/vect-alias-check-20.c (internal compiler error: Segmentation fault) FAIL: gcc.dg/vect/vect-alias-check-20.c (test for excess errors) FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (internal compiler error: Segmentation fault) FAIL: gcc.dg/vect/vect-alias-check-20.c -flto -ffat-lto-objects (test for excess errors) gcc/ChangeLog: * config/riscv/riscv-vsetvl.cc (vector_insn_info::get_avl_or_vl_reg): New function. (pass_vsetvl::compute_local_properties): Fix bug. (pass_vsetvl::commit_vsetvls): Ditto. * config/riscv/riscv-vsetvl.h: New function. --- gcc/config/riscv/riscv-vsetvl.cc | 46 +++++++++++++++++++++----------- gcc/config/riscv/riscv-vsetvl.h | 1 + 2 files changed, 31 insertions(+), 16 deletions(-)