From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 74D94383581A for ; Mon, 6 Sep 2021 12:40:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 74D94383581A Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 595F622125; Mon, 6 Sep 2021 12:40:32 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 12258A3B99; Mon, 6 Sep 2021 12:40:32 +0000 (UTC) Date: Mon, 6 Sep 2021 14:40:31 +0200 (CEST) From: Richard Biener To: Matthias Kretz cc: gcc-patches@gcc.gnu.org, jason@redhat.com, "Joseph S. Myers" Subject: Re: [PATCH v2] c-family: Add __builtin_assoc_barrier In-Reply-To: <1911968.eP8hAHRRDn@excalibur> Message-ID: References: <2312888.R4OtGj9PrH@excalibur> <1911968.eP8hAHRRDn@excalibur> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Sep 2021 12:40:35 -0000 On Mon, 6 Sep 2021, Matthias Kretz wrote: > Hi, > > On Tuesday, 20 July 2021 22:22:02 CEST Jason Merrill wrote: > > The C++ front end already uses PAREN_EXPR in templates to indicate > > parenthesized initializers in cases where that matters for > > decltype(auto). It should be fine to use it for both that and > > __builtin_assoc_barrier, but you probably want to distinguish them with > > a TREE_LANG_FLAG, and change tsubst_copy_and_build to keep the > > PAREN_EXPR in this case. > > I reused REF_PARENTHESIZED_P for PAREN_EXPR. > > > For constexpr you probably just need to add handling to > > cxx_eval_constant_expression to evaluate its operand instead. > > OK, that was easy. > > On Monday, 19 July 2021 14:34:12 CEST Richard Biener wrote: > > On Mon, 19 Jul 2021, Matthias Kretz wrote: > > > tested on x86_64-pc-linux-gnu with no new failures. OK for master? > > > > I think now that PAREN_EXPR can appear in C++ code you need to > > adjust some machiner to expect it (constexpr folding? template stuff?). > > I suggest to add some testcases covering templates and constexpr > > functions. > > Right. I expanded the test. > > > +@deftypefn {Built-in Function} @var{type} __builtin_assoc_barrier > > (@var{type} @var{expr}) > > +This built-in represents a re-association barrier for the floating-point > > +expression @var{expr} with operations following the built-in. The > > expression > > +@var{expr} itself can be reordered, and the whole expression @var{expr} > > can > > be > > +reordered with operations after the barrier. > > > > What operations follow the built-in also applies to operations leading > > the builtin? Maybe "This built-in represents a re-association barrier > > for the floating-point expression @var{expr} with the expression > > consuming its value." But I'm not an english speaker - I guess > > I'm mostly confused about "follow" here. > > With "follow" I meant time / precedence and not that the operation follows > syntactically. So e.g. a + b * c: the addition follows after the > multiplication. It's probably not as precise as it could/should be. Also "the > whole expression @var{expr} can be reordered with operations after the > barrier" probably should say "with operands" not "with operations", right? > > > I'm not sure if there are better C/C++ language terms describing what > > the builtin does, but basically it appears as opaque operand to the > > surrounding expression and the surrounding expression is opaque > > to the expression inside the parens. > > I can't think of any other term that would help here. > > Based upon your suggestion, the attached patch now says: > "This built-in inhibits re-association of the floating-point expression > @var{expr} with expressions consuming the return value of the built-in. The > expression @var{expr} itself can be reordered, and the whole expression > @var{expr} can be reordered with operands after the barrier. [...]" > > New patch attached. OK to push? OK from my side, I'll leave the C++ frontend parts to Jason. I'll note that currently a + PAREN_EXPR (b * c) is for example also not contracted to PAREN_EXPR (FMA (PAREN_EXPR (a), b, c)) even though technically FP contraction is not association. But that's an implementation detail that could be changed. There are likely other transforms that it prevents as well that are not assocations, the implementation focus was correctness as to preventing association, not so much not hindering unrelated optimizations. If you run into any such issues reporting a bugzilla would be welcome. Thanks, Richard. > ----------------------------------------------------------- > > New builtin to enable explicit use of PAREN_EXPR in C & C++ code. > > Signed-off-by: Matthias Kretz > > gcc/testsuite/ChangeLog: > > * c-c++-common/builtin-assoc-barrier-1.c: New test. > > gcc/cp/ChangeLog: > > * constexpr.c (cxx_eval_constant_expression): Handle PAREN_EXPR > via cxx_eval_constant_expression. > * cp-objcp-common.c (names_builtin_p): Handle > RID_BUILTIN_ASSOC_BARRIER. > * cp-tree.h: Adjust TREE_LANG_FLAG documentation to include > PAREN_EXPR in REF_PARENTHESIZED_P. > (REF_PARENTHESIZED_P): Add PAREN_EXPR. > * parser.c (cp_parser_postfix_expression): Handle > RID_BUILTIN_ASSOC_BARRIER. > * pt.c (tsubst_copy_and_build): If the PAREN_EXPR is not a > parenthesized initializer, evaluate by ignoring the PAREN_EXPR. > * semantics.c (force_paren_expr): Simplify conditionals. Set > REF_PARENTHESIZED_P on PAREN_EXPR. > (maybe_undo_parenthesized_ref): Test PAREN_EXPR for > REF_PARENTHESIZED_P. > > gcc/c-family/ChangeLog: > > * c-common.c (c_common_reswords): Add __builtin_assoc_barrier. > * c-common.h (enum rid): Add RID_BUILTIN_ASSOC_BARRIER. > > gcc/c/ChangeLog: > > * c-decl.c (names_builtin_p): Handle RID_BUILTIN_ASSOC_BARRIER. > * c-parser.c (c_parser_postfix_expression): Likewise. > > gcc/ChangeLog: > > * doc/extend.texi: Document __builtin_assoc_barrier. > --- > gcc/c-family/c-common.c | 1 + > gcc/c-family/c-common.h | 2 +- > gcc/c/c-decl.c | 1 + > gcc/c/c-parser.c | 20 ++++++++ > gcc/cp/constexpr.c | 6 +++ > gcc/cp/cp-objcp-common.c | 1 + > gcc/cp/cp-tree.h | 12 +++-- > gcc/cp/parser.c | 14 ++++++ > gcc/cp/pt.c | 5 +- > gcc/cp/semantics.c | 23 +++------ > gcc/doc/extend.texi | 18 +++++++ > .../c-c++-common/builtin-assoc-barrier-1.c | 48 +++++++++++++++++++ > 12 files changed, 128 insertions(+), 23 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/builtin-assoc-barrier-1.c > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)