This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Adjust PR70251 fix
- From: Richard Biener <rguenther at suse dot de>
- To: Marc Glisse <marc dot glisse at inria dot fr>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 23 Mar 2016 09:38:35 +0100 (CET)
- Subject: Re: [PATCH] Adjust PR70251 fix
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1603221530090 dot 15580 at t29 dot fhfr dot qr> <alpine dot DEB dot 2 dot 20 dot 1603221634370 dot 3529 at laptop-mg dot saclay dot inria dot fr> <2BA58033-713C-4472-A447-F93B584EC208 at suse dot de> <alpine dot DEB dot 2 dot 20 dot 1603221756260 dot 3529 at laptop-mg dot saclay dot inria dot fr>
On Tue, 22 Mar 2016, Marc Glisse wrote:
> On Tue, 22 Mar 2016, Richard Biener wrote:
>
> > On March 22, 2016 4:55:13 PM GMT+01:00, Marc Glisse <marc.glisse@inria.fr>
> > wrote:
> > > On Tue, 22 Mar 2016, Richard Biener wrote:
> > >
> > > >
> > > > This adjusts the PR70251 fix as discussed in the PR audit trail
> > > > and fixes a bug in genmatch required (bah, stupid GENERIC comparisons
> > > in
> > > > GIMPLE operands...).
> > >
> > > Thanks !
> > >
> > > Hmm, the transformation is still disabled on AVX512:
> > >
> > > > ! /* A + (B vcmp C ? 1 : 0) -> A - (B vcmp C ? -1 : 0), since vector
> > > comparisons
> > > > ! return all -1 or all 0 results. */
> > > > /* ??? We could instead convert all instances of the vec_cond to
> > > negate,
> > > > but that isn't necessarily a win on its own. */
> > > > (simplify
> > > > ! (plus:c @3 (view_convert? (vec_cond:s @0 integer_each_onep@1
> > > integer_zerop@2)))
> > > > (if (VECTOR_TYPE_P (type)
> > > > && TYPE_VECTOR_SUBPARTS (type) == TYPE_VECTOR_SUBPARTS
> > > (TREE_TYPE (@0))
> > > > && (TYPE_MODE (TREE_TYPE (type))
> > > > == TYPE_MODE (TREE_TYPE (TREE_TYPE (@0)))))
> > > > ! (minus @3 (view_convert (vec_cond @0 (negate @1) @2)))))
> > >
> > > It seems that the references to @0 in the "if" should use @1 instead
> > > (at
> > > least the last one). I assume this test is to make sure that A has as
> > > many
> > > integer elements of the same size as the result of the vec_cond_expr.
> >
> > It looks like that is always guaranteed by the input form and instead these
> > are now useless checks which were guarding the original view-convert
> > transform.
>
> The input form has a view_convert_expr in it, so I don't see what prevents
> from arriving here with
>
> v4df + view_convert((v8si < v8si) ? v8si : v8si)
>
> for instance. That seems to indicate that some test is still needed, it is
> just better on the second or third argument of the vec_cond_expr than on the
> condition.
Hmm, indeed.
> Or maybe you mean we could drop the view_convert_expr from the input form. It
> should have been sunk in the 2 constant arguments of the vec_cond_expr anyway
> (I didn't check if that really happens). That sounds good.
I think it originally was intended to cover sign changes that the
vectorizer generates eventually. Those may still happen but yes,
we could sink them into the vec_cond. Leaving this (and the
consideration on whether a ? -1 : 0 should be transformed to
view_convert <a>) to GCC 7 time.
Bootstrapping / testing the following now.
Thanks for noticing,
Richard.
2016-03-23 Richard Biener <rguenther@suse.de>
PR middle-end/70251
* match.pd (A + (B vcmp C ? 1 : 0) -> A - (B vcmp C)): Adjust
mode compatibility check.
(A - (B vcmp C ? 1 : 0) -> A + (B vcmp C)): Likewise.
Index: gcc/match.pd
===================================================================
--- gcc/match.pd (revision 234415)
+++ gcc/match.pd (working copy)
@@ -1759,18 +1759,18 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(simplify
(plus:c @3 (view_convert? (vec_cond:s @0 integer_each_onep@1 integer_zerop@2)))
(if (VECTOR_TYPE_P (type)
- && TYPE_VECTOR_SUBPARTS (type) == TYPE_VECTOR_SUBPARTS (TREE_TYPE (@0))
+ && TYPE_VECTOR_SUBPARTS (type) == TYPE_VECTOR_SUBPARTS (TREE_TYPE (@1))
&& (TYPE_MODE (TREE_TYPE (type))
- == TYPE_MODE (TREE_TYPE (TREE_TYPE (@0)))))
+ == TYPE_MODE (TREE_TYPE (TREE_TYPE (@1)))))
(minus @3 (view_convert (vec_cond @0 (negate @1) @2)))))
/* ... likewise A - (B vcmp C ? 1 : 0) -> A + (B vcmp C ? -1 : 0). */
(simplify
(minus @3 (view_convert? (vec_cond:s @0 integer_each_onep@1 integer_zerop@2)))
(if (VECTOR_TYPE_P (type)
- && TYPE_VECTOR_SUBPARTS (type) == TYPE_VECTOR_SUBPARTS (TREE_TYPE (@0))
+ && TYPE_VECTOR_SUBPARTS (type) == TYPE_VECTOR_SUBPARTS (TREE_TYPE (@1))
&& (TYPE_MODE (TREE_TYPE (type))
- == TYPE_MODE (TREE_TYPE (TREE_TYPE (@0)))))
+ == TYPE_MODE (TREE_TYPE (TREE_TYPE (@1)))))
(plus @3 (view_convert (vec_cond @0 (negate @1) @2)))))