Message ID | 5d937aa3-c33d-4ade-8b7e-9fd0effd147d@www.fastmail.com |
---|---|
State | New, archived |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:ecc5:0:0:0:0:0 with SMTP id s5csp399477wro; Wed, 31 Aug 2022 14:00:40 -0700 (PDT) X-Google-Smtp-Source: AA6agR6Kfn73E4btS9TjaTF8kf2AGXNbCCX8SVT4EwkPtwSCUnxiigZxua4+WWkqEom8gq0coWl6 X-Received: by 2002:a17:906:7945:b0:73b:e605:f31 with SMTP id l5-20020a170906794500b0073be6050f31mr22005629ejo.129.1661979640139; Wed, 31 Aug 2022 14:00:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1661979640; cv=none; d=google.com; s=arc-20160816; b=dLc/Iem5XoFde/a3ta2az71u+3jSGrtVg4doZgkS1BtZw3ZeA2zrHbBqIu2lRT8cnP YbIesV3byUNK5YvG1XguOjH5V97Zi8O+kgswnKjA+3uwf/uD6HhJzVrl0mv0ZfvRT14y h4vgF92iTOocoSa6FpogHGyBunkSJjfPz3rzsDdP8xN9NO8AEzxE9mSUtfLWM5tjaQRy oPXuKQFf480a+iv4vEQDycy5GhuwUFBxmlcfXD6/mzBDqU+eAYsEQC7EwfSTHrKvLYX5 jOHfaLhHBz+g76FPazjmRPIjQmp6ZcqWFc9mJvkYZ7L583lRAtJrLZWKD0pKP75kJwWS Pzmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject:to:from:date:message-id :mime-version:user-agent:feedback-id:dkim-signature:dkim-signature :dmarc-filter:delivered-to; bh=XJsiwNxNJlxFMLsVqrRYEwthFevk40jMc3pp4hzFmZQ=; b=kg/p/diq5QdjxkqXziWZUZdxeyn7ZN9hGN9rsa3ZCkAaLmkdAEmd5yetzld3G3VrGQ nV+NFUDBot6E9V+SwRAzH7J2rTm0iJNc+oLjGjzlO/foo97AZc5wb6Z191ie2VLymrM7 JLJwu6hQXda1ALYYr08BZk3fVCkw+VsyVJoEEL2uB/PfIgg/7ZEA3wQb27F4oXfcsq/C Hzfc+uFz4el7l9+ezJG8hk+HnpFB1Ox+C/V3HFVFBh8iQ8xVuRQMe02BErUpTWs8LpXU ZA5TG6irEehDvPTdm6EJJG9en2vofdXpKVQP5bFTtaRqsgcP5ZuyAsQTqDFjLwzTq1Np qHqw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@vvalter.com header.s=fm1 header.b=clUA+FOs; dkim=fail header.i=@messagingengine.com header.s=fm1 header.b=wsUvo0vW; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id w2-20020a17090652c200b0072b11a2a2afsi4234350ejn.57.2022.08.31.14.00.39 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 14:00:40 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=fail header.i=@vvalter.com header.s=fm1 header.b=clUA+FOs; dkim=fail header.i=@messagingengine.com header.s=fm1 header.b=wsUvo0vW; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 723173851148 for <ouuuleilei@gmail.com>; Wed, 31 Aug 2022 21:00:36 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by sourceware.org (Postfix) with ESMTPS id EF433385800F for <gcc-patches@gcc.gnu.org>; Wed, 31 Aug 2022 21:00:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EF433385800F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vvalter.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vvalter.com Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 205AF32001C6 for <gcc-patches@gcc.gnu.org>; Wed, 31 Aug 2022 17:00:10 -0400 (EDT) Received: from imap47 ([10.202.2.97]) by compute5.internal (MEProxy); Wed, 31 Aug 2022 17:00:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vvalter.com; h= cc:content-type:date:date:from:from:in-reply-to:message-id :mime-version:reply-to:sender:subject:subject:to:to; s=fm1; t= 1661979609; x=1662066009; bh=XJsiwNxNJlxFMLsVqrRYEwthFevk40jMc3p p4hzFmZQ=; b=clUA+FOsZrfjMSVFWrq2Tou+pbx6ayE0X4WhmL1aLsvcEh0C8ZP pe/GhPOrz4ORkolNEIolTu5quIzWVUVN5vil7+NiJBuN+Xb7YFu0+ZsHg73r2z89 VPSR+WtExA8xHjHBb/hZxhUDiOM/RLgw2IA1R7KA/BPDvcA5esmlUQ6nhSBgEOQp bPktNFtpbcQuG7xQ7u0XXHkNli95OHIiE+ZZpuujzrFv2qJ+GMl9SrnZZw3Oh3iT xLi2zcG/ige3pVsSUyOYppUsHcpMI60TEIjQfwS08Tmmy6wnaFq4H1S7lwBufix7 wmTPozbjz02kAOa/84KCcclzXgh8Fw0AmsA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:message-id:mime-version :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1661979609; x= 1662066009; bh=XJsiwNxNJlxFMLsVqrRYEwthFevk40jMc3pp4hzFmZQ=; b=w sUvo0vWecZ0yXJGR08PyINq9d8olNRsMJPWn6e9q5Kb+E73kOKCXqlOu+rS/7P1Q jXvZZ1Ggi0cZz1c+s92+uyAhSsTRiGqUSvdzLbmX0RzwvZKN4YrlJJrgACalq21X 55BXi4rOZywXjqXOkSqan4uwq+ZRQXenYehObbUhtY46UBBnhJpHtyxLW9WQ42q8 2B9pHBp4OJkPIDloLXWP5tUeLt3TmmaX77BLhr/RdRNXKmsMIRtXbvJmVf3zBD0i cvs8lysJUs5cFKxOseoy5vPuQ8J7dNp7pu8QGsD/Q/Pw6UvC+lVS1NVoTw52qi1G e4/y1AOsC0CiOD5PtNrSQ== X-ME-Sender: <xms:2csPY2BZWZVrErw0KtZZJRdM3n4oU0qyh7iLZgBNS7BsxN7piDnlow> <xme:2csPYwhqzcm4Wp1D8tE9uO79yVAtfYxvGIg8Q55VhExbkCZ_tJ9Mw0_ptpvuInj3g SfNvD6ftdclDTjfpbg> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrvdekiedguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepofgfggfkfffhvffutgesthdtre dtreertdenucfhrhhomhepfdfuihhmohhnucftrghinhgvrhdfuceoghgttgdrghhnuhes vhhvrghlthgvrhdrtghomheqnecuggftrfgrthhtvghrnhepjeelgfeghefgheeludeuje dtudegtedtjeeghfeiheffheevtdffhfffudehveevnecuvehluhhsthgvrhfuihiivgep tdenucfrrghrrghmpehmrghilhhfrhhomhepghgttgdrghhnuhesvhhvrghlthgvrhdrtg homh X-ME-Proxy: <xmx:2csPY5mteSBxrUrWBUcWUeR6447HK-14LWnDrGpP0c8skXOfFa6sXg> <xmx:2csPY0zMtSEDZk79r2PLZjtT9Vco6O33SCwV0i2_HkSdJ1qCfjTJfg> <xmx:2csPY7QISCoi12NJ0pcvgQ4_68lo-L9rtSwJr9IR1mugMOGdTLvXpw> <xmx:2csPYyeQN9bXO6C00XnuotPMzaaIE9jjV5j4RGXz5Ryq0qOHT7c_NA> Feedback-ID: ibff947e6:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 4839BA6007C; Wed, 31 Aug 2022 17:00:09 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.7.0-alpha0-841-g7899e99a45-fm-20220811.002-g7899e99a Mime-Version: 1.0 Message-Id: <5d937aa3-c33d-4ade-8b7e-9fd0effd147d@www.fastmail.com> Date: Wed, 31 Aug 2022 23:00:08 +0200 From: "Simon Rainer" <gcc.gnu@vvalter.com> To: gcc-patches@gcc.gnu.org Subject: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] Content-Type: text/plain X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1742711963025610343?= X-GMAIL-MSGID: =?utf-8?q?1742711963025610343?= |
Series |
ipa: Fix throw in multi-versioned functions [PR106627]
|
|
Commit Message
Simon Rainer
Aug. 31, 2022, 9 p.m. UTC
Hi,
This patch fixes PR106627. I ran the i386.exp tests on my x86_64-linux-gnu machine with a fully bootstrapped checkout. I also tested manually that no exception handling code is generated if none of the function versions throws an exception.
I don't have access to a machine to test the change to rs6000.cc, but the code seems like an exact copy and I don't see a reason why it shouldn't work there the same way.
Regards
Simon Rainer
From 6fcb1c742fa1d61048f7d63243225a8d1931af4a Mon Sep 17 00:00:00 2001
From: Simon Rainer <gcc.gnu@vvalter.com>
Date: Wed, 31 Aug 2022 20:56:04 +0200
Subject: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627]
Any multi-versioned function was implicitly declared as noexcept, which
leads to an abort if an exception is thrown inside the function.
The reason for this is that the function declaration is replaced by a
newly created dispatcher declaration, which has TREE_NOTHROW always set
to 1. Instead we need to set TREE_NOTHROW to the value of the original
declaration.
PR ipa/106627
gcc/ChangeLog:
* config/i386/i386-features.cc (ix86_get_function_versions_dispatcher): Set TREE_NOTHROW
correctly for dispatcher declaration
* config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher): Likewise
gcc/testsuite/ChangeLog:
* g++.target/i386/pr106627.C: New test.
---
gcc/config/i386/i386-features.cc | 1 +
gcc/config/rs6000/rs6000.cc | 1 +
gcc/testsuite/g++.target/i386/pr106627.C | 30 ++++++++++++++++++++++++
3 files changed, 32 insertions(+)
create mode 100644 gcc/testsuite/g++.target/i386/pr106627.C
Comments
On Wed, Aug 31, 2022 at 11:00 PM Simon Rainer <gcc.gnu@vvalter.com> wrote: > > Hi, > > This patch fixes PR106627. I ran the i386.exp tests on my x86_64-linux-gnu machine with a fully bootstrapped checkout. I also tested manually that no exception handling code is generated if none of the function versions throws an exception. > I don't have access to a machine to test the change to rs6000.cc, but the code seems like an exact copy and I don't see a reason why it shouldn't work there the same way. > > Regards > Simon Rainer > > From 6fcb1c742fa1d61048f7d63243225a8d1931af4a Mon Sep 17 00:00:00 2001 > From: Simon Rainer <gcc.gnu@vvalter.com> > Date: Wed, 31 Aug 2022 20:56:04 +0200 > Subject: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] > > Any multi-versioned function was implicitly declared as noexcept, which > leads to an abort if an exception is thrown inside the function. > The reason for this is that the function declaration is replaced by a > newly created dispatcher declaration, which has TREE_NOTHROW always set > to 1. Instead we need to set TREE_NOTHROW to the value of the original > declaration. Looks quite obvious. The middle-end to target interface is a bit iffy since we have to duplicate this everywhere. There's also other flags like pure/const and noreturn that do not impose correctness issues but may cause irritations if the IL gets a call to the dispatcher not marked noreturn but there's no code following. That said, the fix looks good to me. Thanks, Richard. > PR ipa/106627 > > gcc/ChangeLog: > > * config/i386/i386-features.cc (ix86_get_function_versions_dispatcher): Set TREE_NOTHROW > correctly for dispatcher declaration > * config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher): Likewise > > gcc/testsuite/ChangeLog: > > * g++.target/i386/pr106627.C: New test. > --- > gcc/config/i386/i386-features.cc | 1 + > gcc/config/rs6000/rs6000.cc | 1 + > gcc/testsuite/g++.target/i386/pr106627.C | 30 ++++++++++++++++++++++++ > 3 files changed, 32 insertions(+) > create mode 100644 gcc/testsuite/g++.target/i386/pr106627.C > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > index d6bb66cbe01..5b3b1aeff28 100644 > --- a/gcc/config/i386/i386-features.cc > +++ b/gcc/config/i386/i386-features.cc > @@ -3268,6 +3268,7 @@ ix86_get_function_versions_dispatcher (void *decl) > > /* Right now, the dispatching is done via ifunc. */ > dispatch_decl = make_dispatcher_decl (default_node->decl); > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > gcc_assert (dispatcher_node != NULL); > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index 2f3146e56f8..9280da8a5c8 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -24861,6 +24861,7 @@ rs6000_get_function_versions_dispatcher (void *decl) > > /* Right now, the dispatching is done via ifunc. */ > dispatch_decl = make_dispatcher_decl (default_node->decl); > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > gcc_assert (dispatcher_node != NULL); > diff --git a/gcc/testsuite/g++.target/i386/pr106627.C b/gcc/testsuite/g++.target/i386/pr106627.C > new file mode 100644 > index 00000000000..a67f5ae4813 > --- /dev/null > +++ b/gcc/testsuite/g++.target/i386/pr106627.C > @@ -0,0 +1,30 @@ > +/* PR c++/103012 Exception handling with multiversioned functions */ > +/* { dg-do run } */ > +/* { dg-require-ifunc "" } */ > + > +#include <assert.h> > + > +__attribute__((target("default"))) > +void f() { > + throw 1; > +} > + > +__attribute__((target("sse4.2,bmi"))) > +void f() { > + throw 2; > +} > + > +int main() > +{ > + try { > + f(); > + } > + catch(...) > + { > + return 0; > + } > + > + assert (false); > + return 1; > +} > + > -- > 2.34.1 >
Hi, Thanks for taking a look at my patch. I tested some combinations with pure/noreturn attributes. gcc seems to ignore those attributes on multiversion functions and generates sub-optimal assembly. But I wasn't able to fix this by simply copying members like DECL_PURE_P. It's pretty hard for me to tell which members of tree are relevant for a function declaration and should be copied and which should not be copied. Anyway, I think the TREE_NOTHROW change is the most important one, because it leads to correctness problems (and is what broke my original program :D ), so could you please commit my patch as I don't have write-access myself. Should I open a new bug on bugzilla for the pure/noreturn issue? Thanks Simon Rainer On Thu, Sep 1, 2022, at 08:37, Richard Biener wrote: > On Wed, Aug 31, 2022 at 11:00 PM Simon Rainer <gcc.gnu@vvalter.com> wrote: > > > > Hi, > > > > This patch fixes PR106627. I ran the i386.exp tests on my x86_64-linux-gnu machine with a fully bootstrapped checkout. I also tested manually that no exception handling code is generated if none of the function versions throws an exception. > > I don't have access to a machine to test the change to rs6000.cc, but the code seems like an exact copy and I don't see a reason why it shouldn't work there the same way. > > > > Regards > > Simon Rainer > > > > From 6fcb1c742fa1d61048f7d63243225a8d1931af4a Mon Sep 17 00:00:00 2001 > > From: Simon Rainer <gcc.gnu@vvalter.com> > > Date: Wed, 31 Aug 2022 20:56:04 +0200 > > Subject: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] > > > > Any multi-versioned function was implicitly declared as noexcept, which > > leads to an abort if an exception is thrown inside the function. > > The reason for this is that the function declaration is replaced by a > > newly created dispatcher declaration, which has TREE_NOTHROW always set > > to 1. Instead we need to set TREE_NOTHROW to the value of the original > > declaration. > > Looks quite obvious. The middle-end to target interface is a bit iffy > since we have > to duplicate this everywhere. There's also other flags like > pure/const and noreturn > that do not impose correctness issues but may cause irritations if the IL gets > a call to the dispatcher not marked noreturn but there's no code following. > > That said, the fix looks good to me. > > Thanks, > Richard. > > > PR ipa/106627 > > > > gcc/ChangeLog: > > > > * config/i386/i386-features.cc (ix86_get_function_versions_dispatcher): Set TREE_NOTHROW > > correctly for dispatcher declaration > > * config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher): Likewise > > > > gcc/testsuite/ChangeLog: > > > > * g++.target/i386/pr106627.C: New test. > > --- > > gcc/config/i386/i386-features.cc | 1 + > > gcc/config/rs6000/rs6000.cc | 1 + > > gcc/testsuite/g++.target/i386/pr106627.C | 30 ++++++++++++++++++++++++ > > 3 files changed, 32 insertions(+) > > create mode 100644 gcc/testsuite/g++.target/i386/pr106627.C > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > index d6bb66cbe01..5b3b1aeff28 100644 > > --- a/gcc/config/i386/i386-features.cc > > +++ b/gcc/config/i386/i386-features.cc > > @@ -3268,6 +3268,7 @@ ix86_get_function_versions_dispatcher (void *decl) > > > > /* Right now, the dispatching is done via ifunc. */ > > dispatch_decl = make_dispatcher_decl (default_node->decl); > > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > > gcc_assert (dispatcher_node != NULL); > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > > index 2f3146e56f8..9280da8a5c8 100644 > > --- a/gcc/config/rs6000/rs6000.cc > > +++ b/gcc/config/rs6000/rs6000.cc > > @@ -24861,6 +24861,7 @@ rs6000_get_function_versions_dispatcher (void *decl) > > > > /* Right now, the dispatching is done via ifunc. */ > > dispatch_decl = make_dispatcher_decl (default_node->decl); > > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > > gcc_assert (dispatcher_node != NULL); > > diff --git a/gcc/testsuite/g++.target/i386/pr106627.C b/gcc/testsuite/g++.target/i386/pr106627.C > > new file mode 100644 > > index 00000000000..a67f5ae4813 > > --- /dev/null > > +++ b/gcc/testsuite/g++.target/i386/pr106627.C > > @@ -0,0 +1,30 @@ > > +/* PR c++/103012 Exception handling with multiversioned functions */ > > +/* { dg-do run } */ > > +/* { dg-require-ifunc "" } */ > > + > > +#include <assert.h> > > + > > +__attribute__((target("default"))) > > +void f() { > > + throw 1; > > +} > > + > > +__attribute__((target("sse4.2,bmi"))) > > +void f() { > > + throw 2; > > +} > > + > > +int main() > > +{ > > + try { > > + f(); > > + } > > + catch(...) > > + { > > + return 0; > > + } > > + > > + assert (false); > > + return 1; > > +} > > + > > -- > > 2.34.1 > > >
On Thu, Sep 1, 2022 at 7:51 PM Simon Rainer <gcc.gnu@vvalter.com> wrote: > > Hi, > > Thanks for taking a look at my patch. I tested some combinations with pure/noreturn attributes. gcc seems to ignore those attributes on multiversion functions and generates sub-optimal assembly. > But I wasn't able to fix this by simply copying members like DECL_PURE_P. It's pretty hard for me to tell which members of tree are relevant for a function declaration and should be copied and which should not be copied. > > Anyway, I think the TREE_NOTHROW change is the most important one, because it leads to correctness problems (and is what broke my original program :D ), so could you please commit my patch as I don't have write-access myself. Sure, will do - thanks for the fix! > > Should I open a new bug on bugzilla for the pure/noreturn issue? Yes, I think it's worth investigating. Richard. > Thanks > Simon Rainer > > > On Thu, Sep 1, 2022, at 08:37, Richard Biener wrote: > > On Wed, Aug 31, 2022 at 11:00 PM Simon Rainer <gcc.gnu@vvalter.com> wrote: > > > > > > Hi, > > > > > > This patch fixes PR106627. I ran the i386.exp tests on my x86_64-linux-gnu machine with a fully bootstrapped checkout. I also tested manually that no exception handling code is generated if none of the function versions throws an exception. > > > I don't have access to a machine to test the change to rs6000.cc, but the code seems like an exact copy and I don't see a reason why it shouldn't work there the same way. > > > > > > Regards > > > Simon Rainer > > > > > > From 6fcb1c742fa1d61048f7d63243225a8d1931af4a Mon Sep 17 00:00:00 2001 > > > From: Simon Rainer <gcc.gnu@vvalter.com> > > > Date: Wed, 31 Aug 2022 20:56:04 +0200 > > > Subject: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] > > > > > > Any multi-versioned function was implicitly declared as noexcept, which > > > leads to an abort if an exception is thrown inside the function. > > > The reason for this is that the function declaration is replaced by a > > > newly created dispatcher declaration, which has TREE_NOTHROW always set > > > to 1. Instead we need to set TREE_NOTHROW to the value of the original > > > declaration. > > > > Looks quite obvious. The middle-end to target interface is a bit iffy > > since we have > > to duplicate this everywhere. There's also other flags like > > pure/const and noreturn > > that do not impose correctness issues but may cause irritations if the IL gets > > a call to the dispatcher not marked noreturn but there's no code following. > > > > That said, the fix looks good to me. > > > > Thanks, > > Richard. > > > > > PR ipa/106627 > > > > > > gcc/ChangeLog: > > > > > > * config/i386/i386-features.cc (ix86_get_function_versions_dispatcher): Set TREE_NOTHROW > > > correctly for dispatcher declaration > > > * config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher): Likewise > > > > > > gcc/testsuite/ChangeLog: > > > > > > * g++.target/i386/pr106627.C: New test. > > > --- > > > gcc/config/i386/i386-features.cc | 1 + > > > gcc/config/rs6000/rs6000.cc | 1 + > > > gcc/testsuite/g++.target/i386/pr106627.C | 30 ++++++++++++++++++++++++ > > > 3 files changed, 32 insertions(+) > > > create mode 100644 gcc/testsuite/g++.target/i386/pr106627.C > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > index d6bb66cbe01..5b3b1aeff28 100644 > > > --- a/gcc/config/i386/i386-features.cc > > > +++ b/gcc/config/i386/i386-features.cc > > > @@ -3268,6 +3268,7 @@ ix86_get_function_versions_dispatcher (void *decl) > > > > > > /* Right now, the dispatching is done via ifunc. */ > > > dispatch_decl = make_dispatcher_decl (default_node->decl); > > > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > > > > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > > > gcc_assert (dispatcher_node != NULL); > > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > > > index 2f3146e56f8..9280da8a5c8 100644 > > > --- a/gcc/config/rs6000/rs6000.cc > > > +++ b/gcc/config/rs6000/rs6000.cc > > > @@ -24861,6 +24861,7 @@ rs6000_get_function_versions_dispatcher (void *decl) > > > > > > /* Right now, the dispatching is done via ifunc. */ > > > dispatch_decl = make_dispatcher_decl (default_node->decl); > > > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > > > > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > > > gcc_assert (dispatcher_node != NULL); > > > diff --git a/gcc/testsuite/g++.target/i386/pr106627.C b/gcc/testsuite/g++.target/i386/pr106627.C > > > new file mode 100644 > > > index 00000000000..a67f5ae4813 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.target/i386/pr106627.C > > > @@ -0,0 +1,30 @@ > > > +/* PR c++/103012 Exception handling with multiversioned functions */ > > > +/* { dg-do run } */ > > > +/* { dg-require-ifunc "" } */ > > > + > > > +#include <assert.h> > > > + > > > +__attribute__((target("default"))) > > > +void f() { > > > + throw 1; > > > +} > > > + > > > +__attribute__((target("sse4.2,bmi"))) > > > +void f() { > > > + throw 2; > > > +} > > > + > > > +int main() > > > +{ > > > + try { > > > + f(); > > > + } > > > + catch(...) > > > + { > > > + return 0; > > > + } > > > + > > > + assert (false); > > > + return 1; > > > +} > > > + > > > -- > > > 2.34.1 > > > > >
Hi, Thanks for committing the patch. I created PR106816 to track the noreturn/pure problem. Regards Simon Rainer On Fri, Sep 2, 2022, at 08:03, Richard Biener wrote: > On Thu, Sep 1, 2022 at 7:51 PM Simon Rainer <gcc.gnu@vvalter.com> wrote: > > > > Hi, > > > > Thanks for taking a look at my patch. I tested some combinations with pure/noreturn attributes. gcc seems to ignore those attributes on multiversion functions and generates sub-optimal assembly. > > But I wasn't able to fix this by simply copying members like DECL_PURE_P. It's pretty hard for me to tell which members of tree are relevant for a function declaration and should be copied and which should not be copied. > > > > Anyway, I think the TREE_NOTHROW change is the most important one, because it leads to correctness problems (and is what broke my original program :D ), so could you please commit my patch as I don't have write-access myself. > > Sure, will do - thanks for the fix! > > > > > Should I open a new bug on bugzilla for the pure/noreturn issue? > > Yes, I think it's worth investigating. > > Richard. > > > Thanks > > Simon Rainer > > > > > > On Thu, Sep 1, 2022, at 08:37, Richard Biener wrote: > > > On Wed, Aug 31, 2022 at 11:00 PM Simon Rainer <gcc.gnu@vvalter.com> wrote: > > > > > > > > Hi, > > > > > > > > This patch fixes PR106627. I ran the i386.exp tests on my x86_64-linux-gnu machine with a fully bootstrapped checkout. I also tested manually that no exception handling code is generated if none of the function versions throws an exception. > > > > I don't have access to a machine to test the change to rs6000.cc, but the code seems like an exact copy and I don't see a reason why it shouldn't work there the same way. > > > > > > > > Regards > > > > Simon Rainer > > > > > > > > From 6fcb1c742fa1d61048f7d63243225a8d1931af4a Mon Sep 17 00:00:00 2001 > > > > From: Simon Rainer <gcc.gnu@vvalter.com> > > > > Date: Wed, 31 Aug 2022 20:56:04 +0200 > > > > Subject: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] > > > > > > > > Any multi-versioned function was implicitly declared as noexcept, which > > > > leads to an abort if an exception is thrown inside the function. > > > > The reason for this is that the function declaration is replaced by a > > > > newly created dispatcher declaration, which has TREE_NOTHROW always set > > > > to 1. Instead we need to set TREE_NOTHROW to the value of the original > > > > declaration. > > > > > > Looks quite obvious. The middle-end to target interface is a bit iffy > > > since we have > > > to duplicate this everywhere. There's also other flags like > > > pure/const and noreturn > > > that do not impose correctness issues but may cause irritations if the IL gets > > > a call to the dispatcher not marked noreturn but there's no code following. > > > > > > That said, the fix looks good to me. > > > > > > Thanks, > > > Richard. > > > > > > > PR ipa/106627 > > > > > > > > gcc/ChangeLog: > > > > > > > > * config/i386/i386-features.cc (ix86_get_function_versions_dispatcher): Set TREE_NOTHROW > > > > correctly for dispatcher declaration > > > > * config/rs6000/rs6000.cc (rs6000_get_function_versions_dispatcher): Likewise > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.target/i386/pr106627.C: New test. > > > > --- > > > > gcc/config/i386/i386-features.cc | 1 + > > > > gcc/config/rs6000/rs6000.cc | 1 + > > > > gcc/testsuite/g++.target/i386/pr106627.C | 30 ++++++++++++++++++++++++ > > > > 3 files changed, 32 insertions(+) > > > > create mode 100644 gcc/testsuite/g++.target/i386/pr106627.C > > > > > > > > diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc > > > > index d6bb66cbe01..5b3b1aeff28 100644 > > > > --- a/gcc/config/i386/i386-features.cc > > > > +++ b/gcc/config/i386/i386-features.cc > > > > @@ -3268,6 +3268,7 @@ ix86_get_function_versions_dispatcher (void *decl) > > > > > > > > /* Right now, the dispatching is done via ifunc. */ > > > > dispatch_decl = make_dispatcher_decl (default_node->decl); > > > > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > > > > > > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > > > > gcc_assert (dispatcher_node != NULL); > > > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > > > > index 2f3146e56f8..9280da8a5c8 100644 > > > > --- a/gcc/config/rs6000/rs6000.cc > > > > +++ b/gcc/config/rs6000/rs6000.cc > > > > @@ -24861,6 +24861,7 @@ rs6000_get_function_versions_dispatcher (void *decl) > > > > > > > > /* Right now, the dispatching is done via ifunc. */ > > > > dispatch_decl = make_dispatcher_decl (default_node->decl); > > > > + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); > > > > > > > > dispatcher_node = cgraph_node::get_create (dispatch_decl); > > > > gcc_assert (dispatcher_node != NULL); > > > > diff --git a/gcc/testsuite/g++.target/i386/pr106627.C b/gcc/testsuite/g++.target/i386/pr106627.C > > > > new file mode 100644 > > > > index 00000000000..a67f5ae4813 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/g++.target/i386/pr106627.C > > > > @@ -0,0 +1,30 @@ > > > > +/* PR c++/103012 Exception handling with multiversioned functions */ > > > > +/* { dg-do run } */ > > > > +/* { dg-require-ifunc "" } */ > > > > + > > > > +#include <assert.h> > > > > + > > > > +__attribute__((target("default"))) > > > > +void f() { > > > > + throw 1; > > > > +} > > > > + > > > > +__attribute__((target("sse4.2,bmi"))) > > > > +void f() { > > > > + throw 2; > > > > +} > > > > + > > > > +int main() > > > > +{ > > > > + try { > > > > + f(); > > > > + } > > > > + catch(...) > > > > + { > > > > + return 0; > > > > + } > > > > + > > > > + assert (false); > > > > + return 1; > > > > +} > > > > + > > > > -- > > > > 2.34.1 > > > > > > > >
diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index d6bb66cbe01..5b3b1aeff28 100644 --- a/gcc/config/i386/i386-features.cc +++ b/gcc/config/i386/i386-features.cc @@ -3268,6 +3268,7 @@ ix86_get_function_versions_dispatcher (void *decl) /* Right now, the dispatching is done via ifunc. */ dispatch_decl = make_dispatcher_decl (default_node->decl); + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); dispatcher_node = cgraph_node::get_create (dispatch_decl); gcc_assert (dispatcher_node != NULL); diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 2f3146e56f8..9280da8a5c8 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -24861,6 +24861,7 @@ rs6000_get_function_versions_dispatcher (void *decl) /* Right now, the dispatching is done via ifunc. */ dispatch_decl = make_dispatcher_decl (default_node->decl); + TREE_NOTHROW(dispatch_decl) = TREE_NOTHROW(fn); dispatcher_node = cgraph_node::get_create (dispatch_decl); gcc_assert (dispatcher_node != NULL); diff --git a/gcc/testsuite/g++.target/i386/pr106627.C b/gcc/testsuite/g++.target/i386/pr106627.C new file mode 100644 index 00000000000..a67f5ae4813 --- /dev/null +++ b/gcc/testsuite/g++.target/i386/pr106627.C @@ -0,0 +1,30 @@ +/* PR c++/103012 Exception handling with multiversioned functions */ +/* { dg-do run } */ +/* { dg-require-ifunc "" } */ + +#include <assert.h> + +__attribute__((target("default"))) +void f() { + throw 1; +} + +__attribute__((target("sse4.2,bmi"))) +void f() { + throw 2; +} + +int main() +{ + try { + f(); + } + catch(...) + { + return 0; + } + + assert (false); + return 1; +} +