From: Philipp Tomsich <philipp.tomsich@vrull.eu>
To: Di Zhao OS <dizhao@os.amperecomputing.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PING][PATCH] tree-optimization/110279- Check for nested FMA chains in reassoc
Date: Fri, 7 Jul 2023 16:31:40 +0200 [thread overview]
Message-ID: <CAAeLtUArJvNhTtZ7f+2JPPS6R0saEbmddgrrAs1wsLcXSZuu+Q@mail.gmail.com> (raw)
In-Reply-To: <SN6PR01MB4240BCADC30D5B7F3CB37C97E82DA@SN6PR01MB4240.prod.exchangelabs.com>
On Fri, 7 Jul 2023 at 10:28, Di Zhao OS via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Update the patch so it can apply.
>
> Tested on spec2017 fprate cases again. With option "-funroll-loops -Ofast -flto",
> the improvements of 1-copy run are:
>
> Ampere1:
> 508.namd_r 4.26%
> 510.parest_r 2.55%
> Overall 0.54%
> Intel Xeon:
> 503.bwaves_r 1.3%
> 508.namd_r 1.58%
> overall 0.42%
This looks like a worthwhile improvement.
From reviewing the patch, a few nit-picks:
- given that 'has_fma' can now take three values { -1, 0, 1 }, an enum
with more descriptive names for these 3 states should be used;
- using "has_fma >= 0" and "fma > 0" tests are hard to read; after
changing this to an enum, you can use macros or helper functions to
test the predicates (i.e., *_P macros or *_p helpers) for readability
- the meaning of the return values of rank_ops_for_fma should be
documented in the comment describing the function
- changing convert_mult_to_fma_1 to return a tree* (i.e., return_lhs
or NULL_TREE) removes the need for an in/out parameter
Thanks,
Philipp.
>
>
> Thanks,
> Di Zhao
>
>
> > -----Original Message-----
> > From: Di Zhao OS
> > Sent: Friday, June 16, 2023 4:51 PM
> > To: gcc-patches@gcc.gnu.org
> > Subject: [PATCH] tree-optimization/110279- Check for nested FMA chains in
> > reassoc
> >
> > This patch is to fix the regressions found in SPEC2017 fprate cases
> > on aarch64.
> >
> > 1. Reused code in pass widening_mul to check for nested FMA chains
> > (those connected by MULT_EXPRs), since re-writing to parallel
> > generates worse codes.
> >
> > 2. Avoid re-arrange to produce less FMA chains that can be slow.
> >
> > Tested on ampere1 and neoverse-n1, this fixed the regressions in
> > 508.namd_r and 510.parest_r 1 copy run. While I'm still collecting data
> > on x86 machines we have, I'd like to know what do you think of this.
> >
> > (Previously I tried to improve things with FMA by adding a widening_mul
> > pass before reassoc2 for it's easier to recognize different patterns
> > of FMA chains and decide whether to split them. But I suppose handling
> > them all in reassoc pass is more efficient.)
> >
> > Thanks,
> > Di Zhao
> >
> > ---
> > gcc/ChangeLog:
> >
> > * tree-ssa-math-opts.cc (convert_mult_to_fma_1): Add new parameter.
> > Support new mode that merely do the checking.
> > (struct fma_transformation_info): Moved to header.
> > (class fma_deferring_state): Moved to header.
> > (convert_mult_to_fma): Add new parameter.
> > * tree-ssa-math-opts.h (struct fma_transformation_info):
> > (class fma_deferring_state): Moved from .cc.
> > (convert_mult_to_fma): Add function decl.
> > * tree-ssa-reassoc.cc (rewrite_expr_tree_parallel):
> > (rank_ops_for_fma): Return -1 if nested FMAs are found.
> > (reassociate_bb): Avoid rewriting to parallel if nested FMAs are
> > found.
>
prev parent reply other threads:[~2023-07-07 14:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-16 8:50 [PATCH] " Di Zhao OS
2023-07-07 8:26 ` [PING][PATCH] " Di Zhao OS
2023-07-07 14:31 ` Philipp Tomsich [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=CAAeLtUArJvNhTtZ7f+2JPPS6R0saEbmddgrrAs1wsLcXSZuu+Q@mail.gmail.com \
--to=philipp.tomsich@vrull.eu \
--cc=dizhao@os.amperecomputing.com \
--cc=gcc-patches@gcc.gnu.org \
/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).