public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: Tamar Christina <Tamar.Christina@arm.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>, nd <nd@arm.com>
Subject: RE: [PATCH]middle-end: don't lower past veclower [PR106063]
Date: Thu, 7 Jul 2022 07:47:16 +0000 (UTC)	[thread overview]
Message-ID: <nycvar.YFH.7.77.849.2207070745250.14950@jbgna.fhfr.qr> (raw)
In-Reply-To: <VI1PR08MB53256CE131AEDEE73EC1B2CBFF839@VI1PR08MB5325.eurprd08.prod.outlook.com>

On Thu, 7 Jul 2022, Tamar Christina wrote:

> > -----Original Message-----
> > From: Richard Biener <rguenther@suse.de>
> > Sent: Thursday, July 7, 2022 8:19 AM
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> > Subject: Re: [PATCH]middle-end: don't lower past veclower [PR106063]
> > 
> > On Tue, 5 Jul 2022, Tamar Christina wrote:
> > 
> > > Hi All,
> > >
> > > My previous patch can cause a problem if the pattern matches after
> > > veclower as it may replace the construct with a vector sequence which
> > > the target may not directly support.
> > >
> > > As such don't perform the rewriting if after veclower.
> > 
> > Note that when doing the rewriting before veclower to a variant not
> > supported by the target can cause veclower to generate absymal code.  In
> > some cases we are very careful and try to at least preserve code supported
> > by the target over transforming that into a variant not supported.
> > 
> > That said, a better fix would be to check whether the target can perform the
> > new comparison.  Before veclower it would be OK to do the transform
> > nevertheless in case it cannot do the original transform.
> 
> This last statement is somewhat confusing. Did you want me to change it such that
> before veclower the rewrite is always done and after veclowering only if the target
> supports it?
> 
> Or did you want me to never do the rewrite if the target doesn't support it?

I meant before veclower you can do the rewrite if either the rewriting
result is supported by the target OR if the original expression is
_not_ supported by the target.  The latter case might be not too
important to worry doing (it would still canonicalize for those
targets then).  After veclower you can only rewrite under the former
condition.

Richard.

> Thanks,
> Tamar
> 
> > 
> > Richard.
> > 
> > > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> > > and no issues.
> > >
> > > Ok for master? and backport to GCC 12?
> > >
> > > Thanks,
> > > Tamar
> > >
> > >
> > > gcc/ChangeLog:
> > >
> > > 	PR tree-optimization/106063
> > > 	* match.pd: Do not apply pattern after veclower.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 	PR tree-optimization/106063
> > > 	* gcc.dg/pr106063.c: New test.
> > >
> > > --- inline copy of patch --
> > > diff --git a/gcc/match.pd b/gcc/match.pd index
> > >
> > 40c09bedadb89dabb6622559a8f69df5384e61fd..ba161892a98756c0278dc40fc
> > 377
> > > d7d0deaacbcf 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -6040,7 +6040,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >    (simplify
> > >     (cmp (bit_and:c@2 @0 cst@1) integer_zerop)
> > >      (with { tree csts = bitmask_inv_cst_vector_p (@1); }
> > > -     (if (csts && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2)))
> > > +     (if (csts && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2))
> > > +	  && optimize_vectors_before_lowering_p ())
> > >        (if (TYPE_UNSIGNED (TREE_TYPE (@1)))
> > >         (icmp @0 { csts; })
> > >         (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
> > > diff --git a/gcc/testsuite/gcc.dg/pr106063.c
> > > b/gcc/testsuite/gcc.dg/pr106063.c new file mode 100644 index
> > >
> > 0000000000000000000000000000000000000000..b23596724f6bb98c53af2dce77
> > d3
> > > 1509bab10378
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/pr106063.c
> > > @@ -0,0 +1,9 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -fno-tree-forwprop --disable-tree-evrp" } */
> > > +typedef __int128 __attribute__((__vector_size__ (16))) V;
> > > +
> > > +V
> > > +foo (V v)
> > > +{
> > > +  return (v & (V){15}) == v;
> > > +}
> > >
> > >
> > >
> > >
> > >
> > 
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH, Frankenstra
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstra

  reply	other threads:[~2022-07-07  7:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-05 15:00 Tamar Christina
2022-07-06 13:14 ` Jeff Law
2022-07-07  7:18 ` Richard Biener
2022-07-07  7:41   ` Tamar Christina
2022-07-07  7:47     ` Richard Biener [this message]
2022-07-08  6:26       ` Tamar Christina
2022-07-08  7:16         ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.YFH.7.77.849.2207070745250.14950@jbgna.fhfr.qr \
    --to=rguenther@suse.de \
    --cc=Tamar.Christina@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).