diff mbox series

amdgcn: Add stub personality function

Message ID 1efeee77-2062-bb35-fca7-8e44ff422ea1@codesourcery.com
State New
Headers show
Series amdgcn: Add stub personality function | expand

Commit Message

Kwok Cheung Yeung April 22, 2020, 9:10 p.m. UTC 
Hello

This patch adds a stub implementation of __gxx_personality_v0, which is used in 
C++ exception handling. AMD GCN currently does not actually support exception 
handling (the unwind functions are all stubs too), so adding an extra stub 
function does not regress the current level of functionality any. This allows 
the following tests in the libgomp testsuite that were previously failing with a 
linker error to compile and run, provided that they do not throw any exceptions:

libgomp.c-c++-common/function-not-offloaded.c
libgomp.c++/for-15.C
libgomp.c++/for-24.C
libgomp.oacc-c-c++-common/routine-1.c
libgomp.oacc-c++/pr71959.C
libgomp.oacc-c++/routine-1-auto.C
libgomp.oacc-c++/routine-1-template-auto.C
libgomp.oacc-c++/routine-1-template-trailing-return-type.C
libgomp.oacc-c++/routine-1-template.C
libgomp.oacc-c++/routine-1-trailing-return-type.C

Tested with offloaded and standalone builds of GCC for AMD GCN. Okay for trunk?

Kwok

2020-04-22  Kwok Cheung Yeung  <kcy@codesourcery.com>

	libgcc/
	* config/gcn/unwind-gcn.c (__gxx_personality_v0): New.

+}

Comments

Andrew Stubbs April 23, 2020, 8:15 a.m. UTC | #1 
On 22/04/2020 22:10, Kwok Cheung Yeung wrote:
> Hello
> 
> This patch adds a stub implementation of __gxx_personality_v0, which is 
> used in C++ exception handling. AMD GCN currently does not actually 
> support exception handling (the unwind functions are all stubs too), so 
> adding an extra stub function does not regress the current level of 
> functionality any. This allows the following tests in the libgomp 
> testsuite that were previously failing with a linker error to compile 
> and run, provided that they do not throw any exceptions:
> 
> libgomp.c-c++-common/function-not-offloaded.c
> libgomp.c++/for-15.C
> libgomp.c++/for-24.C
> libgomp.oacc-c-c++-common/routine-1.c
> libgomp.oacc-c++/pr71959.C
> libgomp.oacc-c++/routine-1-auto.C
> libgomp.oacc-c++/routine-1-template-auto.C
> libgomp.oacc-c++/routine-1-template-trailing-return-type.C
> libgomp.oacc-c++/routine-1-template.C
> libgomp.oacc-c++/routine-1-trailing-return-type.C
> 
> Tested with offloaded and standalone builds of GCC for AMD GCN. Okay for 
> trunk?
> 
> Kwok
> 
> 2020-04-22  Kwok Cheung Yeung  <kcy@codesourcery.com>
> 
>      libgcc/
>      * config/gcn/unwind-gcn.c (__gxx_personality_v0): New.
> 
> diff --git a/libgcc/config/gcn/unwind-gcn.c 
> b/libgcc/config/gcn/unwind-gcn.c
> index 813f03f..6508b45 100644
> --- a/libgcc/config/gcn/unwind-gcn.c
> +++ b/libgcc/config/gcn/unwind-gcn.c
> @@ -35,3 +35,13 @@ _Unwind_GetIPInfo (struct _Unwind_Context *c, int 
> *ip_before_insn)
>   {
>     return 0;
>   }
> +
> +_Unwind_Reason_Code
> +__gxx_personality_v0 (int version,
> +              _Unwind_Action actions,
> +              _Unwind_Exception_Class exception_class,
> +              struct _Unwind_Exception *ue_header,
> +              struct _Unwind_Context *context)
> +{
> +  return 0;
> +}
> 

OK.

Andrew
Thomas Schwinge April 23, 2020, 11:05 a.m. UTC | #2 
Hi!

On 2020-04-23T09:15:29+0100, Andrew Stubbs <ams@codesourcery.com> wrote:
> On 22/04/2020 22:10, Kwok Cheung Yeung wrote:
>> This patch adds a stub implementation of __gxx_personality_v0, which is
>> used in C++ exception handling. AMD GCN currently does not actually
>> support exception handling

So we should simply disable it properly (see below)...

>> (the unwind functions are all stubs too), so
>> adding an extra stub function does not regress the current level of
>> functionality any.

... instead of adding such stub functions.

>> This allows the following tests in the libgomp
>> testsuite that were previously failing with a linker error to compile
>> and run, provided that they do not throw any exceptions:
>>
>> libgomp.c-c++-common/function-not-offloaded.c
>> libgomp.c++/for-15.C
>> libgomp.c++/for-24.C
>> libgomp.oacc-c-c++-common/routine-1.c
>> libgomp.oacc-c++/pr71959.C
>> libgomp.oacc-c++/routine-1-auto.C
>> libgomp.oacc-c++/routine-1-template-auto.C
>> libgomp.oacc-c++/routine-1-template-trailing-return-type.C
>> libgomp.oacc-c++/routine-1-template.C
>> libgomp.oacc-c++/routine-1-trailing-return-type.C

That's <https://gcc.gnu.org/PR94282> "[amdgcn] ld: error: undefined
symbol: __gxx_personality_v0", by the way, so should be referenced in the
ChangeLog.

>> Tested with offloaded and standalone builds of GCC for AMD GCN. Okay for
>> trunk?

>>      libgcc/
>>      * config/gcn/unwind-gcn.c (__gxx_personality_v0): New.

>> --- a/libgcc/config/gcn/unwind-gcn.c
>> +++ b/libgcc/config/gcn/unwind-gcn.c

>> +_Unwind_Reason_Code
>> +__gxx_personality_v0 (int version,
>> +              _Unwind_Action actions,
>> +              _Unwind_Exception_Class exception_class,
>> +              struct _Unwind_Exception *ue_header,
>> +              struct _Unwind_Context *context)
>> +{
>> +  return 0;
>> +}

What does a 'return 0' semantically mean here?  Shouldn't this rather
return some '_URC_*' code -- or even abort (given that we're not
supporting unwinding)?

> OK.

I suggest we instead apply what I'd proposed a month ago in
<https://gcc.gnu.org/PR94282> "[amdgcn] ld: error: undefined symbol:
__gxx_personality_v0", and now yesterday (coincidentally) posted.

    +static enum unwind_info_type
    +gcn_except_unwind_info (struct gcc_options *opts ATTRIBUTE_UNUSED)
    +{
    +  return UI_NONE;
    +}

    +#undef  TARGET_EXCEPT_UNWIND_INFO
    +#define TARGET_EXCEPT_UNWIND_INFO gcn_except_unwind_info


Grüße
 Thomas
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
Kwok Cheung Yeung April 23, 2020, 11:21 a.m. UTC | #3 
On 23/04/2020 12:05 pm, Thomas Schwinge wrote:
> So we should simply disable it properly (see below)...
> 
> ... instead of adding such stub functions.
> 
> 
> I suggest we instead apply what I'd proposed a month ago in
> <https://gcc.gnu.org/PR94282> "[amdgcn] ld: error: undefined symbol:
> __gxx_personality_v0", and now yesterday (coincidentally) posted.
> 
>      +static enum unwind_info_type
>      +gcn_except_unwind_info (struct gcc_options *opts ATTRIBUTE_UNUSED)
>      +{
>      +  return UI_NONE;
>      +}
> 
>      +#undef  TARGET_EXCEPT_UNWIND_INFO
>      +#define TARGET_EXCEPT_UNWIND_INFO gcn_except_unwind_info
> 

I agree that not generating the problematic code in the first place is the 
better approach. Does that mean we can now remove libgcc/config/gcn/unwind-gcn.c 
completely?

Thanks

Kwok
Andrew Stubbs April 23, 2020, 12:15 p.m. UTC | #4 
On 23/04/2020 12:21, Kwok Cheung Yeung wrote:
> I agree that not generating the problematic code in the first place is 
> the better approach. Does that mean we can now remove 
> libgcc/config/gcn/unwind-gcn.c completely?

That was added for the benefit of libgfortran, not C++. It's used by the 
backtrace code called from the stop handler, among other places, and I 
don't believe there was another workaround for that (without adding 
extra config checks to libgfortran).

I had not got to Thomas's patch yet, so I hadn't noticed they solved the 
same problem.

It probably is better to use Thomas's patch. I'm just worried that it'll 
catch me out when we do implement this stuff, in a way that the stub 
would not.

Andrew
diff mbox series

Patch

diff --git a/libgcc/config/gcn/unwind-gcn.c b/libgcc/config/gcn/unwind-gcn.c
index 813f03f..6508b45 100644
--- a/libgcc/config/gcn/unwind-gcn.c
+++ b/libgcc/config/gcn/unwind-gcn.c
@@ -35,3 +35,13 @@  _Unwind_GetIPInfo (struct _Unwind_Context *c, int 
*ip_before_insn)
  {
    return 0;
  }
+
+_Unwind_Reason_Code
+__gxx_personality_v0 (int version,
+		      _Unwind_Action actions,
+		      _Unwind_Exception_Class exception_class,
+		      struct _Unwind_Exception *ue_header,
+		      struct _Unwind_Context *context)
+{
+  return 0;