* [PATCH] ipa: Fix throw in multi-versioned functions [PR106627]
@ 2022-08-31 21:00 Simon Rainer
2022-09-01 6:37 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Simon Rainer @ 2022-08-31 21:00 UTC (permalink / raw)
To: gcc-patches
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
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627]
2022-08-31 21:00 [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] Simon Rainer
@ 2022-09-01 6:37 ` Richard Biener
2022-09-01 17:51 ` Simon Rainer
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2022-09-01 6:37 UTC (permalink / raw)
To: Simon Rainer, Jan Hubicka; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627]
2022-09-01 6:37 ` Richard Biener
@ 2022-09-01 17:51 ` Simon Rainer
2022-09-02 6:03 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Simon Rainer @ 2022-09-01 17:51 UTC (permalink / raw)
To: Richard Biener, Jan Hubicka; +Cc: GCC Patches
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
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627]
2022-09-01 17:51 ` Simon Rainer
@ 2022-09-02 6:03 ` Richard Biener
2022-09-02 15:37 ` Simon Rainer
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2022-09-02 6:03 UTC (permalink / raw)
To: Simon Rainer; +Cc: Jan Hubicka, GCC Patches
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
> > >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipa: Fix throw in multi-versioned functions [PR106627]
2022-09-02 6:03 ` Richard Biener
@ 2022-09-02 15:37 ` Simon Rainer
0 siblings, 0 replies; 5+ messages in thread
From: Simon Rainer @ 2022-09-02 15:37 UTC (permalink / raw)
To: Richard Biener; +Cc: Jan Hubicka, GCC Patches
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
> > > >
> > >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-02 15:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31 21:00 [PATCH] ipa: Fix throw in multi-versioned functions [PR106627] Simon Rainer
2022-09-01 6:37 ` Richard Biener
2022-09-01 17:51 ` Simon Rainer
2022-09-02 6:03 ` Richard Biener
2022-09-02 15:37 ` Simon Rainer
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).