Re: [PATCH] vect: Use xor to invert oversized vector masks

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Mar 15, 2024 at 4:35 AM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Thu, Mar 14, 2024 at 11:42 PM Andrew Stubbs <ams@baylibre.com> wrote:
> >
> > Don't enable excess lanes when inverting vector bit-masks smaller than the
> > integer mode.  This is yet another case of wrong-code due to mishandling
> > of oversized bitmasks.
> >
> > This issue shows up in vect/tsvc/vect-tsvc-s278.c and
> > vect/tsvc/vect-tsvc-s279.c if I set the preferred vector size to V32
> > (down from V64) on amdgcn.
> >
> > OK for mainline?
> >
> > Andrew
> >
> > gcc/ChangeLog:
> >
> >         * expr.cc (expand_expr_real_2): Use xor to invert vector masks.
> > ---
> >  gcc/expr.cc | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/gcc/expr.cc b/gcc/expr.cc
> > index 403eeaa108e4..3540327d879e 100644
> > --- a/gcc/expr.cc
> > +++ b/gcc/expr.cc
> > @@ -10497,6 +10497,17 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
> >                                immed_wide_int_const (mask, int_mode),
> >                                target, 1, OPTAB_LIB_WIDEN);
> >         }
> > +      /* If it's a vector mask don't enable excess bits.  */
> > +      else if (VECTOR_BOOLEAN_TYPE_P (type)
> > +              && SCALAR_INT_MODE_P (mode)
> > +              && maybe_ne (GET_MODE_PRECISION (mode),
> > +                           TYPE_VECTOR_SUBPARTS (type).to_constant ()))
> > +       {
> > +         auto nunits = TYPE_VECTOR_SUBPARTS (type).to_constant ();
> > +         temp = expand_binop (mode, xor_optab, op0,
> > +                              GEN_INT ((HOST_WIDE_INT_1U << nunits) - 1),
> > +                              target, true, OPTAB_WIDEN);
> > +       }
> Not review, just curious, should the issue be fixed by the commit in PR113576.
> Also wonder besides cbranch, excess land bits also matter?
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113576#c35

Yes, you patch BIT_NOT but we decided to patch final compares.  Is it that
we need to fixup every mask use in a .COND_* expansion as well?  If so
we should do it there.

Richard.

> >        else
> >         temp = expand_unop (mode, one_cmpl_optab, op0, target, 1);
> >        gcc_assert (temp);
> > --
> > 2.41.0
> >
>
>
> --
> BR,
> Hongtao