From: Hongtao Liu <crazylht@gmail.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: liuhongt <hongtao.liu@intel.com>,
gcc-patches@gcc.gnu.org, hjl.tools@gmail.com
Subject: Re: [V2 PATCH] Handle bitop with INTEGER_CST in analyze_and_compute_bitop_with_inv_effect.
Date: Tue, 7 Nov 2023 21:03:22 +0800 [thread overview]
Message-ID: <CAMZc-bx99L8cOsyFgeyh+3wgmkRMyYORq7Rt843YLDigPR3wbg@mail.gmail.com> (raw)
In-Reply-To: <CAFiYyc3+X9uAZ3yWiDCFu-HTuSZcfdvVhcybfd3B50U+Big-Sg@mail.gmail.com>
On Tue, Nov 7, 2023 at 4:10 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Nov 7, 2023 at 7:08 AM liuhongt <hongtao.liu@intel.com> wrote:
> >
> > analyze_and_compute_bitop_with_inv_effect assumes the first operand is
> > loop invariant which is not the case when it's INTEGER_CST.
> >
> > Bootstrapped and regtseted on x86_64-pc-linux-gnu{-m32,}.
> > Ok for trunk?
>
> So this addresses a missed optimization, right? It seems to me that
> even with two SSA names we are only "lucky" when rhs1 is the invariant
> one. So instead of swapping this way I'd do
Yes, it's a miss optimization.
And I think expr_invariant_in_loop_p (loop, match_op[1]) should be
enough, if match_op[1] is a loop invariant.it must be false for the
below conditions(there couldn't be any header_phi from its
definition).
>
> unsigned i;
> for (i = 0; i < 2; ++i)
> if (TREE_CODE (match_op[i]) == SSA_NAME
> && ...)
> break; /* found! */
>
> if (i == 2)
> return NULL_TREE;
> if (i == 0)
> std::swap (match_op[0], match_op[1]);
>
> to also handle a "swapped" pair of SSA names?
>
> > gcc/ChangeLog:
> >
> > PR tree-optimization/105735
> > PR tree-optimization/111972
> > * tree-scalar-evolution.cc
> > (analyze_and_compute_bitop_with_inv_effect): Handle bitop with
> > INTEGER_CST.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/i386/pr105735-3.c: New test.
> > ---
> > gcc/testsuite/gcc.target/i386/pr105735-3.c | 87 ++++++++++++++++++++++
> > gcc/tree-scalar-evolution.cc | 3 +
> > 2 files changed, 90 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr105735-3.c
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/pr105735-3.c b/gcc/testsuite/gcc.target/i386/pr105735-3.c
> > new file mode 100644
> > index 00000000000..9e268a1a997
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr105735-3.c
> > @@ -0,0 +1,87 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -fdump-tree-sccp-details" } */
> > +/* { dg-final { scan-tree-dump-times {final value replacement} 8 "sccp" } } */
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +foo (unsigned int tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit++)
> > + tmp &= 11304;
> > + return tmp;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +foo1 (unsigned int tmp)
> > +{
> > + for (int bit = 63; bit >= 0; bit -=3)
> > + tmp &= 11304;
> > + return tmp;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +foo2 (unsigned int tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit++)
> > + tmp |= 11304;
> > + return tmp;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +foo3 (unsigned int tmp)
> > +{
> > + for (int bit = 63; bit >= 0; bit -=3)
> > + tmp |= 11304;
> > + return tmp;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +foo4 (unsigned int tmp)
> > +{
> > + for (int bit = 0; bit < 64; bit++)
> > + tmp ^= 11304;
> > + return tmp;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +foo5 (unsigned int tmp)
> > +{
> > + for (int bit = 0; bit < 63; bit++)
> > + tmp ^= 11304;
> > + return tmp;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +f (unsigned int tmp, int bit)
> > +{
> > + unsigned int res = tmp;
> > + for (int i = 0; i < bit; i++)
> > + res &= 11304;
> > + return res;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +f1 (unsigned int tmp, int bit)
> > +{
> > + unsigned int res = tmp;
> > + for (int i = 0; i < bit; i++)
> > + res |= 11304;
> > + return res;
> > +}
> > +
> > +unsigned int
> > +__attribute__((noipa))
> > +f2 (unsigned int tmp, int bit)
> > +{
> > + unsigned int res = tmp;
> > + for (int i = 0; i < bit; i++)
> > + res ^= 11304;
> > + return res;
> > +}
> > diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> > index 70b17c5bca1..f61277c32df 100644
> > --- a/gcc/tree-scalar-evolution.cc
> > +++ b/gcc/tree-scalar-evolution.cc
> > @@ -3689,6 +3689,9 @@ analyze_and_compute_bitop_with_inv_effect (class loop* loop, tree phidef,
> > match_op[0] = gimple_assign_rhs1 (def);
> > match_op[1] = gimple_assign_rhs2 (def);
> >
> > + if (expr_invariant_in_loop_p (loop, match_op[1]))
> > + std::swap (match_op[0], match_op[1]);
> > +
> > if (TREE_CODE (match_op[1]) != SSA_NAME
> > || !expr_invariant_in_loop_p (loop, match_op[0])
> > || !(header_phi = dyn_cast <gphi *> (SSA_NAME_DEF_STMT (match_op[1])))
> > --
> > 2.31.1
> >
--
BR,
Hongtao
next prev parent reply other threads:[~2023-11-07 13:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-30 10:41 [PATCH] " liuhongt
2023-11-07 6:05 ` [V2 PATCH] " liuhongt
2023-11-07 8:10 ` Richard Biener
2023-11-07 13:03 ` Hongtao Liu [this message]
2023-11-07 14:34 ` Richard Biener
2023-11-08 1:18 ` Hongtao Liu
2023-11-08 7:53 ` Richard Biener
2023-11-08 8:22 ` Hongtao Liu
2023-11-10 9:12 ` Richard Biener
2023-11-13 7:58 ` Hongtao Liu
2023-11-13 11:52 ` 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=CAMZc-bx99L8cOsyFgeyh+3wgmkRMyYORq7Rt843YLDigPR3wbg@mail.gmail.com \
--to=crazylht@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
--cc=hongtao.liu@intel.com \
--cc=richard.guenther@gmail.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).