Hi,
> > You also need to do the same thing for TARGET_HARD_REGNO_SCRATCH_OK,
> > to stop peephole2 from using unsaved registers as scratch registers.
> >
> > I should dig out my patches to clean up this interface. It's just
> > too brittle to have two macros that say what registers can be used
> > after reload.
Indeed. I naively thought that there would be a single place that needed
an update and overlooked this hook.
> Gosh, I don't like the interface of them at all. I have interrupt functions
> and I want all the goodness out of gcc and I want gcc to just know that it
> can't use registers it doesn't want to save for any reason that the port
> marked as saved by the calling api of the function. :-( Very few ports (3)
> define TARGET_HARD_REGNO_SCRATCH_OK and many port (19 or so) don't define
> this but do seem to have interrupt functions. The existing documentation
> seems not give be enough information to let me decide if I need to define it
> or not. I read through all the ports that do define it, and none of them
> shed any light on to allow me to decide if my port needs to or not.
>
> So, first question is, are the 16 other ports that have interrupt functions
> and don't define this just buggy (or non-optimal)? How can I tell if I need
> to or not? A single example of that does cause a port to see it and a single
> example of why a port would not need to define it would be instructive.
>
> Along the lines of TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS, I'd be
> willing to explain the required abi of each function, and then I just want
> gcc to just work. All functions are normal, except for interrupt functions,
> all allocatable register need to be saved. I think this is a universally
> true thing, and it just happens to be true on my port as well.
>
> Anyway, I'd love to see gcc improved in this area.
>
> This one is a pet peeve of mine as I've seen what happens to software when a
> single register that should have been saved, wasn't. It was slightly
> annoying to track down and find. Fixing was trivial.
The cleanup as Richard suggested would probably be a good start. It's
interesting
that this kind of bug wasn't discovered earlier and likely existing for
several years.
The below is an updated version of the patch to include the other hook.
Regards,
Robert
gcc/
* config/mips/mips-protos.h (mips_hard_regno_rename_ok): New prototype.
* config/mips/mips.c (mips_hard_regno_rename_ok): New function.
(mips_hard_regno_scratch_ok): Likewise.
(TARGET_HARD_REGNO_SCRATCH_OK): Define macro.
* config/mips/mips.h (HARD_REGNO_RENAME_OK): New.
gcc/testsuite/
* gcc.target/mips/interrupt_handler-bug-1.c: New test.
---
gcc/config/mips/mips-protos.h | 1 +
gcc/config/mips/mips.c | 30 ++++++++++++++++++++++
gcc/config/mips/mips.h | 3 +++
.../gcc.target/mips/interrupt_handler-bug-1.c | 11 ++++++++
4 files changed, 45 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
diff --git a/gcc/config/mips/mips-protos.h b/gcc/config/mips/mips-protos.h
index 6ce3d70..e455553 100644
--- a/gcc/config/mips/mips-protos.h
+++ b/gcc/config/mips/mips-protos.h
@@ -317,6 +317,7 @@ extern unsigned int mips_sync_loop_insns (rtx, rtx *);
extern const char *mips_output_division (const char *, rtx *);
extern const char *mips_msa_output_division (const char *, rtx *);
extern const char *mips_output_probe_stack_range (rtx, rtx);
+extern bool mips_hard_regno_rename_ok (unsigned int, unsigned int);
extern unsigned int mips_hard_regno_nregs (int, machine_mode);
extern bool mips_linked_madd_p (rtx, rtx);
extern bool mips_store_data_bypass_p (rtx, rtx);
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index a9829bd..d0851e9 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -12959,6 +12959,33 @@ mips_hard_regno_mode_ok_p (unsigned int regno,
machine_mode mode)
return false;
}
+/* Return nonzero if register OLD_REG can be renamed to register NEW_REG. */
+
+bool
+mips_hard_regno_rename_ok (unsigned int old_reg ATTRIBUTE_UNUSED,
+ unsigned int new_reg)
+{
+ /* Interrupt functions can only use registers that have already been
+ saved by the prologue, even if they would normally be call-clobbered. */
+ if (cfun->machine->interrupt_handler_p && !df_regs_ever_live_p (new_reg))
+ return false;
+
+ return true;
+}
+
+/* Return nonzero if register REGNO can be used as a scratch register
+ in peephole2. */
+
+bool
+mips_hard_regno_scratch_ok (unsigned int regno)
+{
+ /* See mips_hard_regno_rename_ok. */
+ if (cfun->machine->interrupt_handler_p && !df_regs_ever_live_p (regno))
+ return false;
+
+ return true;
+}
+
/* Implement HARD_REGNO_NREGS. */
unsigned int
@@ -22428,6 +22455,9 @@ mips_lra_p (void)
#undef TARGET_SCHED_SET_SCHED_FLAGS
#define TARGET_SCHED_SET_SCHED_FLAGS mips_set_sched_flags
+#undef TARGET_HARD_REGNO_SCRATCH_OK
+#define TARGET_HARD_REGNO_SCRATCH_OK mips_hard_regno_scratch_ok
+
struct gcc_target targetm = TARGET_INITIALIZER;
#include "gt-mips.h"
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 6578ae5..3fe690a 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -1932,6 +1932,9 @@ struct mips_cpu_info {
#define HARD_REGNO_MODE_OK(REGNO, MODE)
\
mips_hard_regno_mode_ok[ (int)(MODE) ][ (REGNO) ]
+#define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \
+ mips_hard_regno_rename_ok (OLD_REG, NEW_REG)
+
/* Select a register mode required for caller save of hard regno REGNO. */
#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE) \
mips_hard_regno_caller_save_mode (REGNO, NREGS, MODE)
diff --git a/gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
b/gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
new file mode 100644
index 0000000..128ba90
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/interrupt_handler-bug-1.c
@@ -0,0 +1,11 @@
+/* { dg-options "-funroll-loops" } */
+int foo;
+int bar;
+
+void __attribute__ ((interrupt))
+isr (void)
+{
+ if (!foo)
+ while (bar & 0xFF30);
+}
+/* { dg-final { scan-assembler-not "\\\$8" } } */
--
2.4.5
