[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #88 from Tamar Christina  ---
Thanks Jakub!

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #87 from Jakub Jelinek  ---
Author: jakub
Date: Tue Apr 30 16:30:44 2019
New Revision: 270705

URL: https://gcc.gnu.org/viewcvs?rev=270705=gcc=rev
Log:
PR target/89093
* gcc.target/aarch64/return_address_sign_3.c: Remove extra space in
target attribute.

Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/aarch64/return_address_sign_3.c

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #86 from Tamar Christina  ---
for aarch64-none-linux-gnu. I am still building the toolchain to take a look so
not able to give more detail.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Tamar Christina  changed:

   What|Removed |Added

 CC||tnfchris at gcc dot gnu.org

--- Comment #85 from Tamar Christina  ---
Hi Jakub,

commit r270690 seems to have caused a regression on
gcc.target/aarch64/return_address_sign_3.c

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #84 from Jakub Jelinek  ---
Author: jakub
Date: Tue Apr 30 12:07:27 2019
New Revision: 270690

URL: https://gcc.gnu.org/viewcvs?rev=270690=gcc=rev
Log:
PR target/89093
* config/aarch64/aarch64.c (aarch64_process_one_target_attr): Don't
skip
whitespace at the start of target attribute string.

* gcc.target/aarch64/pr89093.c: New test.
* gcc.target/aarch64/pr63304_1.c: Remove space from target string.

Added:
trunk/gcc/testsuite/gcc.target/aarch64/pr89093.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/aarch64/aarch64.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/aarch64/pr63304_1.c

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #83 from Jakub Jelinek  ---
Fixed then.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ian at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #82 from ian at gcc dot gnu.org  ---
Author: ian
Date: Wed Apr 24 12:45:45 2019
New Revision: 270542

URL: https://gcc.gnu.org/viewcvs?rev=270542=gcc=rev
Log:
PR target/89093
runtime: mark unwind functions general-regs-only on ARM

For https://gcc.gnu.org/PR89093.

Change-Id: Ic426b43d633c77104bda01d4e7835bc9ab4695ef
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/173657
Reviewed-by: Ian Lance Taylor 


Modified:
trunk/gcc/go/gofrontend/MERGE
trunk/libgo/runtime/go-unwind.c

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #81 from Jakub Jelinek  ---
Fixed for Ada as well, only Go left to do.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #80 from Jakub Jelinek  ---
Author: jakub
Date: Wed Apr 24 08:16:07 2019
New Revision: 270535

URL: https://gcc.gnu.org/viewcvs?rev=270535=gcc=rev
Log:
PR target/89093
* raise-gcc.c (TARGET_ATTRIBUTE): Define.
(continue_unwind, personality_body, PERSONALITY_FUNCTION): Add
TARGET_ATTRIBUTE.

Modified:
trunk/gcc/ada/ChangeLog
trunk/gcc/ada/raise-gcc.c

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

   Priority|P1  |P4
 CC||ebotcazou at gcc dot gnu.org,
   ||ian at gcc dot gnu.org

--- Comment #79 from Jakub Jelinek  ---
Fixed for all languages but Ada and Go, neither of them is release critical.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #78 from Jakub Jelinek  ---
Author: jakub
Date: Tue Apr 23 10:03:41 2019
New Revision: 270504

URL: https://gcc.gnu.org/viewcvs?rev=270504=gcc=rev
Log:
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.
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.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm.c
trunk/gcc/config/arm/arm.h
trunk/gcc/config/arm/arm.opt
trunk/gcc/doc/extend.texi
trunk/gcc/doc/invoke.texi
trunk/libgcc/ChangeLog
trunk/libgcc/config/arm/pr-support.c
trunk/libgcc/config/arm/unwind-arm.c
trunk/libgcc/unwind-c.c
trunk/libobjc/ChangeLog
trunk/libobjc/exception.c
trunk/libphobos/ChangeLog
trunk/libphobos/libdruntime/gcc/deh.d
trunk/libstdc++-v3/ChangeLog
trunk/libstdc++-v3/libsupc++/eh_personality.cc

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

  Attachment #46205|0   |1
is obsolete||

--- Comment #77 from Jakub Jelinek  ---
Created attachment 46220
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46220=edit
gcc9-pr89093.patch

The above patch passed bootstrap, but there is a regression:
+FAIL: compiler driver --help=target option(s): "^ +-.*[^:.]\$" absent from
output: "  -mgeneral-regs-only Generate code which uses the cor
e registers only(r0-r14)"
because there is no full stop at the end of description.  Fixed in this patch
(I've also added a space before (.

I've also seen
+FAIL: TestAbort
+FAIL: TestBreakpoint
+FAIL: TestCgoCrashHandler
+FAIL: TestCgoSignalDeadlock
+FAIL: TestSelectStackAdjust
regressions, but 1) not sure if they are reproducible or not, go has usually
lots of random intermitent failures 2) even if yes, the libgo part of the patch
I believe can't be committed together with the rest, as libgo has a different
upstream and will need to guard it on being compiled with gcc 9+, because it
can be built with other compilers too.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

  Attachment #46202|0   |1
is obsolete||
  Attachment #46203|0   |1
is obsolete||

--- Comment #76 from Jakub Jelinek  ---
Created attachment 46205
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46205=edit
gcc9-pr89093.patch

Updated patch.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #75 from Jakub Jelinek  ---
It failed for me as well.  And a GCC version check won't really help when using
earlier GCC 9 snapshot as system compiler (though, admittedly that isn't
supported).
Another option would be to define the attribute only #ifdef IN_RTS, because
when it is #ifdef IN_GCC_FRONTEND, it is compiled with -fno-exceptions and so
won't be really invoked.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Bernd Edlinger  changed:

   What|Removed |Added

  Attachment #46200|0   |1
is obsolete||

--- Comment #74 from Bernd Edlinger  ---
Created attachment 46203
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46203=edit
gcc9-pr89093.patch

Same patch, just guard the target attribute with a version check,
so that supported boot-strap configurations should work

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #73 from Bernd Edlinger  ---
Okay, the requirement is only to be able to boot-strap with
a released gcc version, so gcc-8 should not use the pragma,
while gcc-9 should use the pagma.
I was able to bootstrap from x86_64 -> arm cross -> arm native
that worked fine.
I will guard the pragma with a gcc-version check.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #72 from Bernd Edlinger  ---
I use host Compiler from last week:
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/ed/gnu/arm-linux-gnueabihf/libexec/gcc/armv7l-unknown-linux-gnueabihf/9.0.1/lto-wrapper
Target: armv7l-unknown-linux-gnueabihf
Configured with: ../gcc-9-20190331/configure
--prefix=/home/ed/gnu/arm-linux-gnueabihf --enable-languages=all
--with-arch=armv7-a --with-tune=cortex-a9 --with-fpu=vfpv3-d16
--with-float=hard
Thread model: posix
gcc version 9.0.1 20190331 (experimental) (GCC)

and libgnat, is definietly affected since the personality function pushes d8-d9

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #71 from Bernd Edlinger  ---
I am sorry, but my native arm bootstrap Fails:

g++ -std=gnu++98 -fno-PIE -c  -I../../gcc-trunk-r270444/gcc/../libgcc
-DEH_MECHANISM_arm -DIN_GCC_FRONTEND -g -DIN_GCC -fno-exceptions -fno-rtti
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
-Wcast-qual -Wno-format  -Wmissing-format-attribute -Woverloaded-virtual
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common 
-DHAVE_CONFIG_H -I. -Iada -I../../gcc-trunk-r270444/gcc
-I../../gcc-trunk-r270444/gcc/ada -I../../gcc-trunk-r270444/gcc/../include
-I../../gcc-trunk-r270444/gcc/../libcpp/include
-I/home/ed/gnu/gcc-build-arm-linux-gnueabihf/./gmp
-I/home/ed/gnu/gcc-trunk-r270444/gmp
-I/home/ed/gnu/gcc-build-arm-linux-gnueabihf/./mpfr/src
-I/home/ed/gnu/gcc-trunk-r270444/mpfr/src
-I/home/ed/gnu/gcc-trunk-r270444/mpc/src 
-I../../gcc-trunk-r270444/gcc/../libdecnumber
-I../../gcc-trunk-r270444/gcc/../libdecnumber/dpd -I../libdecnumber
-I../../gcc-trunk-r270444/gcc/../libbacktrace
-I/home/ed/gnu/gcc-build-arm-linux-gnueabihf/./isl/include
-I/home/ed/gnu/gcc-trunk-r270444/isl/include  -o ada/raise-gcc.o -MT
ada/raise-gcc.o -MMD -MP -MF ada/.deps/raise-gcc.TPo
../../gcc-trunk-r270444/gcc/ada/raise-gcc.c
g++ -std=gnu++98 -fno-PIE -c  -DIN_GCC_FRONTEND -g -DIN_GCC -fno-exceptions
-fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
-Wcast-qual -Wno-format  -Wmissing-format-attribute -Woverloaded-virtual
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common 
-DHAVE_CONFIG_H -I. -Iada -I../../gcc-trunk-r270444/gcc
-I../../gcc-trunk-r270444/gcc/ada -I../../gcc-trunk-r270444/gcc/../include
-I../../gcc-trunk-r270444/gcc/../libcpp/include
-I/home/ed/gnu/gcc-build-arm-linux-gnueabihf/./gmp
-I/home/ed/gnu/gcc-trunk-r270444/gmp
-I/home/ed/gnu/gcc-build-arm-linux-gnueabihf/./mpfr/src
-I/home/ed/gnu/gcc-trunk-r270444/mpfr/src
-I/home/ed/gnu/gcc-trunk-r270444/mpc/src 
-I../../gcc-trunk-r270444/gcc/../libdecnumber
-I../../gcc-trunk-r270444/gcc/../libdecnumber/dpd -I../libdecnumber
-I../../gcc-trunk-r270444/gcc/../libbacktrace
-I/home/ed/gnu/gcc-build-arm-linux-gnueabihf/./isl/include
-I/home/ed/gnu/gcc-trunk-r270444/isl/include  -o ada/seh_init.o -MT
ada/seh_init.o -MMD -MP -MF ada/.deps/seh_init.TPo
../../gcc-trunk-r270444/gcc/ada/seh_init.c
../../gcc-trunk-r270444/gcc/ada/raise-gcc.c:1165:55: error: unknown target
attribute or pragma 'general-regs-only'
 1165 |struct _Unwind_Context* uw_context ATTRIBUTE_UNUSED)
  |   ^
../../gcc-trunk-r270444/gcc/ada/raise-gcc.c:1183:32: error: unknown target
attribute or pragma 'general-regs-only'
 1183 | _Unwind_Context *uw_context)
  |^
../../gcc-trunk-r270444/gcc/ada/raise-gcc.c:1354:43: error: unknown target
attribute or pragma 'general-regs-only'
 1354 | struct _Unwind_Context* uw_context)
  |   ^
make[3]: *** [ada/raise-gcc.o] Error 1
make[3]: *** Waiting for unfinished jobs
make[3]: Leaving directory `/home/ed/gnu/gcc-build-arm-linux-gnueabihf/gcc'
make[2]: *** [all-stage1-gcc] Error 2
make[2]: Leaving directory `/home/ed/gnu/gcc-build-arm-linux-gnueabihf'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/ed/gnu/gcc-build-arm-linux-gnueabihf'
make: *** [all] Error 2

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #70 from Bernd Edlinger  ---
Yes, thanks, now switching to your latest patch.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

  Attachment #46198|0   |1
is obsolete||

--- Comment #69 from Jakub Jelinek  ---
Created attachment 46202
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46202=edit
gcc9-pr89093.patch

So, here is the above patch updated in the one libgo hunk.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #68 from Jakub Jelinek  ---
Comment on attachment 46200
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46200
updated patch

I believe the second hunk in libgo/runtime/go-unwind.c is incorrect, that is on
code not guarded with #ifdef __ARM_EABI_UNWINDER__, so that will break all
other targets.
I can do a distro build which includes --enable-checking=release
bootstrap/regtest within ~ 24 hours or so, but all that is done without me
having easy access to it, so if it works, fine, if it doesn't, I'll just know
it does not.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #67 from Bernd Edlinger  ---
Created attachment 46200
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46200=edit
updated patch

So, that is what I am going to bootstrap now.
Adds a libgo patch and some minor changes, mostly where to
put the target-attribute (immediately before the function name).
I assume we can apply that to libgo when its ready.
(I wrote a change log entry but actually there is no libgo/ChangeLog)

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #66 from Bernd Edlinger  ---
(In reply to Jakub Jelinek from comment #65)
> Created attachment 46198 [details]
> gcc9-pr89093.patch
> 
> So, can we converge to a single patch that does everything?  Attached is
> completely untested compilation of Ramana's above patch, the above
> discussions and some cleanups/fixes, but I don't have cycles to actually
> verify it and test it.

Thanks, it misses the go and obj-c personality functions.
I can quickly add that (and what I mentioned above regarding
defining TARGET_HARD_FLOAT as TARGET_HARD_FLOAT_SUB &&
!TARGET_GENERAL_REGS_ONLY.

I can do a native armhf boot/reg-test but that will take 4-5 days.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #65 from Jakub Jelinek  ---
Created attachment 46198
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46198=edit
gcc9-pr89093.patch

So, can we converge to a single patch that does everything?  Attached is
completely untested compilation of Ramana's above patch, the above discussions
and some cleanups/fixes, but I don't have cycles to actually verify it and test
it.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #64 from Bernd Edlinger  ---
Okay, using Ian's suggestion I've got this now:

Index: libphobos/libdruntime/gcc/deh.d
===
--- libphobos/libdruntime/gcc/deh.d (revision 270395)
+++ libphobos/libdruntime/gcc/deh.d (working copy)
@@ -28,6 +28,7 @@
 import gcc.unwind.pe;
 import gcc.builtins;
 import gcc.config;
+import gcc.attribute;

 extern(C)
 {
@@ -519,10 +520,19 @@
 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 @@
  * 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 @@
 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 @@
 }
 }

+@personality_fn_attributes
 private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
   _Unwind_Exception_Class
exceptionClass,
   _Unwind_Exception* unwindHeader,

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #63 from Iain Buclaw  ---
(In reply to Jakub Jelinek from comment #59)
> That looks like a D FE bug then.

That shouldn't be difficult, I've create PR d/90136 to keep track of that.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Iain Buclaw  changed:

   What|Removed |Added

 CC||ibuclaw at gdcproject dot org

--- Comment #62 from Iain Buclaw  ---
(In reply to Bernd Edlinger from comment #55)
> But, how about that:
> 

The gcc.attribute module just reuses the UDA mechanism, the value after @ can
be any kind of compile-time evaluated tuple or literal.

So you can do instead. if it makes things much nicer for you.

static if (GNU_ARM_EABI_Unwinder)
enum personality_fn_attributes = attribute("target", ("general-regs-only"))
else
enum personality_fn_attributes = "";

@personality_fn_attributes
private _Unwind_Reason_Code __gdc_personality(...)

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #61 from Jakub Jelinek  ---
At least looking at x86_64-linux gcc/deh.o, I really don't see any .text
comdats, only data comdats, all STT_FUNC symbols are in the same section,
except for the global ctors in .text.startup and dtors in .text.exit.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #60 from Bernd Edlinger  ---
(In reply to Jakub Jelinek from comment #59)
> That looks like a D FE bug then.
> In any case, why can't you just use -mgeneral-regs-only on the deh.d
> compilation command line?

Could work, just anxious, that something in comdat segment depends on this
flag.

Another alternative would be to re-factor the code so that
CONTINUE_UNWINDING just returns _URC_CONTINUE_UNWIND,
and _only_ gdc_personality does call __gnu_unwind_frame and has the
@attribute("target", ("general-regs-only")).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #59 from Jakub Jelinek  ---
That looks like a D FE bug then.
In any case, why can't you just use -mgeneral-regs-only on the deh.d
compilation command line?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #58 from Bernd Edlinger  ---
No, sorry, the attribute on the prototype gets ignored, as the following
is compiled without generating an error:


private int test(double x)
{
  return x > 1.0;
}

static if (GNU_ARM_EABI_Unwinder)
{
@attribute("target", ("general-regs-only"))
private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
  _Unwind_Exception_Class
exceptionClass,
  _Unwind_Exception*
unwindHeader,
  _Unwind_Context* context);
}

private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
  _Unwind_Exception_Class
exceptionClass,
  _Unwind_Exception* unwindHeader,
  _Unwind_Context* context)
{
const(ubyte)* lsda;
_Unwind_Ptr landingPad;
_Unwind_Word cfa;
int handler;

test(3.14);
// Shortcut for phase 2 found handler for domestic exception.
if (actions == (_UA_CLEANUP_PHASE | _UA_HANDLER_FRAME)
&& isGdcExceptionClass(exceptionClass))
{
ExceptionHeader.restore(unwindHeader, handler, lsda, landingPad, cfa);

but with the previous patch calling test(3.14) gives this:
../../../../gcc-trunk/libphobos/libdruntime/gcc/deh.d: In function
'__gdc_personality':
../../../../gcc-trunk/libphobos/libdruntime/gcc/deh.d:906:1: error: argument of
type 'double' not permitted with -mgeneral-regs-only
  906 | test(3.14);
  | ^

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #57 from Bernd Edlinger  ---
(In reply to Jakub Jelinek from comment #56)
> Can't you just add prototypes?
> Like:
> static if (GNU_ARM_EABI_Unwinder)
> {
>   @attribute("target", ("general-regs-only"))
>   private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
>   _Unwind_Exception_Class
> exceptionClass,
>   _Unwind_Exception*
> unwindHeader,
>   _Unwind_Context* context);
> }
> etc. before the actual definitions?  attribute ("", ("")) seems ugly.

I think that is worth a try.  hang on...

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #56 from Jakub Jelinek  ---
Can't you just add prototypes?
Like:
static if (GNU_ARM_EABI_Unwinder)
{
  @attribute("target", ("general-regs-only"))
  private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
  _Unwind_Exception_Class
exceptionClass,
  _Unwind_Exception* unwindHeader,
  _Unwind_Context* context);
}
etc. before the actual definitions?  attribute ("", ("")) seems ugly.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #55 from Bernd Edlinger  ---
But, how about that:

Index: deh.d
===
--- deh.d   (revision 270395)
+++ deh.d   (working copy)
@@ -28,6 +28,7 @@
 import gcc.unwind.pe;
 import gcc.builtins;
 import gcc.config;
+import gcc.attribute;

 extern(C)
 {
@@ -519,10 +520,21 @@
 terminate("unwind error", __LINE__);
 }

+static if (GNU_ARM_EABI_Unwinder)
+{
+enum ATTRIBUTE_NAME = "target";
+enum ATTRIBUTE_ARGS = "general-regs-only";
+}
+else
+{
+enum ATTRIBUTE_NAME = "";
+enum ATTRIBUTE_ARGS = "";
+}

 /**
  * Read and extract information from the LSDA (.gcc_except_table section).
  */
+@attribute(ATTRIBUTE_NAME, (ATTRIBUTE_ARGS))
 _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 +784,7 @@
  * Called when the personality function has found neither a cleanup or
handler.
  * To support ARM EABI personality routines, that must also unwind the stack.
  */
+@attribute(ATTRIBUTE_NAME, (ATTRIBUTE_ARGS))
 _Unwind_Reason_Code CONTINUE_UNWINDING(_Unwind_Exception* unwindHeader,
_Unwind_Context* context)
 {
 static if (GNU_ARM_EABI_Unwinder)
@@ -814,6 +827,7 @@
 static if (GNU_ARM_EABI_Unwinder)
 {
 pragma(mangle, PERSONALITY_FUNCTION)
+@attribute(ATTRIBUTE_NAME, (ATTRIBUTE_ARGS))
 extern(C) _Unwind_Reason_Code gdc_personality(_Unwind_State state,
   _Unwind_Exception*
unwindHeader,
   _Unwind_Context* context)
@@ -873,6 +887,7 @@
 }
 }

+@attribute(ATTRIBUTE_NAME, (ATTRIBUTE_ARGS))
 private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
   _Unwind_Exception_Class
exceptionClass,
   _Unwind_Exception* unwindHeader,

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #54 from Bernd Edlinger  ---
Hmm, I see.  What I am trying to accomplish is, put the target
attribute on every function that calls directly or in-directly
to CONTINUE_UNWINDING.  And do that only for ARM.

For gdc_personality it is straight forward to do, as you pointed out.
But for __gdc_personality and scanLSDA what I would like to do is

static if (GNU_ARM_EABI_Unwinder)
{
  @attribute("target", ("general-regs-only"))
}
private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
  _Unwind_Exception_Class
exceptionClass,
  _Unwind_Exception* unwindHeader,
  _Unwind_Context* context)
{
...


but that does not work, what would work is

static if (GNU_ARM_EABI_Unwinder)
{
  @attribute("target", ("general-regs-only"))
  private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
  _Unwind_Exception_Class
exceptionClass,
  _Unwind_Exception* unwindHeader,
  _Unwind_Context* context)
  {
   ...
  }
}
else
{
  private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
  _Unwind_Exception_Class
exceptionClass,
  _Unwind_Exception* unwindHeader,
  _Unwind_Context* context)
  {
   ...
  }
}

duplicating all that code is of course not an option.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #53 from Jakub Jelinek  ---
(In reply to Bernd Edlinger from comment #52)
> I digged a bit, and found a D syntax for the target attribute,
> it is a bit of a complication since D does not have a pre-processor,
> but an empty target attribute does seem to be ignored without warnings.
> 
> 
> --- libphobos/libdruntime/gcc/deh.d   2019-01-01 13:31:55.0 +0100
> +++ libphobos/libdruntime/gcc/deh.d   2019-04-17 11:24:24.171579381 +0200
> @@ -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 TARGET_ATTRIBUTE = "general-regs-only";
> +}
> +else
> +{
> +enum TARGET_ATTRIBUTE = "";
> +}
>  
>  /**
>   * Read and extract information from the LSDA (.gcc_except_table section).
>   */
> +@attribute("target", (TARGET_ATTRIBUTE))
>  _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 act
>   * Called when the personality function has found neither a cleanup or
> handler.
>   * To support ARM EABI personality routines, that must also unwind the
> stack.
>   */
> +@attribute("target", (TARGET_ATTRIBUTE))
>  _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)
> +@attribute("target", (TARGET_ATTRIBUTE))
>  extern(C) _Unwind_Reason_Code gdc_personality(_Unwind_State state,
>_Unwind_Exception*
> unwindHeader,
>_Unwind_Context* context)
> @@ -873,6 +885,7 @@ else
>  }
>  }
>  
> +@attribute("target", (TARGET_ATTRIBUTE))
>  private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
>_Unwind_Exception_Class
> exceptionClass,
>_Unwind_Exception*
> unwindHeader,

That is not going to work I'm afraid, many targets don't support target
attribute at all.
default_target_option_valid_attribute_p
will then just complain.
Only i386, rs6000, s390, arm, aarch64 and nios2 backends do support those.
On the other side, given the above, I thought all you want to ensure is that
the attribute is on the personality routine, not on the other ones, and the
gdc_personality definition is in
static if (GNU_ARM_EABI_Unwinder)
{
pragma(mangle, PERSONALITY_FUNCTION)
extern(C) _Unwind_Reason_Code gdc_personality(_Unwind_State state,
so can't you just stick @attribute("target", "general-regs-only") to there?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #52 from Bernd Edlinger  ---
I digged a bit, and found a D syntax for the target attribute,
it is a bit of a complication since D does not have a pre-processor,
but an empty target attribute does seem to be ignored without warnings.


--- libphobos/libdruntime/gcc/deh.d 2019-01-01 13:31:55.0 +0100
+++ libphobos/libdruntime/gcc/deh.d 2019-04-17 11:24:24.171579381 +0200
@@ -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 TARGET_ATTRIBUTE = "general-regs-only";
+}
+else
+{
+enum TARGET_ATTRIBUTE = "";
+}

 /**
  * Read and extract information from the LSDA (.gcc_except_table section).
  */
+@attribute("target", (TARGET_ATTRIBUTE))
 _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 act
  * Called when the personality function has found neither a cleanup or
handler.
  * To support ARM EABI personality routines, that must also unwind the stack.
  */
+@attribute("target", (TARGET_ATTRIBUTE))
 _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)
+@attribute("target", (TARGET_ATTRIBUTE))
 extern(C) _Unwind_Reason_Code gdc_personality(_Unwind_State state,
   _Unwind_Exception*
unwindHeader,
   _Unwind_Context* context)
@@ -873,6 +885,7 @@ else
 }
 }

+@attribute("target", (TARGET_ATTRIBUTE))
 private _Unwind_Reason_Code __gdc_personality(_Unwind_Action actions,
   _Unwind_Exception_Class
exceptionClass,
   _Unwind_Exception* unwindHeader,

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #51 from Jakub Jelinek  ---
Author: jakub
Date: Wed Apr 17 08:30:44 2019
New Revision: 270404

URL: https://gcc.gnu.org/viewcvs?rev=270404=gcc=rev
Log:
PR target/89093
* config/arm/arm.c (arm_valid_target_attribute_rec): Don't skip
whitespace at the start of target attribute string.

* gcc.target/arm/pr89093-2.c: New test.

Added:
trunk/gcc/testsuite/gcc.target/arm/pr89093-2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm.c
trunk/gcc/testsuite/ChangeLog

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #50 from Jakub Jelinek  ---
(In reply to Florian Weimer from comment #49)
> (In reply to Bernd Edlinger from comment #48)
> > (In reply to Florian Weimer from comment #47)
> > > (In reply to Bernd Edlinger from comment #43)
> > > > does anybody know what is the Ada and/or D syntax for that?
> > > 
> > > Syntax for what?
> > 
> > I mean the Ada and D equivalent of
> > #pragma GCC target ("general-regs-only")
> > and/or
> > __attribute__((target("general-regs-only")))
> 
> I don't think the target pragma/attribute is supported in Ada.
> 
>pragma Machine_Attribute (Subprogram, "target", "general-regs-only");
> 
> appears to be ignored (even if I use a string which would be accepted by the
> C/C++ pragma for that target, and not "general-regs-only".  But it's been
> years since I did much Ada programming.

I don't understand why we are discussing Ada, because gcc/ada/raise_gcc.c, the
TU containing the Ada personality routine, is written in C.
The only problem is the D personality routine, which is written in D.  And in
that case IMHO we can just use -mgeneral-regs-only on the command line for arm
in the Makefiles or something similar.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #49 from Florian Weimer  ---
(In reply to Bernd Edlinger from comment #48)
> (In reply to Florian Weimer from comment #47)
> > (In reply to Bernd Edlinger from comment #43)
> > > does anybody know what is the Ada and/or D syntax for that?
> > 
> > Syntax for what?
> 
> I mean the Ada and D equivalent of
> #pragma GCC target ("general-regs-only")
> and/or
> __attribute__((target("general-regs-only")))

I don't think the target pragma/attribute is supported in Ada.

   pragma Machine_Attribute (Subprogram, "target", "general-regs-only");

appears to be ignored (even if I use a string which would be accepted by the
C/C++ pragma for that target, and not "general-regs-only".  But it's been years
since I did much Ada programming.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #48 from Bernd Edlinger  ---
(In reply to Florian Weimer from comment #47)
> (In reply to Bernd Edlinger from comment #43)
> > does anybody know what is the Ada and/or D syntax for that?
> 
> Syntax for what?

I mean the Ada and D equivalent of
#pragma GCC target ("general-regs-only")
and/or
__attribute__((target("general-regs-only")))

> 
> I fear we need eagerly load all vector registers before calling the
> personality routine.

I am afraid the unwind handler will probably not know which coprocessor
registers exist before the unwind code hits them.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #47 from Florian Weimer  ---
(In reply to Bernd Edlinger from comment #43)
> does anybody know what is the Ada and/or D syntax for that?

Syntax for what?

I fear we need eagerly load all vector registers before calling the personality
routine.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #46 from Jakub Jelinek  ---
Author: jakub
Date: Sat Apr 13 15:20:46 2019
New Revision: 270339

URL: https://gcc.gnu.org/viewcvs?rev=270339=gcc=rev
Log:
PR target/89093
* config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp
instead of strncmp when checking for thumb and arm.  Formatting fixes.

* gcc.target/arm/pr89093.c: New test.

Added:
trunk/gcc/testsuite/gcc.target/arm/pr89093.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/arm/arm.c
trunk/gcc/testsuite/ChangeLog

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #45 from Ramana Radhakrishnan  ---
(In reply to Jakub Jelinek from comment #42)
> Thanks for the explanation.
> In that case, I think it would be better to just add
> __attribute__((target("general-regs-only")))
> to the 
> #ifdef __ARM_EABI_UNWINDER__
> _Unwind_Reason_Code
> PERSONALITY_FUNCTION (_Unwind_State, struct _Unwind_Exception *,
>   struct _Unwind_Context *);
> decl in unwind-c.c and similarly for eh_personality.cc and to other
> personality routines that use CONTINUE_UNWINDING as well (plus to
> unwind-arm.c and pr-support.c using pragma for everything).

Thanks for all the analysis, this is what I had  - I've been swamped this week
on a few other things, let me get this wrapped up soonish. (read it as during
next week).(In reply to Bernd Edlinger from comment #44)
> Comment on attachment 46013 [details]
> updated patch.
> 
> @@ -122,12 +122,21 @@ extern tree arm_fp16_type_node;
>  #define TARGET_32BIT_P(flags)  (TARGET_ARM_P (flags) || TARGET_THUMB2_P
> (flags))
>  
>  /* Run-time Target Specification.  */
> -/* Use hardware floating point instructions. */
> +/* Use hardware floating point instructions. -mgeneral-regs-only prevents
> +the use of floating point instructions and registers but does not prevent
> +emission of floating point pcs attributes.  */
>  #define TARGET_HARD_FLOAT(arm_float_abi != ARM_FLOAT_ABI_SOFT\
> +  && bitmap_bit_p (arm_active_target.isa, \
> +   isa_bit_vfpv2) \
> +  && TARGET_32BIT \
> +  && !TARGET_GENERAL_REGS_ONLY)
> +
> +#define TARGET_HARD_FLOAT_SUB(arm_float_abi != ARM_FLOAT_ABI_SOFT
> \
>&& bitmap_bit_p (arm_active_target.isa, \
> isa_bit_vfpv2) \
>&& TARGET_32BIT)
> 
> 
> BTW, you could define TARGET_HARD_FLOAT in terms of TARGET_HARD_FLOAT_SUB and
> !TARGET_GENERAL_REGS_ONLY.

Yep I could - been traveling quite a lot and I haven't managed to find someone
else to catch this - I will pick this up next week .

My fault, apologies.

Ramana

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #44 from Bernd Edlinger  ---
Comment on attachment 46013
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46013
updated patch.

@@ -122,12 +122,21 @@ extern tree arm_fp16_type_node;
 #define TARGET_32BIT_P(flags)  (TARGET_ARM_P (flags) || TARGET_THUMB2_P
(flags))

 /* Run-time Target Specification.  */
-/* Use hardware floating point instructions. */
+/* Use hardware floating point instructions. -mgeneral-regs-only prevents
+the use of floating point instructions and registers but does not prevent
+emission of floating point pcs attributes.  */
 #define TARGET_HARD_FLOAT  (arm_float_abi != ARM_FLOAT_ABI_SOFT\
+&& bitmap_bit_p (arm_active_target.isa, \
+ isa_bit_vfpv2) \
+&& TARGET_32BIT \
+&& !TARGET_GENERAL_REGS_ONLY)
+
+#define TARGET_HARD_FLOAT_SUB  (arm_float_abi != ARM_FLOAT_ABI_SOFT\
 && bitmap_bit_p (arm_active_target.isa, \
  isa_bit_vfpv2) \
 && TARGET_32BIT)


BTW, you could define TARGET_HARD_FLOAT in terms of TARGET_HARD_FLOAT_SUB and
!TARGET_GENERAL_REGS_ONLY.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #43 from Bernd Edlinger  ---
does anybody know what is the Ada and/or D syntax for that?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #42 from Jakub Jelinek  ---
Thanks for the explanation.
In that case, I think it would be better to just add
__attribute__((target("general-regs-only")))
to the 
#ifdef __ARM_EABI_UNWINDER__
_Unwind_Reason_Code
PERSONALITY_FUNCTION (_Unwind_State, struct _Unwind_Exception *,
  struct _Unwind_Context *);
decl in unwind-c.c and similarly for eh_personality.cc and to other personality
routines that use CONTINUE_UNWINDING as well (plus to unwind-arm.c and
pr-support.c using pragma for everything).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #41 from Bernd Edlinger  ---
(In reply to Jakub Jelinek from comment #39)
> (and, note, not just C++ personality routine, we have also libgcc/unwind-c.c
> with C personality routine (also changed in the patch) and perhaps
> personality routines for other languages; I see e.g. libobjc/exception.c or
> gcc/ada/raise-gcc.c not patched though, and
> libphobos/libdruntime/gcc/deh.d), but perhaps it only cares about the
> personality routine itself and not on what it calls.  This needs to be
> clarified.

Yes you are absolutely right about objC, Ada, and D.

At the very least the personality routine, and all functions
that are calling __gnu_unwind_frame need that pragma.
in obj-C it is a macro CONTINUE_UNWINDING, that is fine.

In ada it is continue_unwind a function, that calls __gnu_unwind_frame,
and personality_body calling continue_unwind,
and the personality function itself.

In D it is CONTINUE_UNWINDING, a function.
scanLSDA, __gdc_personality, and gdc_personality itself.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #40 from Bernd Edlinger  ---
Not that I invented this, but as far as I understand,
normally the interrupted execution context registers are saved on a
register file in memory.  But not on ARM.
On arm only the core registers are saved on a register file,
and the non-core, VFP registers are only saved when needed.
When the unwinder needs to restore a VFP register it is
done in _Unwind_VRS_Pop, and when in phase 1 this function
does save the current VFP register bank and does modify the
live VFP registers. Only when core registers are modified that
operates on the register file.
Therefore only the functions that invoke __gnu_unwind_execute
directly or indirectly must not save & restore any non-core registers
in the prologue and epilogue.
When function that saves & restores the d8-d15 but does clobber
d0-d7, that could cause those registers to be different than what
they would be otherwise.
It is possible that _Unwind_VRS_Pop could restore also call-clobbered
registers d0-d7, but I don't know if those registers would
actually need to be restored to their previous state.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #39 from Jakub Jelinek  ---
(In reply to Richard Biener from comment #38)
> Isn't the issue also latent on all branches?

It is, but we have been lucky that the RA didn't decide to emit that.
On the trunk (unless something changed in RA since end of January) it
unfortunately results in real issues, so we really can't ship GCC 9 with that.

(In reply to Bernd Edlinger from comment #37)
> If a non-general-regs-only function is called from here,
> it will only preserve d8-d15, and the call-clobbered registers
> d0-d7 would of course be modified.
> But is that a problem at all, if the call-clobbered registers are not
> restored?

I'm afraid I don't understand enough why the floating registers can't be used
in the personality routine and other unwinder routines.  I'd think that even in
the
personality routine itself the d8-d15 registers if they are used will be
saved/restored first, so if that wasn't working, I'd say any use of those
registers in the personality routine and anything it calls are problematic
(and, note, not just C++ personality routine, we have also libgcc/unwind-c.c
with C personality routine (also changed in the patch) and perhaps personality
routines for other languages; I see e.g. libobjc/exception.c or
gcc/ada/raise-gcc.c not patched though, and libphobos/libdruntime/gcc/deh.d),
but perhaps it only cares about the personality routine itself and not on what
it calls.  This needs to be clarified.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #38 from Richard Biener  ---
Isn't the issue also latent on all branches?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #37 from Bernd Edlinger  ---
If a non-general-regs-only function is called from here,
it will only preserve d8-d15, and the call-clobbered registers
d0-d7 would of course be modified.
But is that a problem at all, if the call-clobbered registers are not restored?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #36 from Jakub Jelinek  ---
(In reply to Jonathan Wakely from comment #35)
> (In reply to Bernd Edlinger from comment #33)
> > (In reply to Ramana Radhakrishnan from comment #32)
> > > 
> > > Either I drop the warning or I keep the hunk in eh_personality.cc - any
> > > preferences / thoughts ?
> > 
> > It would feel safer, if only the functions that need it
> > had a target attribute like:
> > 
> > _Unwind_Reason_Code
> > #ifdef __ARM_EABI_UNWINDER__
> > __attribute__((target("general-regs-only")))
> > PERSONALITY_FUNCTION (_Unwind_State state,
> >   struct _Unwind_Exception* ue_header,
> >   struct _Unwind_Context* context)
> 
> Agreed - will this work instead?

Aren't there many functions that are inlined into it (then it would be fine)
but could not be inlined (and then it would be a problem, at least in theory)?
Though, if there are inlines that are used in other TUs and those TUs are not
general-regs-only and we decide not to inline, then it might be pure luck if
COMDAT is won bu eh-personality.o versions or some other ones.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #35 from Jonathan Wakely  ---
(In reply to Bernd Edlinger from comment #33)
> (In reply to Ramana Radhakrishnan from comment #32)
> > 
> > Either I drop the warning or I keep the hunk in eh_personality.cc - any
> > preferences / thoughts ?
> 
> It would feel safer, if only the functions that need it
> had a target attribute like:
> 
> _Unwind_Reason_Code
> #ifdef __ARM_EABI_UNWINDER__
> __attribute__((target("general-regs-only")))
> PERSONALITY_FUNCTION (_Unwind_State state,
>   struct _Unwind_Exception* ue_header,
>   struct _Unwind_Context* context)

Agreed - will this work instead?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #34 from Jakub Jelinek  ---
@@ -30877,6 +30883,11 @@ arm_valid_target_attribute_rec (tree args, struct
gcc_options *opts)
   else if (!strncmp (q, "arm", 3))
  opts->x_target_flags &= ~MASK_THUMB;

+  else if (!strncmp (q, "general-regs-only", strlen
("general-regs-only")))
+   {
+ opts->x_target_flags |= MASK_GENERAL_REGS_ONLY;
+   }
+
   else if (!strncmp (q, "fpu=", 4))
{
  int fpu_index;

I'm really not sure I understand this strncmp (but also the ones for arm and
thumb), does that mean you want to accept also general-regs-only123 or
general-regs-onlycorgewaldo or thumb__ ?  If you want to support e.g. only
optional whitespace after the string, each handled case should update the
pointer and then something after it should verify/diagnose.
Also, single stmt if bodies shouldn't be wrapped with {} and the arm case is
misindented.

Otherwise, I think the eh_personality.cc change is acceptable to me (but ask
Jonathan or other libstdc++ maintainers).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #33 from Bernd Edlinger  ---
(In reply to Ramana Radhakrishnan from comment #32)
> 
> Either I drop the warning or I keep the hunk in eh_personality.cc - any
> preferences / thoughts ?

It would feel safer, if only the functions that need it
had a target attribute like:

_Unwind_Reason_Code
#ifdef __ARM_EABI_UNWINDER__
__attribute__((target("general-regs-only")))
PERSONALITY_FUNCTION (_Unwind_State state,
  struct _Unwind_Exception* ue_header,
  struct _Unwind_Context* context)

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Ramana Radhakrishnan  changed:

   What|Removed |Added

  Attachment #45552|0   |1
is obsolete||
  Attachment #45580|0   |1
is obsolete||

--- Comment #32 from Ramana Radhakrishnan  ---
Created attachment 46013
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46013=edit
updated patch.

Having discussed this with Richard further , instead of adding -mfpu=none , we
would prefer a mgeneral-regs-only option which keeps the 2 separate. 

I'm sorry about the time it has taken to get back but this is what I have right
now. The hunk in eh_personality.cc isn't very pleasing for me but that's
because of the warning in the ABI code which triggers on the build for a hard
float build because we do have inline functions consuming floating point. 


Either I drop the warning or I keep the hunk in eh_personality.cc - any
preferences / thoughts ?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #31 from Jakub Jelinek  ---
(In reply to Ramana Radhakrishnan from comment #30)
> (In reply to Jakub Jelinek from comment #29)
> > Ramana, any progress on this?
> 
> I'm still trying to get the various spec files and the t-multilib bits
> sorted and half-term has intervened here in the UK.

Any further progress?  This is a wrong-code P1, so really has to be fixed for
GCC9.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #30 from Ramana Radhakrishnan  ---
(In reply to Jakub Jelinek from comment #29)
> Ramana, any progress on this?

I'm still trying to get the various spec files and the t-multilib bits sorted
and half-term has intervened here in the UK.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #29 from Jakub Jelinek  ---
Ramana, any progress on this?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #28 from Bernd Edlinger  ---
(In reply to Ramana Radhakrishnan from comment #27)
> (In reply to Bernd Edlinger from comment #25)
> > you might consider adding something like that to your patch:
> > 
> > Index: elf.h
> > ===
> > --- elf.h   (revision 268337)
> > +++ elf.h   (working copy)
> > @@ -64,7 +64,7 @@
> >  %{mapcs-*:-mapcs-%*} \
> >  %(subtarget_asm_float_spec) \
> >  %{mthumb-interwork:-mthumb-interwork} \
> > -%{mfloat-abi=*} %{!mfpu=auto: %{mfpu=*}} \
> > +%{mfloat-abi=*} %{!mfpu=auto: %{!mfpu=none: %{mfpu=*}}} \
> >  %(subtarget_extra_asm_spec)"
> >  #endif
> >  
> > 
> > 
> > otherwise using -mfpu=none won't work on the command line.
> > becuse gas does not understand it.
> 
> Yes, that's what I've been playing with. I've run out of time this week
> because of other work commitments, I hope to get back to this early next
> week.
> 
> Ramana

It is a bit unfortunate that -mfpu= on the command line seems to have
an impact on the eabi_attribute section, while the pragma does not:

  if (TARGET_HARD_FLOAT && TARGET_VFP_SINGLE)
arm_emit_eabi_attribute ("Tag_ABI_HardFP_use", 27, 1);

  if (TARGET_HARD_FLOAT_ABI)
arm_emit_eabi_attribute ("Tag_ABI_VFP_args", 28, 1);

where TARGET_HARD_FLOAT and TARGET_VFP_SINGLE are defined as follows:

#define TARGET_HARD_FLOAT   (arm_float_abi != ARM_FLOAT_ABI_SOFT\
 && bitmap_bit_p (arm_active_target.isa, \
  isa_bit_vfpv2) \
 && TARGET_32BIT)

#define TARGET_VFP_DOUBLE (bitmap_bit_p (arm_active_target.isa,
isa_bit_fp_dbl))

#define TARGET_VFP_SINGLE (!TARGET_VFP_DOUBLE)


But arm_active_target.isa seems to depend on the effective -mfpu= command line
option, while later in the code generation the same macros are used to
select the use of VFP instructions.

I wonder if it would be better to have an orthogonal way to specify
the used ABI and the used register banks.

So like -mfpu=vfpv3-d16 and -mno-vfp,
where -mfpu= affects the eabi_attribute only
and -mno-vfp makes sure that no VFP registers are used, and, in particular
if functions are defined or called, where the ABI is incompatible because it
passes the values in VFP registers, that should be diagnosed, because it
will not work as expected.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #27 from Ramana Radhakrishnan  ---
(In reply to Bernd Edlinger from comment #25)
> you might consider adding something like that to your patch:
> 
> Index: elf.h
> ===
> --- elf.h (revision 268337)
> +++ elf.h (working copy)
> @@ -64,7 +64,7 @@
>  %{mapcs-*:-mapcs-%*} \
>  %(subtarget_asm_float_spec) \
>  %{mthumb-interwork:-mthumb-interwork} \
> -%{mfloat-abi=*} %{!mfpu=auto: %{mfpu=*}} \
> +%{mfloat-abi=*} %{!mfpu=auto: %{!mfpu=none: %{mfpu=*}}} \
>  %(subtarget_extra_asm_spec)"
>  #endif
>  
> 
> 
> otherwise using -mfpu=none won't work on the command line.
> becuse gas does not understand it.

Yes, that's what I've been playing with. I've run out of time this week because
of other work commitments, I hope to get back to this early next week.

Ramana

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #26 from Jakub Jelinek  ---
(In reply to Bernd Edlinger from comment #25)
> you might consider adding something like that to your patch:
> 
> Index: elf.h
> ===
> --- elf.h (revision 268337)
> +++ elf.h (working copy)
> @@ -64,7 +64,7 @@
>  %{mapcs-*:-mapcs-%*} \
>  %(subtarget_asm_float_spec) \
>  %{mthumb-interwork:-mthumb-interwork} \
> -%{mfloat-abi=*} %{!mfpu=auto: %{mfpu=*}} \
> +%{mfloat-abi=*} %{!mfpu=auto: %{!mfpu=none: %{mfpu=*}}} \
>  %(subtarget_extra_asm_spec)"
>  #endif
>  
> 
> 
> otherwise using -mfpu=none won't work on the command line.
> becuse gas does not understand it.

And if that works, then it might be cleaner to add -mfpu=none in
libgcc/config/arm/t-arm for the libgcc objects.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #25 from Bernd Edlinger  ---
you might consider adding something like that to your patch:

Index: elf.h
===
--- elf.h   (revision 268337)
+++ elf.h   (working copy)
@@ -64,7 +64,7 @@
 %{mapcs-*:-mapcs-%*} \
 %(subtarget_asm_float_spec) \
 %{mthumb-interwork:-mthumb-interwork} \
-%{mfloat-abi=*} %{!mfpu=auto: %{mfpu=*}} \
+%{mfloat-abi=*} %{!mfpu=auto: %{!mfpu=none: %{mfpu=*}}} \
 %(subtarget_extra_asm_spec)"
 #endif



otherwise using -mfpu=none won't work on the command line.
becuse gas does not understand it.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Andrew Pinski  changed:

   What|Removed |Added

 CC||bernd.edlinger at hotmail dot 
de

--- Comment #24 from Andrew Pinski  ---
*** Bug 89228 has been marked as a duplicate of this bug. ***

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #23 from Jakub Jelinek  ---
Created attachment 45580
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45580=edit
gcc9-pr89093.patch

This is what we are successfully using in Fedora for now (passed
bootstrap/regtest and fixed the issues).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #22 from Jakub Jelinek  ---
One more issue, shouldn't the #pragma GCC target be added before all include
files?  Various define many inline functions, e.g. unwind-pe.h or unwind-cxx.h.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Ramana Radhakrishnan  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |ramana at gcc dot 
gnu.org

--- Comment #21 from Ramana Radhakrishnan  ---
(In reply to Jakub Jelinek from comment #19)
> (In reply to Florian Weimer from comment #18)
> > (In reply to Ramana Radhakrishnan from comment #15)
> > > Testing this and would be grateful for a test run.
> > 
> > Is this hunk needed as well, or will the unwinding information take care of
> > this?  (__cxa_call_unexpected has another d8 register spill.)
> 
> No idea here.

I'll try and analyse that - The key is ensuring that there is absolutely no
floating point code in eh_call.cc , if there is likely to be floating point
anywhere this isn't correct 

> 
> > --- libstdc++-v3/libsupc++/eh_call.cc   (revision 268364)
> > +++ libstdc++-v3/libsupc++/eh_call.cc   (working copy)
> > @@ -22,6 +22,11 @@
> >  // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >  // .
> >  
> > +#ifdef __arm__
> > +#pragma GCC target ("fpu=none")
> > +#pragma GCC push_options
> > +#endif
> 
> But why the #pragma GCC push_options?  That makes no sense.
> Either you need to push options before GCC target and pop later on, but if
> you pop at the end of TU and don't really expect anything else to be emitted
> there, only #pragma GCC target should be enough (that applies to the other
> patch too).

I think that's just percolated my quick hack to discuss the issue. 

It should be enough to do #pragma GCC target . The final patch I have does that
. Thanks for confirming that the patch in it's essence fixes up the issue.

regards
Ramana

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #20 from Florian Weimer  ---
(In reply to Ramana Radhakrishnan from comment #15)
> Created attachment 45552 [details]
> new patch.
> 
> Testing this and would be grateful for a test run.

I can confirm that this patch fixes the glibc test suite failure,
nptl/tst-thread-exit-clobber.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

 CC||redi at gcc dot gnu.org

--- Comment #19 from Jakub Jelinek  ---
(In reply to Florian Weimer from comment #18)
> (In reply to Ramana Radhakrishnan from comment #15)
> > Testing this and would be grateful for a test run.
> 
> Is this hunk needed as well, or will the unwinding information take care of
> this?  (__cxa_call_unexpected has another d8 register spill.)

No idea here.

> --- libstdc++-v3/libsupc++/eh_call.cc   (revision 268364)
> +++ libstdc++-v3/libsupc++/eh_call.cc   (working copy)
> @@ -22,6 +22,11 @@
>  // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  // .
>  
> +#ifdef __arm__
> +#pragma GCC target ("fpu=none")
> +#pragma GCC push_options
> +#endif

But why the #pragma GCC push_options?  That makes no sense.
Either you need to push options before GCC target and pop later on, but if you
pop at the end of TU and don't really expect anything else to be emitted there,
only #pragma GCC target should be enough (that applies to the other patch too).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #18 from Florian Weimer  ---
(In reply to Ramana Radhakrishnan from comment #15)
> Created attachment 45552 [details]
> new patch.
> 
> Testing this and would be grateful for a test run.

Is this hunk needed as well, or will the unwinding information take care of
this?  (__cxa_call_unexpected has another d8 register spill.)

Index: libstdc++-v3/libsupc++/eh_call.cc
===
--- libstdc++-v3/libsupc++/eh_call.cc   (revision 268364)
+++ libstdc++-v3/libsupc++/eh_call.cc   (working copy)
@@ -22,6 +22,11 @@
 // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 // .

+#ifdef __arm__
+#pragma GCC target ("fpu=none")
+#pragma GCC push_options
+#endif
+
 #include 
 #include 
 #include 

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #17 from Florian Weimer  ---
(In reply to Ramana Radhakrishnan from comment #15)
> Created attachment 45552 [details]
> new patch.
> 
> Testing this and would be grateful for a test run.

I believe the #pragma GCC push_options needs to come first, but it shouldn't
matter in this context because the option set is never actually restored.

(I have not tested the patch yet.)

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

   Priority|P3  |P1

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #16 from Ramana Radhakrishnan  ---
(In reply to Jakub Jelinek from comment #14)
> We require GNU make, so one can use something like:
> unwind-arm.o unwind-c.o libunwind.o pr-support.o: CFLAGS += -mfpu=none
> or similar in libgcc/config/arm/t-arm (or similar) with a comment explaining
> the reason.  For eh_personality.o that needs to be done elsewhere and there
> are no such makefile fragments (and libtool is used).

Sadly that doesn't work for -mfpu=none in t-arm because we still need gcc-9 to
build with older binutils that don't necessarily support -mfpu=none, thus for
now let's hide this with target pragmas.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Ramana Radhakrishnan  changed:

   What|Removed |Added

  Attachment #45547|0   |1
is obsolete||

--- Comment #15 from Ramana Radhakrishnan  ---
Created attachment 45552
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45552=edit
new patch.

Testing this and would be grateful for a test run.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #14 from Jakub Jelinek  ---
We require GNU make, so one can use something like:
unwind-arm.o unwind-c.o libunwind.o pr-support.o: CFLAGS += -mfpu=none
or similar in libgcc/config/arm/t-arm (or similar) with a comment explaining
the reason.  For eh_personality.o that needs to be done elsewhere and there are
no such makefile fragments (and libtool is used).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Ramana Radhakrishnan  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2019-01-29
 Ever confirmed|0   |1

--- Comment #13 from Ramana Radhakrishnan  ---
(In reply to Jakub Jelinek from comment #12)
> If one appends -mfloat-abi=soft to command lines of those files, does that
> imply incompatible ABI even if nothing is passed in float/VFP etc. registers
> nor there is any floating point code?

-mfloat-abi=soft is an interesting option, it means use floating point
emulation code using the base pcs as well as use the base parameter passing
conventions for passing floating point parameters to functions. 

so it would end up failing at link time . That's why we need an -mfpu=none
option which is silent and I've not liked it for a while. 

(In reply to Jakub Jelinek from comment #11)
> Comment on attachment 45547 [details]
> untested prototype patch.
> 
> Doesn't the whole unwinder (so eh_personality.cc (whole, not just one
> function in it), unwind-arm.c, unwind-c.c, maybe some other unwind-*.c))
> need that?

Yes that would be needed. Reading the EHABI again suggests that - I don't see a
macro that would help with that everywhere.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #12 from Jakub Jelinek  ---
If one appends -mfloat-abi=soft to command lines of those files, does that
imply incompatible ABI even if nothing is passed in float/VFP etc. registers
nor there is any floating point code?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #11 from Jakub Jelinek  ---
Comment on attachment 45547
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45547
untested prototype patch.

Doesn't the whole unwinder (so eh_personality.cc (whole, not just one function
in it), unwind-arm.c, unwind-c.c, maybe some other unwind-*.c)) need that?

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #10 from Ramana Radhakrishnan  ---
Created attachment 45547
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45547=edit
untested prototype patch.

Not sure if this is complete yet but it gives a framework to dig further.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #9 from Jakub Jelinek  ---
If the EABI has such requirements, then libgcc/config/arm/t-arm (or whatever
else) needs to pass down -msoft-float (or whatever else disables the VFP
registers), rather than relying on that the compiler won't choose them.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #8 from joseph at codesourcery dot com  ---
The Arm rule is that the EH machinery needs to avoid using VFP (or other 
non-core) registers so that the unwinder can save them on-demand only.  
See 
 
section 4.7.

This means the unwind support code in libgcc, and personality routines, 
need to be built not to use such registers (possibly with a series of 
-ffixed-* options).  To complicate things, some of that code can end up 
calling memcpy.  While __aeabi_memcpy is required to use core registers 
only, that doesn't apply to memcpy.  (There are also possible glibc 
dynamic linker issues, 
.)

I don't guarantee this issue with demand-saving of VFP registers is what's 
involved here, but it's a potential issue I identified when looking for 
Arm ABI issues in the GNU toolchain in 2008 (and included in a list of 
such issues sent to Arm in 2010).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[ramana at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Ramana Radhakrishnan  changed:

   What|Removed |Added

 CC||ramana at gcc dot gnu.org

--- Comment #7 from Ramana Radhakrishnan  ---
(In reply to Florian Weimer from comment #0)
> In glibc, we have a test, nptl/tst-thread-exit-clobber, that attempts to
> verify if registers are properly restored by unwinding.  (The actual target
> of the the test is pthread_exit, but it covers more than that.)
> 
> This tests fails when running with GCC 9 libstdc++, even if glibc and the
> test were built with GCC 8, and libgcc_s is replaced with the version for
> GCC 8 (which works when running against GCC 8 libstdc++).
> 
> In the test, the d8 register is not restored properly during unwinding, it
> is set to zero.  d9, d10 etc. are restored.
> 
> I noticed that in GCC 9, __gxx_personality_v0 saves the d8 VFP register:
> 
> 0007b620 <__gxx_personality_v0@@CXXABI_1.3>:
>7b620:   e92d4ff0push{r4, r5, r6, r7, r8, r9, sl, fp, lr}
>7b624:   ed2d8b02vpush   {d8}
>7b628:   e3a03000mov r3, #0
>7b62c:   e1a08001mov r8, r1
> 
> And it actually uses s16 and s17, apparently for spilling integer registers.
> Perhaps the unwinder is not prepared to deal with that.

d8 is composed of s16 and s17. That should just be fine. The single precision
FP registers are packed into double precision registers in the VFP
architecture.

Ramana

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #6 from Florian Weimer  ---
Okay, please assume that __gxx_personality_v0 is a red herring.  Apparently,
there is unwinding information for VFP registers, too.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #5 from Jakub Jelinek  ---
I admit I don't know much about ARM unwinding, which is different from all the
other arches, but normally the personality routine doesn't call the landing pad
code nor anything similar, it should set some registers in unwind context and
return and through return value and whatever it stores into the unwind context
tell the caller what to do (whether to install the context etc.).

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #4 from Florian Weimer  ---
So I'm not really an Arm person or a GCC person, but doesn't the personality
routine call the landing pad, as identified by the LDSA?  And then that ends
with a call to __cxa_end_cleanup, which is clear a no-return function (I think
other targets call this function _Unwind_Resume).  So the epilogue of the
personality routine never runs.

I doubt the Arm unwinding information covers the the VFP registers, so there is
not much the unwinder can do about this.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

--- Comment #3 from Jakub Jelinek  ---
Like in https://bugzilla.redhat.com/show_bug.cgi?id=1670069 (another wrong-code
that has been reported to us today), eh_personality.cc (__gxx_personality_v0)
is compiled this way since r266385.  Though, in this case it seems more like a
missed optimization, at least I still fail to understand what is incorrect on
it.

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek  ---
It is indeed saved in the prologue:
@ args = 0, pretend = 0, frame = 64
@ frame_needed = 0, uses_anonymous_args = 0
and ip, r0, #3
push{r4, r5, r6, r7, r8, r9, r10, fp, lr}
.save {r4, r5, r6, r7, r8, r9, r10, fp, lr}
cmp ip, #1
vpush.64{d8}
.vsave {d8}
mov r3, #0
.pad #76
sub sp, sp, #76
but it is restored in the epilogue:
.L61:
add sp, sp, #76
@ sp needed
vldmsp!, {d8}
pop {r4, r5, r6, r7, r8, r9, r10, fp, pc}
and there are no other returns that I can see.  So, why isn't it restored
there?
Has the stack pointer been clobbered, or the stack slot in there?
eh_personality.cc has been compiled with
-mtune=cortex-a9 -mfloat-abi=hard -mfpu=vfpv3-d16 -mtls-dialect=gnu -marm
-march=armv7-a+fp -O2 -fPIC

[Bug target/89093] [9 Regression] C++ exception handling clobbers d8 VFP register

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89093

Andrew Pinski  changed:

   What|Removed |Added

  Known to work||8.1.0
   Target Milestone|--- |9.0
Summary|C++ exception handling  |[9 Regression] C++
   |clobbers d8 VFP register|exception handling clobbers
   ||d8 VFP register

--- Comment #1 from Andrew Pinski  ---
According to https://sourceware.org/ml/libc-alpha/2019-01/msg00651.html it
worked in GCC 8.1.0.