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.zh...@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
>

Reply via email to