RE: [PATCH PR96757] aarch64: ICE during GIMPLE pass: vect

[duanbo (C)]

> -----Original Message-----
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Wednesday, September 30, 2020 6:38 PM
> To: duanbo (C) <duan...@huawei.com>
> Cc: GCC Patches <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH PR96757] aarch64: ICE during GIMPLE pass: vect
> 
> Thanks for the update, looks good apart from…
> 
> "duanbo (C)" <duan...@huawei.com> writes:
> > @@ -4361,7 +4391,7 @@ vect_recog_mask_conversion_pattern (vec_info
> *vinfo,
> >        if (known_eq (TYPE_VECTOR_SUBPARTS (vectype1),
> >                 TYPE_VECTOR_SUBPARTS (vectype2))
> >       && (TREE_CODE (rhs1) == SSA_NAME
> > -         || rhs1_type == TREE_TYPE (TREE_OPERAND (rhs1, 0))))
> > +         || !rhs1_op0_type || !rhs1_op1_type))
> >     return NULL;
> 
> …I think this should be:
> 
>         && (TREE_CODE (rhs1) == SSA_NAME
>             || (!rhs1_op0_type && !rhs1_op1_type))
> 
> i.e. punt only if both types are already OK.  If one operand wants a specific
> mask type, we should continue to the code below and attach the chosen
> type to the comparison.
> 
> Although I guess this simplifies to:
> 
>       if (known_eq (TYPE_VECTOR_SUBPARTS (vectype1),
>                     TYPE_VECTOR_SUBPARTS (vectype2))
>           && !rhs1_op0_type
>           && !rhs1_op1_type)
>         return NULL;
> 
> (I think the comment above the code is still accurate with this change.)
> 
> > @@ -4393,7 +4423,16 @@ vect_recog_mask_conversion_pattern
> (vec_info *vinfo,
> >        if (TREE_CODE (rhs1) != SSA_NAME)
> >     {
> >       tmp = vect_recog_temp_ssa_var (TREE_TYPE (rhs1), NULL);
> > -     pattern_stmt = gimple_build_assign (tmp, rhs1);
> > +     if (rhs1_op0_type && TYPE_PRECISION (rhs1_op0_type)
> > +                           != TYPE_PRECISION (rhs1_type))
> > +       rhs1_op0 = build_mask_conversion (vinfo, rhs1_op0,
> > +                                         vectype2, stmt_vinfo);
> > +     if (rhs1_op1_type && TYPE_PRECISION (rhs1_op1_type)
> > +                           != TYPE_PRECISION (rhs1_type))
> 
> Very minor -- I would have fixed this up before committing if it wasn't for 
> the
> above -- but: GCC formatting is instead:
> 
>         if (rhs1_op1_type
>             && TYPE_PRECISION (rhs1_op1_type) != TYPE_PRECISION
> (rhs1_type))
> 
> LGTM with those changes, thanks.
> 
> Richard
> 
> > +       rhs1_op1 = build_mask_conversion (vinfo, rhs1_op1,
> > +                                         vectype2, stmt_vinfo);
> > +     pattern_stmt = gimple_build_assign (tmp, TREE_CODE (rhs1),
> > +                                         rhs1_op0, rhs1_op1);
> >       rhs1 = tmp;
> >       append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt,
> vectype2,
> >                               rhs1_type);

Sorry for the late reply.
I have modified the patch according to your suggestion, and it works well.
Ok for trunk?

Thanks,
Duan bo

pr96757-v3.patch
Description: pr96757-v3.patch