From: Alexander Monakov <amonakov@ispras.ru>
To: Marek Polacek <polacek@redhat.com>, Jason Merrill <jason@redhat.com>
Cc: Richard Biener <richard.guenther@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c-family: implement -ffp-contract=on
Date: Mon, 19 Jun 2023 20:03:22 +0300 (MSK) [thread overview]
Message-ID: <2207f489-f600-2a05-0741-5733b4a6f49a@ispras.ru> (raw)
In-Reply-To: <69ad69cc-90bd-5323-2be5-d3db80acd1a7@ispras.ru>
[-- Attachment #1: Type: text/plain, Size: 9097 bytes --]
Ping. OK for trunk?
On Mon, 5 Jun 2023, Alexander Monakov wrote:
> Ping for the front-end maintainers' input.
>
> On Mon, 22 May 2023, Richard Biener wrote:
>
> > On Thu, May 18, 2023 at 11:04 PM Alexander Monakov via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > Implement -ffp-contract=on for C and C++ without changing default
> > > behavior (=off for -std=cNN, =fast for C++ and -std=gnuNN).
> >
> > The documentation changes mention the defaults are changed for
> > standard modes, I suppose you want to remove that hunk.
> >
> > > gcc/c-family/ChangeLog:
> > >
> > > * c-gimplify.cc (fma_supported_p): New helper.
> > > (c_gimplify_expr) [PLUS_EXPR, MINUS_EXPR]: Implement FMA
> > > contraction.
> > >
> > > gcc/ChangeLog:
> > >
> > > * common.opt (fp_contract_mode) [on]: Remove fallback.
> > > * config/sh/sh.md (*fmasf4): Correct flag_fp_contract_mode test.
> > > * doc/invoke.texi (-ffp-contract): Update.
> > > * trans-mem.cc (diagnose_tm_1): Skip internal function calls.
> > > ---
> > > gcc/c-family/c-gimplify.cc | 78 ++++++++++++++++++++++++++++++++++++++
> > > gcc/common.opt | 3 +-
> > > gcc/config/sh/sh.md | 2 +-
> > > gcc/doc/invoke.texi | 8 ++--
> > > gcc/trans-mem.cc | 3 ++
> > > 5 files changed, 88 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/gcc/c-family/c-gimplify.cc b/gcc/c-family/c-gimplify.cc
> > > index ef5c7d919f..f7635d3b0c 100644
> > > --- a/gcc/c-family/c-gimplify.cc
> > > +++ b/gcc/c-family/c-gimplify.cc
> > > @@ -41,6 +41,8 @@ along with GCC; see the file COPYING3. If not see
> > > #include "c-ubsan.h"
> > > #include "tree-nested.h"
> > > #include "context.h"
> > > +#include "tree-pass.h"
> > > +#include "internal-fn.h"
> > >
> > > /* The gimplification pass converts the language-dependent trees
> > > (ld-trees) emitted by the parser into language-independent trees
> > > @@ -686,6 +688,14 @@ c_build_bind_expr (location_t loc, tree block, tree body)
> > > return bind;
> > > }
> > >
> > > +/* Helper for c_gimplify_expr: test if target supports fma-like FN. */
> > > +
> > > +static bool
> > > +fma_supported_p (enum internal_fn fn, tree type)
> > > +{
> > > + return direct_internal_fn_supported_p (fn, type, OPTIMIZE_FOR_BOTH);
> > > +}
> > > +
> > > /* Gimplification of expression trees. */
> > >
> > > /* Do C-specific gimplification on *EXPR_P. PRE_P and POST_P are as in
> > > @@ -739,6 +749,74 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
> > > break;
> > > }
> > >
> > > + case PLUS_EXPR:
> > > + case MINUS_EXPR:
> > > + {
> > > + tree type = TREE_TYPE (*expr_p);
> > > + /* For -ffp-contract=on we need to attempt FMA contraction only
> > > + during initial gimplification. Late contraction across statement
> > > + boundaries would violate language semantics. */
> > > + if (SCALAR_FLOAT_TYPE_P (type)
> > > + && flag_fp_contract_mode == FP_CONTRACT_ON
> > > + && cfun && !(cfun->curr_properties & PROP_gimple_any)
> > > + && fma_supported_p (IFN_FMA, type))
> > > + {
> > > + bool neg_mul = false, neg_add = code == MINUS_EXPR;
> > > +
> > > + tree *op0_p = &TREE_OPERAND (*expr_p, 0);
> > > + tree *op1_p = &TREE_OPERAND (*expr_p, 1);
> > > +
> > > + /* Look for ±(x * y) ± z, swapping operands if necessary. */
> > > + if (TREE_CODE (*op0_p) == NEGATE_EXPR
> > > + && TREE_CODE (TREE_OPERAND (*op0_p, 0)) == MULT_EXPR)
> > > + /* '*EXPR_P' is '-(x * y) ± z'. This is fine. */;
> > > + else if (TREE_CODE (*op0_p) != MULT_EXPR)
> > > + {
> > > + std::swap (op0_p, op1_p);
> > > + std::swap (neg_mul, neg_add);
> > > + }
> > > + if (TREE_CODE (*op0_p) == NEGATE_EXPR)
> > > + {
> > > + op0_p = &TREE_OPERAND (*op0_p, 0);
> > > + neg_mul = !neg_mul;
> > > + }
> > > + if (TREE_CODE (*op0_p) != MULT_EXPR)
> > > + break;
> > > + auto_vec<tree, 3> ops (3);
> > > + ops.quick_push (TREE_OPERAND (*op0_p, 0));
> > > + ops.quick_push (TREE_OPERAND (*op0_p, 1));
> > > + ops.quick_push (*op1_p);
> > > +
> > > + enum internal_fn ifn = IFN_FMA;
> > > + if (neg_mul)
> > > + {
> > > + if (fma_supported_p (IFN_FNMA, type))
> > > + ifn = IFN_FNMA;
> > > + else
> > > + ops[0] = build1 (NEGATE_EXPR, type, ops[0]);
> > > + }
> > > + if (neg_add)
> > > + {
> > > + enum internal_fn ifn2 = ifn == IFN_FMA ? IFN_FMS : IFN_FNMS;
> > > + if (fma_supported_p (ifn2, type))
> > > + ifn = ifn2;
> > > + else
> > > + ops[2] = build1 (NEGATE_EXPR, type, ops[2]);
> > > + }
> > > + for (auto &&op : ops)
> > > + if (gimplify_expr (&op, pre_p, post_p, is_gimple_val, fb_rvalue)
> > > + == GS_ERROR)
> > > + return GS_ERROR;
> > > +
> > > + gcall *call = gimple_build_call_internal_vec (ifn, ops);
> > > + gimple_seq_add_stmt_without_update (pre_p, call);
> > > + *expr_p = create_tmp_var (type);
> > > + gimple_call_set_lhs (call, *expr_p);
> >
> > it would be possible to do
> >
> > *expr_p = build_call_expr_internal (ifn, type, ops[0], ops[1]. ops[2]);
> > return GS_OK;
> >
> > and not worry about temporary creation and gimplifying of the operands.
> > That would in theory also leave the possibility to do this during
> > genericization instead (and avoid the guard against late invocation of
> > the hook).
> >
> > Otherwise it looks OK, but I'll let frontend maintainers have a chance to look
> > as well.
> >
> > Thanks for tackling this long-standing issue.
> > Richard.
> >
> > > + return GS_ALL_DONE;
> > > + }
> > > + break;
> > > + }
> > > +
> > > default:;
> > > }
> > >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index a28ca13385..3daec85aef 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -1662,9 +1662,8 @@ Name(fp_contract_mode) Type(enum fp_contract_mode) UnknownError(unknown floating
> > > EnumValue
> > > Enum(fp_contract_mode) String(off) Value(FP_CONTRACT_OFF)
> > >
> > > -; Not implemented, fall back to conservative FP_CONTRACT_OFF.
> > > EnumValue
> > > -Enum(fp_contract_mode) String(on) Value(FP_CONTRACT_OFF)
> > > +Enum(fp_contract_mode) String(on) Value(FP_CONTRACT_ON)
> > >
> > > EnumValue
> > > Enum(fp_contract_mode) String(fast) Value(FP_CONTRACT_FAST)
> > > diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
> > > index 4622dba012..5cb1795482 100644
> > > --- a/gcc/config/sh/sh.md
> > > +++ b/gcc/config/sh/sh.md
> > > @@ -9269,7 +9269,7 @@ (define_insn_and_split "*fmasf4"
> > > (match_operand:SF 3 "arith_reg_operand" "0")))
> > > (clobber (reg:SI FPSCR_STAT_REG))
> > > (use (reg:SI FPSCR_MODES_REG))]
> > > - "TARGET_SH2E && flag_fp_contract_mode != FP_CONTRACT_OFF"
> > > + "TARGET_SH2E && flag_fp_contract_mode == FP_CONTRACT_FAST"
> > > "fmac %1,%2,%0"
> > > "&& can_create_pseudo_p ()"
> > > [(parallel [(set (match_dup 0)
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index b92b857602..cb1e9a1d9f 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -11983,10 +11983,12 @@ This option is enabled by default at optimization levels @option{-O1},
> > > such as forming of fused multiply-add operations if the target has
> > > native support for them.
> > > @option{-ffp-contract=on} enables floating-point expression contraction
> > > -if allowed by the language standard. This is currently not implemented
> > > -and treated equal to @option{-ffp-contract=off}.
> > > +if allowed by the language standard. This is implemented for C and C++,
> > > +where it enables contraction within one expression, but not across
> > > +different statements.
> > >
> > > -The default is @option{-ffp-contract=fast}.
> > > +The default is @option{-ffp-contract=off} for C in a standards compliant mode
> > > +(@option{-std=c11} or similar), @option{-ffp-contract=fast} otherwise.
> > >
> > > @opindex fomit-frame-pointer
> > > @item -fomit-frame-pointer
> > > diff --git a/gcc/trans-mem.cc b/gcc/trans-mem.cc
> > > index 4b129663e0..2174faef4c 100644
> > > --- a/gcc/trans-mem.cc
> > > +++ b/gcc/trans-mem.cc
> > > @@ -637,6 +637,9 @@ diagnose_tm_1 (gimple_stmt_iterator *gsi, bool *handled_ops_p,
> > > {
> > > case GIMPLE_CALL:
> > > {
> > > + if (gimple_call_internal_p (stmt))
> > > + break;
> > > +
> > > tree fn = gimple_call_fn (stmt);
> > >
> > > if ((d->summary_flags & DIAG_TM_OUTER) == 0
> > > --
> > > 2.39.2
> > >
> >
next prev parent reply other threads:[~2023-06-19 17:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-18 21:03 Alexander Monakov
2023-05-22 10:02 ` Richard Biener
2023-05-22 15:16 ` Alexander Monakov
2023-05-23 6:05 ` Richard Biener
2023-05-23 11:21 ` Alexander Monakov
2023-06-05 15:33 ` Alexander Monakov
2023-06-19 17:03 ` Alexander Monakov [this message]
2023-06-19 18:23 ` 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=2207f489-f600-2a05-0741-5733b4a6f49a@ispras.ru \
--to=amonakov@ispras.ru \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=polacek@redhat.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).