> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Monday, October 21, 2024 9:55 AM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>
> Subject: Re: [PATCH]middle-end: Handle more gcond lowering [PR117176]
>
> On Mon, 21 Oct 2024, Tamar Christina wrote:
>
> > Hi All,
> >
> > For boolean mask handling we have to lower BIT_NOT_EXPR for correctness into
> > BIT_XOR_EXPR. Normally this is done through vect_recog_bool_pattern by
> > following the defs of the gimple_assign.
> >
> > In the PR we ICE because early exits have the comparison inside the gcond
> > itself and so vect_recog_bool_pattern does not get a chance to lower it.
> >
> > This patch changes it so vect_recog_gcond_pattern also does the lowering.
> >
> > The reason we ICE with SLP but not non-SLP is because for non-SLP the
> > vectorization is driven from the gcond, where vectorizable_comparison_1
> > rejects the operation. For SLP the body is vectorized before the root.
> >
> > My initial attempt was to try to re-use vect_recog_bool_pattern but this
> > didn't
> > work for the same reasons we didn't change vect_recog_bool_pattern to handle
> > gconds.
> >
> > I also tried introducing an explicit assign for the conditional and having
> > vect_recog_bool_pattern see the conditional. However this fails because
> > vect_recog_bool_pattern and helpers only handle COND_EXPR, converts and
> > operands that are being loaded from memory.
> >
> > Since I believe BIT_NOT_EXPR is the only one needing to be lowered for
> > correctness, I just manually do so like we do in other places.
>
> So how does it work for BIT_IOR_EXPR/BIT_AND_EXPR and their feeding
> stmts? How do those get transformed for the mask use? I'd have expected
> the same mechanism to handle the BIT_NOT_EXPR case.
It's only BIT_NOT that requires lowering because vectorizable_operation does not
allow operations on bit precision types without any truncations being explicit.
As such we only allow BIT_AND, BIT_XOR and BIT_IOR unmodified. Bit NOT needs
to make the precision explicit by using the XOR.
BIT_IOR_EXPR and BIT_AND_EXPR do not require mask handling transformations.
e.g. vect_recog_mask_conversion_pattern explicitly doesn't transform them.
And the only handling done for them in adjust_bool_pattern is a fixup for
optimization reasons around combining with COND_EXPR here the arguments
could have different precisions. i.e. X ^ (a > b ? 1 : 0).
Does this answer your question?
Thanks,
Tamar
>
> Richard.
>
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > PR tree-optimization/117176
> > * tree-vect-patterns.cc (vect_recog_gcond_pattern): Lower
> BIT_NOT_EXPR.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR tree-optimization/117176
> > * gcc.dg/vect/vect-early-break_130-pr117176.c: New test.
> >
> > ---
> > diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_130-pr117176.c
> b/gcc/testsuite/gcc.dg/vect/vect-early-break_130-pr117176.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..841dcce284dd7cff0c4f064
> 8e6dc57082c8756c1
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_130-pr117176.c
> > @@ -0,0 +1,21 @@
> > +/* { dg-do compile } */
> > +/* { dg-add-options vect_early_break } */
> > +/* { dg-require-effective-target vect_early_break } */
> > +/* { dg-require-effective-target vect_int } */
> > +
> > +struct ColorSpace {
> > + int componentCt;
> > +};
> > +
> > +struct Psnr {
> > + double psnr[3];
> > +};
> > +
> > +int f(struct Psnr psnr, struct ColorSpace colorSpace) {
> > + int i, hitsTarget = 1;
> > +
> > + for (i = 1; i < colorSpace.componentCt && hitsTarget; ++i)
> > + hitsTarget = !(psnr.psnr[i] < 1);
> > +
> > + return hitsTarget;
> > +}
> > diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> > index
> 746f100a0842090953571b535ff05375f46033c0..b7cb6cf1308590e2723ca287
> 2cf917edb4036c57 100644
> > --- a/gcc/tree-vect-patterns.cc
> > +++ b/gcc/tree-vect-patterns.cc
> > @@ -5946,8 +5946,45 @@ vect_recog_gcond_pattern (vec_info *vinfo,
> >
> > if (code == NE_EXPR
> > && zerop (rhs)
> > + && TREE_CODE (lhs) == SSA_NAME
> > && VECT_SCALAR_BOOLEAN_TYPE_P (scalar_type))
> > - return NULL;
> > + {
> > + stmt_vec_info def_stmt_info = vect_get_internal_def (vinfo, lhs);
> > + if (!def_stmt_info)
> > + return NULL;
> > +
> > + gimple *lhs_stmt = STMT_VINFO_STMT (def_stmt_info);
> > + if (!is_gimple_assign (lhs_stmt))
> > + return NULL;
> > +
> > + /* This is normally handled by adjust_bool_pattern, however this is
> > + driven from a gimple assign normally and cannot handle gcond.
> > + BIT_NOT_EXPR are the only ones needing to be lowered for correctness,
> > + the rest are more optimizations. */
> > + switch (gimple_assign_rhs_code (lhs_stmt))
> > + {
> > + case BIT_NOT_EXPR:
> > + {
> > + tree rhs1 = gimple_assign_rhs1 (lhs_stmt);
> > + tree itype = TREE_TYPE (rhs1);
> > + tree ilhs = vect_recog_temp_ssa_var (boolean_type_node, NULL);
> > + gimple *pattern_stmt
> > + = gimple_build_assign (ilhs, BIT_XOR_EXPR, rhs1,
> > + build_int_cst (itype, 1));
> > +
> > + tree lhs_vtype = get_mask_type_for_scalar_type (vinfo, itype);
> > + if (lhs_vtype == NULL_TREE)
> > + return NULL;
> > +
> > + append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt, lhs_vtype,
> > + itype);
> > + lhs = ilhs;
> > + break;
> > + }
> > + default:
> > + return NULL;
> > + }
> > + }
> >
> > tree vecitype = get_vectype_for_scalar_type (vinfo, scalar_type);
> > if (vecitype == NULL_TREE)
> > @@ -6208,7 +6245,6 @@ vect_recog_bool_pattern (vec_info *vinfo,
> > return NULL;
> > }
> >
> > -
> > /* A helper for vect_recog_mask_conversion_pattern. Build
> > conversion of MASK to a type suitable for masking VECTYPE.
> > Built statement gets required vectype and is appended to
> >
> >
> >
> >
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
