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: e92d4ff0 push {r4, r5, r6, r7, r8, r9, sl, fp, lr} 7b624: ed2d8b02 vpush {d8} 7b628: e3a03000 mov r3, #0 7b62c: e1a08001 mov r8, r1 And it actually uses s16 and s17, apparently for spilling integer registers. Perhaps the unwinder is not prepared to deal with that. This happens with gcc version 9.0.0 20190119 (Red Hat 9.0.0-0.3) (GCC), built with: --with-tune=generic-armv7-a --with-arch=armv7-a --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux --build=armv7hl-redhat-linux-gnueabi
According to https://sourceware.org/ml/libc-alpha/2019-01/msg00651.html it worked in GCC 8.1.0.
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 vldm sp!, {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
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.
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.
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.).
Okay, please assume that __gxx_personality_v0 is a red herring. Apparently, there is unwinding information for VFP registers, too.
(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: e92d4ff0 push {r4, r5, r6, r7, r8, r9, sl, fp, lr} > 7b624: ed2d8b02 vpush {d8} > 7b628: e3a03000 mov r3, #0 > 7b62c: e1a08001 mov 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
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 <http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf> 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, <https://sourceware.org/bugzilla/show_bug.cgi?id=15792>.) 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).
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.
Created attachment 45547 [details] untested prototype patch. Not sure if this is complete yet but it gives a framework to dig further.
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?
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?
(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.
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).
Created attachment 45552 [details] new patch. Testing this and would be grateful for a test run.
(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.
(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.)
(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 // <http://www.gnu.org/licenses/>. +#ifdef __arm__ +#pragma GCC target ("fpu=none") +#pragma GCC push_options +#endif + #include <bits/c++config.h> #include <cstdlib> #include <bits/exception_defines.h>
(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 > // <http://www.gnu.org/licenses/>. > > +#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).
(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.
(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 > > // <http://www.gnu.org/licenses/>. > > > > +#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
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.
Created attachment 45580 [details] gcc9-pr89093.patch This is what we are successfully using in Fedora for now (passed bootstrap/regtest and fixed the issues).
*** Bug 89228 has been marked as a duplicate of this bug. ***
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.
(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.
(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
(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.
Ramana, any progress on this?
(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.
(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.
Created attachment 46013 [details] 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 ?
(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)
@@ -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).
(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?
(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.
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?
Isn't the issue also latent on all branches?
(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.
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.
(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.
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).
does anybody know what is the Ada and/or D syntax for that?
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.
(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
Author: jakub Date: Sat Apr 13 15:20:46 2019 New Revision: 270339 URL: https://gcc.gnu.org/viewcvs?rev=270339&root=gcc&view=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
(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.
(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.
(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.
(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.
Author: jakub Date: Wed Apr 17 08:30:44 2019 New Revision: 270404 URL: https://gcc.gnu.org/viewcvs?rev=270404&root=gcc&view=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
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.000000000 +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,
(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.000000000 +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?
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.
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,
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.
(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...
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); | ^
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?
(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")).
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.
(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(...)
(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.
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,
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.
(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.
Created attachment 46200 [details] 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)
Comment on attachment 46200 [details] 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.
Created attachment 46202 [details] gcc9-pr89093.patch So, here is the above patch updated in the one libgo hunk.
Yes, thanks, now switching to your latest patch.
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
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
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.
Created attachment 46203 [details] gcc9-pr89093.patch Same patch, just guard the target attribute with a version check, so that supported boot-strap configurations should work
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.
Created attachment 46205 [details] gcc9-pr89093.patch Updated patch.
Created attachment 46220 [details] 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.
Author: jakub Date: Tue Apr 23 10:03:41 2019 New Revision: 270504 URL: https://gcc.gnu.org/viewcvs?rev=270504&root=gcc&view=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
Fixed for all languages but Ada and Go, neither of them is release critical.
Author: jakub Date: Wed Apr 24 08:16:07 2019 New Revision: 270535 URL: https://gcc.gnu.org/viewcvs?rev=270535&root=gcc&view=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
Fixed for Ada as well, only Go left to do.
Author: ian Date: Wed Apr 24 12:45:45 2019 New Revision: 270542 URL: https://gcc.gnu.org/viewcvs?rev=270542&root=gcc&view=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 <iant@golang.org> Modified: trunk/gcc/go/gofrontend/MERGE trunk/libgo/runtime/go-unwind.c
Fixed then.
Author: jakub Date: Tue Apr 30 12:07:27 2019 New Revision: 270690 URL: https://gcc.gnu.org/viewcvs?rev=270690&root=gcc&view=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
Hi Jakub, commit r270690 seems to have caused a regression on gcc.target/aarch64/return_address_sign_3.c
for aarch64-none-linux-gnu. I am still building the toolchain to take a look so not able to give more detail.
Author: jakub Date: Tue Apr 30 16:30:44 2019 New Revision: 270705 URL: https://gcc.gnu.org/viewcvs?rev=270705&root=gcc&view=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
Thanks Jakub!