On Fri, Mar 15, 2024 at 12:24 PM Andrew Stubbs <a...@baylibre.com> wrote: > > On 15/03/2024 07:35, Richard Biener wrote: > > On Fri, Mar 15, 2024 at 4:35 AM Hongtao Liu <crazy...@gmail.com> wrote: > >> > >> On Thu, Mar 14, 2024 at 11:42 PM Andrew Stubbs <a...@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. > > I thought that the "not" to "xor" change was nice and there was already > code there for fixing bitfields, but OK, I take your point. > > The .COND_* statements are handled as internal function calls that are > expanded directly via the optab with no special cases for different call > types. This is because the "expand_cond_*_optab_fn" functions just map > straight to "expand_direct_optab_fn".... would that be the right place > to insert a special case handler to insert "and" operations?
Yes, I think in expand_fn_using_insn where we handle the "undefined" input operands we want to handle the vector bool operands as well, masking rhs_rtx accordingly. Richard. > Andrew