public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Pinski <pinskia@gmail.com>
To: Jeff Law <jeffreyalaw@gmail.com>
Cc: Andrew Pinski <quic_apinski@quicinc.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] PHIOPT: move factor_out_conditional_operation over to use gimple_match_op
Date: Sun, 18 Aug 2024 16:06:42 -0700	[thread overview]
Message-ID: <CA+=Sn1nZy535mwTzmbsS0XcMKP5L4gz7SARA-SQZr-O+6-uSDQ@mail.gmail.com> (raw)
In-Reply-To: <b7c62046-5522-4edf-b634-8f93f41671e3@gmail.com>

On Sun, Aug 18, 2024 at 11:06 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 8/16/24 8:13 PM, Andrew Pinski wrote:
> > To start working on more with expressions with more than one operand, converting
> > over to use gimple_match_op is needed.
> > The added side-effect here is factor_out_conditional_operation can now support
> > builtins/internal calls that has one operand without any extra code added.
> >
> > Note on the changed testcases:
> > * pr87007-5.c: the test was testing testing for avoiding partial register stalls
> > for the sqrt and making sure there is only one zero of the register before the
> > branch, the phiopt would now merge the sqrt's so disable phiopt.
> >
> > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> >       * gimple-match-exports.cc (gimple_match_op::operands_occurs_in_abnormal_phi):
> >       New function.
> >       * gimple-match.h (gimple_match_op): Add operands_occurs_in_abnormal_phi.
> >       * tree-ssa-phiopt.cc (factor_out_conditional_operation): Use gimple_match_op
> >       instead of manually extracting from/creating the gimple.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.target/i386/pr87007-5.c: Disable phi-opt.
> >
>
> > diff --git a/gcc/testsuite/gcc.target/i386/pr87007-5.c b/gcc/testsuite/gcc.target/i386/pr87007-5.c
> > index 8f2dc947f6c..1a240adef63 100644
> > --- a/gcc/testsuite/gcc.target/i386/pr87007-5.c
> > +++ b/gcc/testsuite/gcc.target/i386/pr87007-5.c
> > @@ -1,8 +1,11 @@
> >   /* { dg-do compile } */
> > -/* { dg-options "-Ofast -march=skylake-avx512 -mfpmath=sse -fno-tree-vectorize -fdump-tree-cddce3-details -fdump-tree-lsplit-optimized" } */
> > +/* { dg-options "-Ofast -march=skylake-avx512 -mfpmath=sse -fno-tree-vectorize -fdump-tree-cddce3-details -fdump-tree-lsplit-optimized -fno-ssa-phiopt" } */
> >   /* Load of d2/d3 is hoisted out, the loop is split, store of d1 and sqrt
> >      are sunk out of the loop and the loop is elided.  One vsqrtsd with
> >      memory operand needs a xor to avoid partial dependence.  */
> > +/* Phi-OPT needs to ne disabled otherwise, sqrt calls are merged which is better
> > +   but we are testing to make sure the partial register stall for SSE is still avoided
> > +   for sqrts.  */
> Nit.  s/to ne/to be/g
>
> OK with the nit fixed.
>
> Note this is getting closer to doing generalized sinking a common op
> through PHI nodes which is something we've wanted for a long time.

Yes that is the plan; I just want to do it in steps as I have a few
other projects in progress; and I don't know how much of each I will
be able to get done in time for GCC 15. I originally had this done
differently but I thought it would be better to reuse infrastructure
that was already there instead of creating new ones.
I had implemented this patch back in April and I didn't know if I
could get to the rest due to other projects going on so I submitted it
finally.

Thanks,
Andrew

>
> Jeff
>

      reply	other threads:[~2024-08-18 23:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-17  2:13 Andrew Pinski
2024-08-18 18:05 ` Jeff Law
2024-08-18 23:06   ` Andrew Pinski [this message]

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='CA+=Sn1nZy535mwTzmbsS0XcMKP5L4gz7SARA-SQZr-O+6-uSDQ@mail.gmail.com' \
    --to=pinskia@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    --cc=quic_apinski@quicinc.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).