Hi Christophe, Sorry for the delay.
> -----Original Message----- > From: Gcc-patches <gcc-patches-boun...@gcc.gnu.org> On Behalf Of > Christophe Lyon via Gcc-patches > Sent: 29 April 2020 16:19 > To: gcc-patches@gcc.gnu.org > Subject: [PATCH] arm: Warn if IRQ handler is not compiled with -mgeneral- > regs-only [PR target/94743] > > The interrupt attribute does not guarantee that the FP registers are > saved, which can result in problems difficult to debug. > > Saving the FP registers and status registers can be a large penalty, > so it's probably not desirable to do that all the time. > > If the handler calls other functions, we'd likely need to save all of > them, for lack of knowledge of which registers they actually use. > > This is even more obscure for the end-user when the compiler inserts > calls to helper functions such as memcpy (some multilibs do use FP > registers to speed it up). > > In the PR, we discussed adding routines in libgcc to save the FP > context and saving only locally-clobbered FP registers, but I think > this is too intrusive for stage 4. > > In the mean time, emit a warning to suggest re-compiling with > -mgeneral-regs-only. Note that this can lead to errors if the code > uses floating-point and -mfloat-abi=hard, eg: > argument of type 'double' not permitted with -mgeneral-regs-only > > This can be troublesome for the user, but at least this would make > them aware of the latent issue. > > The patch adds two testcases: > - pr94734-1.c checks that a warning is emitted. One function can make > implicit calls to runtime floating-point routines, the other one > doesn't. We can improve the diagnostic later not to warn in the > second case. > > - pr94734-2.c checks that no warning is emitted when using > -mgeneral-regs-only. > > 2020-04-29 Christophe Lyon <christophe.l...@linaro.org> > > PR target/94743 > gcc/ > * config/arm/arm.c (arm_handle_isr_attribute): Warn if > -mgeneral-regs-only is not used. > > gcc/testsuite/ > * gcc.target/arm/pr94743-1.c: New test. > * gcc.target/arm/pr94743-2.c: New test. > --- > gcc/config/arm/arm.c | 5 +++++ > gcc/testsuite/gcc.target/arm/pr94743-1.c | 20 ++++++++++++++++++++ > gcc/testsuite/gcc.target/arm/pr94743-2.c | 17 +++++++++++++++++ > 3 files changed, 42 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/arm/pr94743-1.c > create mode 100644 gcc/testsuite/gcc.target/arm/pr94743-2.c > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 6a6e804..34aad1d 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -7176,6 +7176,11 @@ arm_handle_isr_attribute (tree *node, tree name, > tree args, int flags, > name); > *no_add_attrs = true; > } > + else if (TARGET_VFP_BASE) > + { > + warning (OPT_Wattributes, "FP registers might be clobbered > despite %qE attribute: compile with -mgeneral-regs-only", > + name); > + } Let's use %< and %> to quote -mgeneral-regs-only properly. Ok with that change. Thanks, Kyrill > /* FIXME: the argument if any is checked for type attributes; > should it be checked for decl ones? */ > } > diff --git a/gcc/testsuite/gcc.target/arm/pr94743-1.c > b/gcc/testsuite/gcc.target/arm/pr94743-1.c > new file mode 100644 > index 0000000..67700c6 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/pr94743-1.c > @@ -0,0 +1,20 @@ > +/* PR target/94743 */ > +/* { dg-do compile } */ > + > +typedef struct { > + double fpdata[32]; > +} dummy_t; > + > +dummy_t global_d; > +dummy_t global_d1; > + > +__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test(void) > +{ /* { dg-warning { FP registers might be clobbered despite 'interrupt' > attribute: compile with -mgeneral-regs-only} "" { target *-*-* } . } */ > + global_d.fpdata[3] += global_d.fpdata[3] * global_d1.fpdata[3]; > +} > + > + > +__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test2(void) > +{ /* { dg-warning { FP registers might be clobbered despite 'interrupt' > attribute: compile with -mgeneral-regs-only} "" { target *-*-* } . } */ > + global_d.fpdata[3] = 1.0; > +} > diff --git a/gcc/testsuite/gcc.target/arm/pr94743-2.c > b/gcc/testsuite/gcc.target/arm/pr94743-2.c > new file mode 100644 > index 0000000..745fd36 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/pr94743-2.c > @@ -0,0 +1,17 @@ > +/* PR target/94743 */ > +/* { dg-do compile } */ > +/* { dg-options "-mgeneral-regs-only" } */ > + > +typedef struct { > + /* Do not use floating-point types, which are not be compatible with > + -mgeneral-regs-only under -mfloat-abi=hard */ > + int fpdata[32]; > +} dummy_t; > + > +dummy_t global_d; > +dummy_t global_d1; > + > +__attribute__ ((interrupt("IRQ"))) void IRQ_HDLR_Test(void) > +{ > + global_d.fpdata[3] += global_d.fpdata[3] * global_d1.fpdata[3]; > +} > -- > 2.7.4