Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-24 Thread Ian Lance Taylor
On Mon, Apr 22, 2019 at 2:14 AM Jakub Jelinek  wrote:
>
> As detailed in the PR, unlike most other targets, on ARM EABI the floating
> point registers are saved lazily, when EH personality routine calls
> __gnu_unwind_frame (usually in the CONTINUE_UNWINDING macro).
> That means the unwinder itself and the personality routines (and whatever
> other functions those call in the path to CONTINUE_UNWINDING) must be
> compiled so that it doesn't use floating point registers.  Calling some
> function that saves those on entry and restores on exit is fine, but calling
> some function which saves those on entry and then calls __gnu_unwind_frame
> and then restores on exit is not fine.
> In 8.x and earlier we were just lucky that the RA when compiling those
> didn't decide to use any of those registers, but starting with the combiner
> hard register changes we are no longer so lucky.
>
> The following patch introduces -mgeneral-regs-only option and
> general-regs-only target attribute for ARM (similarly to how other targets
> like AArch64 and x86), changes the ARM unwinder to be compiled with that
> and changes the personality routines of all languages so that either just
> the personality routine, or whatever other routines called by personality
> routines that call directly or indirectly __gnu_unwind_frame to be compiled
> that way.
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi (and x86_64-linux to make
> sure it compiles on other targets too).
>
> Ok for trunk?
>
> While the libgo changes are included in the patch, I think those need to go
> through go upstream and will likely need some changes if that code can be
> compiled by earlier GCC versions or other compilers.

Thanks.  I've committed the libgo changes.  (Older versions of GCC
have their own copy of libgo, and there are not yet any non-GCC
compilers that use this code and support ARM.)

Ian


Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-24 Thread Eric Botcazou
> The Ada changes need those guards because the file is compiled by both
> the system compiler and by the newly built compilers; when compiled by
> system compiler, as the FE is built with -fno-exceptions I'd hope the EH
> stuff isn't really used there and at least until GCC 9.1 is released we have
> the issue that the system compiler could be some earlier GCC 9.0.1 snapshot
> which doesn't support general-regs-only.

The Ada front-end does use EH on the host, but only the part written in Ada, 
that's why -fno-exceptions is very likely still OK for the C++ part.

The Ada bits are OK and I guess we don't care about earlier 9.0.1 snapshots.

-- 
Eric Botcazou


Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-23 Thread Jakub Jelinek
On Tue, Apr 23, 2019 at 09:17:50AM +0100, Ramana Radhakrishnan wrote:
> > Ok for trunk?
> 
> Ok. Thanks a lot for working on this and fixing the rest of this up.
> I've been busy with a few other things.

Thanks.  Based on this and Iain and Jonathan's acks, I've committed
the patch except for the 

> > 2019-04-22  Ramana Radhakrishnan  
> > Bernd Edlinger  
> > Jakub Jelinek  
> >
> > PR target/89093
> > gcc/ada/
> > * raise-gcc.c (TARGET_ATTRIBUTE): Define.
> > (continue_unwind, personality_body, PERSONALITY_FUNCTION): Add
> > TARGET_ATTRIBUTE.
> > libgo/
> > * runtime/go-unwind.c (PERSONALITY_FUNCTION,
> > __gccgo_personality_dummy): Add general-regs-only target
> > attribute for ARM.

parts, where for gcc/ada/ I'm waiting for an Ada maintainer review (CCed all
maintainers now) and for libgo I think this needs to be fixed upstream and
also dealt with non-gcc compilers or gcc 8.x and earlier.

Jakub


Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-23 Thread Jonathan Wakely

On 22/04/19 11:14 +0200, Jakub Jelinek wrote:

Hi!

As detailed in the PR, unlike most other targets, on ARM EABI the floating
point registers are saved lazily, when EH personality routine calls
__gnu_unwind_frame (usually in the CONTINUE_UNWINDING macro).
That means the unwinder itself and the personality routines (and whatever
other functions those call in the path to CONTINUE_UNWINDING) must be
compiled so that it doesn't use floating point registers.  Calling some
function that saves those on entry and restores on exit is fine, but calling
some function which saves those on entry and then calls __gnu_unwind_frame
and then restores on exit is not fine.
In 8.x and earlier we were just lucky that the RA when compiling those
didn't decide to use any of those registers, but starting with the combiner
hard register changes we are no longer so lucky.

The following patch introduces -mgeneral-regs-only option and
general-regs-only target attribute for ARM (similarly to how other targets
like AArch64 and x86), changes the ARM unwinder to be compiled with that
and changes the personality routines of all languages so that either just
the personality routine, or whatever other routines called by personality
routines that call directly or indirectly __gnu_unwind_frame to be compiled
that way.

Bootstrapped/regtested on armv7hl-linux-gnueabi (and x86_64-linux to make
sure it compiles on other targets too).

Ok for trunk?


The libsupc++ part is OK.




Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-23 Thread Ramana Radhakrishnan
On Mon, Apr 22, 2019 at 10:15 AM Jakub Jelinek  wrote:
>
> Hi!
>
> As detailed in the PR, unlike most other targets, on ARM EABI the floating
> point registers are saved lazily, when EH personality routine calls
> __gnu_unwind_frame (usually in the CONTINUE_UNWINDING macro).
> That means the unwinder itself and the personality routines (and whatever
> other functions those call in the path to CONTINUE_UNWINDING) must be
> compiled so that it doesn't use floating point registers.  Calling some
> function that saves those on entry and restores on exit is fine, but calling
> some function which saves those on entry and then calls __gnu_unwind_frame
> and then restores on exit is not fine.
> In 8.x and earlier we were just lucky that the RA when compiling those
> didn't decide to use any of those registers, but starting with the combiner
> hard register changes we are no longer so lucky.
>
> The following patch introduces -mgeneral-regs-only option and
> general-regs-only target attribute for ARM (similarly to how other targets
> like AArch64 and x86), changes the ARM unwinder to be compiled with that
> and changes the personality routines of all languages so that either just
> the personality routine, or whatever other routines called by personality
> routines that call directly or indirectly __gnu_unwind_frame to be compiled
> that way.
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi (and x86_64-linux to make
> sure it compiles on other targets too).
>
> Ok for trunk?

Ok. Thanks a lot for working on this and fixing the rest of this up.
I've been busy with a few other things.


> While the libgo changes are included in the patch, I think those need to go
> through go upstream and will likely need some changes if that code can be
> compiled by earlier GCC versions or other compilers.
>




> Not sure about libphobos D stuff, does it need to go through upstream and
> is libdruntime/gcc/deh.d compiled by compilers other than GDC?
>
> The Ada changes need those guards because the file is compiled by both
> the system compiler and by the newly built compilers; when compiled by
> system compiler, as the FE is built with -fno-exceptions I'd hope the EH
> stuff isn't really used there and at least until GCC 9.1 is released we have
> the issue that the system compiler could be some earlier GCC 9.0.1 snapshot
> which doesn't support general-regs-only.

I don't grok enough ada to approve this and will leave that part to
Eric or others.

Ramana

>
> 2019-04-22  Ramana Radhakrishnan  
> Bernd Edlinger  
> Jakub Jelinek  
>
> PR target/89093
> * config/arm/arm.c (aapcs_vfp_is_call_or_return_candidate): Diagnose
> if used with general-regs-only.
> (arm_conditional_register_usage): Don't add non-general regs if
> general-regs-only.
> (arm_valid_target_attribute_rec): Handle general-regs-only.
> * config/arm/arm.h (TARGET_HARD_FLOAT): Return false if
> general-regs-only.
> (TARGET_HARD_FLOAT_SUB): Define.
> (TARGET_SOFT_FLOAT): Define as negation of TARGET_HARD_FLOAT_SUB.
> (TARGET_REALLY_IWMMXT): Add && !TARGET_GENERAL_REGS_ONLY.
> (TARGET_REALLY_IWMMXT2): Likewise.
> * config/arm/arm.opt: Add -mgeneral-regs-only.
> * doc/extend.texi: Document ARM general-regs-only target.
> * doc/invoke.texi: Document ARM -mgeneral-regs-only.
> gcc/ada/
> * raise-gcc.c (TARGET_ATTRIBUTE): Define.
> (continue_unwind, personality_body, PERSONALITY_FUNCTION): Add
> TARGET_ATTRIBUTE.
> libgcc/
> * config/arm/pr-support.c: Add #pragma GCC 
> target("general-regs-only").
> * config/arm/unwind-arm.c: Likewise.
> * unwind-c.c (PERSONALITY_FUNCTION): Add general-regs-only target
> attribute for ARM.
> libobjc/
> * exception.c (PERSONALITY_FUNCTION): Add general-regs-only target
> attribute for ARM.
> libphobos/
> * libdruntime/gcc/deh.d: Import gcc.attribute.
> (personality_fn_attributes): New enum.
> (scanLSDA, CONTINUE_UNWINDING, gdc_personality, __gdc_personality):
> Add @personality_fn_attributes.
> libstdc++-v3/
> * libsupc++/eh_personality.cc (PERSONALITY_FUNCTION): Add
> general-regs-only target attribute for ARM.
> libgo/
> * runtime/go-unwind.c (PERSONALITY_FUNCTION,
> __gccgo_personality_dummy): Add general-regs-only target
> attribute for ARM.
>
> --- gcc/config/arm/arm.c(revision 270444)
> +++ gcc/config/arm/arm.c(working copy)
> @@ -6112,6 +6112,11 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pc
>  return false;
>
>*base_mode = new_mode;
> +
> +  if (TARGET_GENERAL_REGS_ONLY)
> +error ("argument of type %qT not permitted with -mgeneral-regs-only",
> +  type);
> +
>return true;
>  }
>
> @@ -28404,7 +28409,7 @@ arm_conditional_register_usage (void)
> }
>  }
>
> -  if (TARGET_REALLY_IWMMXT)
> + 

Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-22 Thread Iain Buclaw
On Mon, 22 Apr 2019 at 11:15, Jakub Jelinek  wrote:
>
> Not sure about libphobos D stuff, does it need to go through upstream and
> is libdruntime/gcc/deh.d compiled by compilers other than GDC?
>

It is not part of upstream, I could make that clearer in
libphobos/README.gcc if there's uncertainty.

> --- libphobos/libdruntime/gcc/deh.d (revision 270444)
> +++ libphobos/libdruntime/gcc/deh.d (working copy)
> @@ -28,6 +28,7 @@ import gcc.unwind;
>  import gcc.unwind.pe;
>  import gcc.builtins;
>  import gcc.config;
> +import gcc.attribute;
>
>  extern(C)
>  {
> @@ -519,10 +520,19 @@ extern(C) void _d_throw(Throwable object)
>  terminate("unwind error", __LINE__);
>  }
>
> +static if (GNU_ARM_EABI_Unwinder)
> +{
> +enum personality_fn_attributes = attribute("target", 
> ("general-regs-only"));
> +}
> +else
> +{
> +enum personality_fn_attributes = "";
> +}
>
>  /**
>   * Read and extract information from the LSDA (.gcc_except_table section).
>   */
> +@personality_fn_attributes
>  _Unwind_Reason_Code scanLSDA(const(ubyte)* lsda, _Unwind_Exception_Class 
> exceptionClass,
>   _Unwind_Action actions, _Unwind_Exception* 
> unwindHeader,
>   _Unwind_Context* context, _Unwind_Word cfa,
> @@ -772,6 +782,7 @@ int actionTableLookup(_Unwind_Action actions, _Unw
>   * Called when the personality function has found neither a cleanup or 
> handler.
>   * To support ARM EABI personality routines, that must also unwind the stack.
>   */
> +@personality_fn_attributes
>  _Unwind_Reason_Code CONTINUE_UNWINDING(_Unwind_Exception* unwindHeader, 
> _Unwind_Context* context)
>  {
>  static if (GNU_ARM_EABI_Unwinder)
> @@ -814,6 +825,7 @@ else
>  static if (GNU_ARM_EABI_Unwinder)
>  {
>  pragma(mangle, PERSONALITY_FUNCTION)
> +@personality_fn_attributes
>  extern(C) _Unwind_Reason_Code gdc_personality(_Unwind_State state,
>_Unwind_Exception* 
> unwindHeader,
>_Unwind_Context* context)
> @@ -873,6 +885,7 @@ else
>  }
>  }
>
> +@personality_fn_attributes
>  private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
>_Unwind_Exception_Class 
> exceptionClass,
>_Unwind_Exception* 
> unwindHeader,

No problem with this part from myself.

-- 
Iain


[PATCH] Fix ARM exception handling (PR target/89093)

2019-04-22 Thread Jakub Jelinek
Hi!

As detailed in the PR, unlike most other targets, on ARM EABI the floating
point registers are saved lazily, when EH personality routine calls
__gnu_unwind_frame (usually in the CONTINUE_UNWINDING macro).
That means the unwinder itself and the personality routines (and whatever
other functions those call in the path to CONTINUE_UNWINDING) must be
compiled so that it doesn't use floating point registers.  Calling some
function that saves those on entry and restores on exit is fine, but calling
some function which saves those on entry and then calls __gnu_unwind_frame
and then restores on exit is not fine.
In 8.x and earlier we were just lucky that the RA when compiling those
didn't decide to use any of those registers, but starting with the combiner
hard register changes we are no longer so lucky.

The following patch introduces -mgeneral-regs-only option and
general-regs-only target attribute for ARM (similarly to how other targets
like AArch64 and x86), changes the ARM unwinder to be compiled with that
and changes the personality routines of all languages so that either just
the personality routine, or whatever other routines called by personality
routines that call directly or indirectly __gnu_unwind_frame to be compiled
that way.

Bootstrapped/regtested on armv7hl-linux-gnueabi (and x86_64-linux to make
sure it compiles on other targets too).

Ok for trunk?

While the libgo changes are included in the patch, I think those need to go
through go upstream and will likely need some changes if that code can be
compiled by earlier GCC versions or other compilers.

Not sure about libphobos D stuff, does it need to go through upstream and
is libdruntime/gcc/deh.d compiled by compilers other than GDC?

The Ada changes need those guards because the file is compiled by both
the system compiler and by the newly built compilers; when compiled by
system compiler, as the FE is built with -fno-exceptions I'd hope the EH
stuff isn't really used there and at least until GCC 9.1 is released we have
the issue that the system compiler could be some earlier GCC 9.0.1 snapshot
which doesn't support general-regs-only.

2019-04-22  Ramana Radhakrishnan  
Bernd Edlinger  
Jakub Jelinek  

PR target/89093
* config/arm/arm.c (aapcs_vfp_is_call_or_return_candidate): Diagnose
if used with general-regs-only.
(arm_conditional_register_usage): Don't add non-general regs if
general-regs-only.
(arm_valid_target_attribute_rec): Handle general-regs-only.
* config/arm/arm.h (TARGET_HARD_FLOAT): Return false if
general-regs-only.
(TARGET_HARD_FLOAT_SUB): Define.
(TARGET_SOFT_FLOAT): Define as negation of TARGET_HARD_FLOAT_SUB.
(TARGET_REALLY_IWMMXT): Add && !TARGET_GENERAL_REGS_ONLY.
(TARGET_REALLY_IWMMXT2): Likewise.
* config/arm/arm.opt: Add -mgeneral-regs-only.
* doc/extend.texi: Document ARM general-regs-only target.
* doc/invoke.texi: Document ARM -mgeneral-regs-only.
gcc/ada/
* raise-gcc.c (TARGET_ATTRIBUTE): Define.
(continue_unwind, personality_body, PERSONALITY_FUNCTION): Add
TARGET_ATTRIBUTE.
libgcc/
* config/arm/pr-support.c: Add #pragma GCC target("general-regs-only").
* config/arm/unwind-arm.c: Likewise.
* unwind-c.c (PERSONALITY_FUNCTION): Add general-regs-only target
attribute for ARM.
libobjc/
* exception.c (PERSONALITY_FUNCTION): Add general-regs-only target
attribute for ARM.
libphobos/
* libdruntime/gcc/deh.d: Import gcc.attribute.
(personality_fn_attributes): New enum.
(scanLSDA, CONTINUE_UNWINDING, gdc_personality, __gdc_personality):
Add @personality_fn_attributes.
libstdc++-v3/
* libsupc++/eh_personality.cc (PERSONALITY_FUNCTION): Add
general-regs-only target attribute for ARM.
libgo/
* runtime/go-unwind.c (PERSONALITY_FUNCTION,
__gccgo_personality_dummy): Add general-regs-only target
attribute for ARM.

--- gcc/config/arm/arm.c(revision 270444)
+++ gcc/config/arm/arm.c(working copy)
@@ -6112,6 +6112,11 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pc
 return false;
 
   *base_mode = new_mode;
+
+  if (TARGET_GENERAL_REGS_ONLY)
+error ("argument of type %qT not permitted with -mgeneral-regs-only",
+  type);
+
   return true;
 }
 
@@ -28404,7 +28409,7 @@ arm_conditional_register_usage (void)
}
 }
 
-  if (TARGET_REALLY_IWMMXT)
+  if (TARGET_REALLY_IWMMXT && !TARGET_GENERAL_REGS_ONLY)
 {
   regno = FIRST_IWMMXT_GR_REGNUM;
   /* The 2002/10/09 revision of the XScale ABI has wCG0
@@ -30878,6 +30883,9 @@ arm_valid_target_attribute_rec (tree args, struct
   else if (!strcmp (q, "arm"))
opts->x_target_flags &= ~MASK_THUMB;
 
+  else if (!strcmp (q, "general-regs-only"))
+   opts->x_target_flags |= MASK_GENERAL_REGS_ONLY;
+
   else if (!strncmp (q,