MSP430: Don't save all callee-saved regs in non-leaf interrupt functions

Previously, all callee-saved regs would unconditionally be saved
in interrupt functions that call another function.

2019-07-22  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.c (msp430_preserve_reg_p): Don't save
	callee-saved regs R4->R10 in an interrupt function that calls another
	function.

2019-07-22  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* gcc.target/msp430/isr-push-pop-main.c: New test.
	* gcc.target/msp430/isr-push-pop-isr-430.c: Likewise.
	* gcc.target/msp430/isr-push-pop-isr-430x.c: Likewise.
	* gcc.target/msp430/isr-push-pop-leaf-isr-430.c: Likewise.
	* gcc.target/msp430/isr-push-pop-leaf-isr-430x.c: Likewise.

From-SVN: r273702

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index eb786a072ecfdc6ac3a298ba1b01a5ee81e312b6..687ae89cdc815a1dc234d64e7357527bde227ed0 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2019-07-22  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
+
+       * config/msp430/msp430.c (msp430_preserve_reg_p): Don't save
+       callee-saved regs R4->R10 in an interrupt function that calls another
+       function.
+
 2019-07-22  Paul A. Clarke  <pc@us.ibm.com>
 
        * config/rs6000/smmintrin.h (_mm_blend_epi16): New.

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 365e9eba747f94d2af35da30b6bd8d229c0fbac3..265c2f642d82c62dcab3217284ebf6dfe0193522 100644 (file)
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1755,11 +1755,19 @@ msp430_preserve_reg_p (int regno)
   if (fixed_regs [regno])
     return false;
 
-  /* Interrupt handlers save all registers they use, even
-     ones which are call saved.  If they call other functions
-     then *every* register is saved.  */
-  if (msp430_is_interrupt_func ())
-    return ! crtl->is_leaf || df_regs_ever_live_p (regno);
+  /* For interrupt functions we must save and restore the used regs that
+     would normally be caller-saved (R11->R15).  */
+  if (msp430_is_interrupt_func () && regno >= 11 && regno <= 15)
+    {
+      if (crtl->is_leaf && df_regs_ever_live_p (regno))
+       /* If the interrupt func is a leaf then we only need to restore the
+          caller-saved regs that are used.  */
+       return true;
+      else if (!crtl->is_leaf)
+       /* If the interrupt function is not a leaf we must save all
+          caller-saved regs in case the callee modifies them.  */
+       return true;
+    }
 
   if (!call_used_regs [regno]
       && df_regs_ever_live_p (regno))

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d45620e4817f45b2cdffe06b9cfee95a445d3ad1..ecd2a9e5554c94ccc6b7cc7b225f4c61c0860115 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2019-07-22  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
+
+       * gcc.target/msp430/isr-push-pop-main.c: New test.
+       * gcc.target/msp430/isr-push-pop-isr-430.c: Likewise.
+       * gcc.target/msp430/isr-push-pop-isr-430x.c: Likewise.
+       * gcc.target/msp430/isr-push-pop-leaf-isr-430.c: Likewise.
+       * gcc.target/msp430/isr-push-pop-leaf-isr-430x.c: Likewise.
+
 2019-07-22  Andrea Corallo <andrea.corallo@arm.com>
 
        * jit.dg/test-error-gcc_jit_context_new_unary_op-bad-res-type.c:

diff --git a/gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430.c b/gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430.c
new file mode 100644 (file)
index 0000000..a2bf843
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430x*" "-mlarge" } { "" } } */
+/* { dg-options "-mcpu=msp430" } */
+/* { dg-final { scan-assembler "PUSH\tR11" } } */
+/* { dg-final { scan-assembler-not "PUSH\tR10" } } */
+
+void __attribute__((noinline)) callee (void);
+
+void __attribute__((interrupt))
+isr (void)
+{
+  callee();
+}

diff --git a/gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430x.c b/gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430x.c
new file mode 100644 (file)
index 0000000..2d65186
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430x.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
+/* { dg-final { scan-assembler "PUSHM.*#5" } } */
+/* { dg-final { scan-assembler-not "PUSHM.*#12" } } */
+
+void __attribute__((noinline)) callee (void);
+
+void __attribute__((interrupt))
+isr (void)
+{
+  callee();
+}

diff --git a/gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430.c b/gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430.c
new file mode 100644 (file)
index 0000000..cbb4597
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430x*" "-mlarge" } { "" } } */
+/* { dg-options "-mcpu=msp430" } */
+/* { dg-final { scan-assembler "PUSH\tR5" } } */
+/* { dg-final { scan-assembler "PUSH\tR12" } } */
+/* { dg-final { scan-assembler-not "PUSH\tR4" } } */
+/* { dg-final { scan-assembler-not "PUSH\tR11" } } */
+
+/* To check that the compiler doesn't blindly save all regs, we omit R4 and R11
+   from the trashing.  */
+#define TRASH_REGS_LITE                                \
+  __asm__ ("mov #0xFFFF, r5" : : : "R5");      \
+  __asm__ ("mov #0xFFFF, r6" : : : "R6");      \
+  __asm__ ("mov #0xFFFF, r7" : : : "R7");      \
+  __asm__ ("mov #0xFFFF, r8" : : : "R8");      \
+  __asm__ ("mov #0xFFFF, r9" : : : "R9");      \
+  __asm__ ("mov #0xFFFF, r10" : : : "R10");    \
+  __asm__ ("mov #0xFFFF, r12" : : : "R12");    \
+  __asm__ ("mov #0xFFFF, r13" : : : "R13");    \
+  __asm__ ("mov #0xFFFF, r14" : : : "R14");    \
+  __asm__ ("mov #0xFFFF, r15" : : : "R15");
+
+void __attribute__((interrupt))
+isr_leaf (void)
+{
+  TRASH_REGS_LITE
+}

diff --git a/gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430x.c b/gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430x.c
new file mode 100644 (file)
index 0000000..872a40e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430x.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
+/* { dg-final { scan-assembler "PUSHM.*#4.*R15" } } */
+/* { dg-final { scan-assembler "PUSHM.*#6.*R10" } } */
+
+/* To check that the compiler doesn't blindly save all regs, we omit R4 and R11
+   from the trashing.  */
+#define TRASH_REGS_LITE                                \
+  __asm__ ("mov #0xFFFF, r5" : : : "R5");      \
+  __asm__ ("mov #0xFFFF, r6" : : : "R6");      \
+  __asm__ ("mov #0xFFFF, r7" : : : "R7");      \
+  __asm__ ("mov #0xFFFF, r8" : : : "R8");      \
+  __asm__ ("mov #0xFFFF, r9" : : : "R9");      \
+  __asm__ ("mov #0xFFFF, r10" : : : "R10");    \
+  __asm__ ("mov #0xFFFF, r12" : : : "R12");    \
+  __asm__ ("mov #0xFFFF, r13" : : : "R13");    \
+  __asm__ ("mov #0xFFFF, r14" : : : "R14");    \
+  __asm__ ("mov #0xFFFF, r15" : : : "R15");
+
+void __attribute__((interrupt))
+isr_leaf (void)
+{
+  TRASH_REGS_LITE
+}

diff --git a/gcc/testsuite/gcc.target/msp430/isr-push-pop-main.c b/gcc/testsuite/gcc.target/msp430/isr-push-pop-main.c
new file mode 100644 (file)
index 0000000..5c7b594
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/isr-push-pop-main.c
@@ -0,0 +1,120 @@
+/* { dg-do run } */
+
+#ifdef __MSP430X__
+#include "isr-push-pop-isr-430x.c"
+#include "isr-push-pop-leaf-isr-430x.c"
+#else
+#include "isr-push-pop-isr-430.c"
+#include "isr-push-pop-leaf-isr-430.c"
+#endif
+
+/* Test that ISRs which call other functions do not save extraneous registers.
+   They only need to save the caller-saved regs R11->R15.
+   We use a lot of asm statements to hide what is going on from the compiler to
+   more accurately simulate an interrupt.  */
+
+/* Store the register number in each general register R4->R15, so they can be
+   later checked their value has been kept.  */
+#define SETUP_REGS             \
+  __asm__ ("mov #4, r4");      \
+  __asm__ ("mov #5, r5");      \
+  __asm__ ("mov #6, r6");      \
+  __asm__ ("mov #7, r7");      \
+  __asm__ ("mov #8, r8");      \
+  __asm__ ("mov #9, r9");      \
+  __asm__ ("mov #10, r10");    \
+  __asm__ ("mov #11, r11");    \
+  __asm__ ("mov #12, r12");    \
+  __asm__ ("mov #13, r13");    \
+  __asm__ ("mov #14, r14");    \
+  __asm__ ("mov #15, r15");
+
+/* Write an arbitrary value to all general regs.  */
+#define TRASH_REGS                             \
+  __asm__ ("mov #0xFFFF, r4" : : : "R4");      \
+  __asm__ ("mov #0xFFFF, r5" : : : "R5");      \
+  __asm__ ("mov #0xFFFF, r6" : : : "R6");      \
+  __asm__ ("mov #0xFFFF, r7" : : : "R7");      \
+  __asm__ ("mov #0xFFFF, r8" : : : "R8");      \
+  __asm__ ("mov #0xFFFF, r9" : : : "R9");      \
+  __asm__ ("mov #0xFFFF, r10" : : : "R10");    \
+  __asm__ ("mov #0xFFFF, r11" : : : "R11");    \
+  __asm__ ("mov #0xFFFF, r12" : : : "R12");    \
+  __asm__ ("mov #0xFFFF, r13" : : : "R13");    \
+  __asm__ ("mov #0xFFFF, r14" : : : "R14");    \
+  __asm__ ("mov #0xFFFF, r15" : : : "R15");
+
+/* Check the value in all general registers is the same as that set in
+   SETUP_REGS.  */
+#define CHECK_REGS                     \
+  __asm__ ("cmp #4,  r4 { jne ABORT"); \
+  __asm__ ("cmp #5,  r5 { jne ABORT"); \
+  __asm__ ("cmp #6,  r6 { jne ABORT"); \
+  __asm__ ("cmp #7,  r7 { jne ABORT"); \
+  __asm__ ("cmp #8,  r8 { jne ABORT"); \
+  __asm__ ("cmp #9,  r9 { jne ABORT"); \
+  __asm__ ("cmp #10, r10 { jne ABORT");        \
+  __asm__ ("cmp #11, r11 { jne ABORT");        \
+  __asm__ ("cmp #12, r12 { jne ABORT");        \
+  __asm__ ("cmp #13, r13 { jne ABORT");        \
+  __asm__ ("cmp #14, r14 { jne ABORT");        \
+  __asm__ ("cmp #15, r15 { jne ABORT");
+
+void __attribute__((noinline))
+callee (void)
+{
+  /* Here were modify all the regs, but tell the compiler that we are since
+     this is just a way to simulate a function that happens to modify all the
+     registers.  */
+  TRASH_REGS
+}
+int 
+#ifdef __MSP430X_LARGE__ 
+__attribute__((lower))
+#endif
+main (void)
+{
+  SETUP_REGS
+
+  /* A surprise branch to the ISR that the compiler cannot prepare for.
+     We must first simulate the interrupt acceptance procedure that the
+     hardware would normally take care of.
+     So push the desired PC return address, and then the SR (R2).
+     MSP430X expects the high bits 19:16 of the PC return address to be stored
+     in bits 12:15 of the SR stack slot.  This is hard to handle in hand-rolled
+     assembly code, so we always place main() in lower memory so the return
+     address is 16-bits.  */
+  __asm__ ("push #CHECK1");
+  __asm__ ("push r2");
+  __asm__ ("br #isr");
+
+  __asm__ ("CHECK1:");
+  /* If any of the regs R4->R15 don't match their original value, this will
+     jump to ABORT.  */
+  CHECK_REGS
+
+  /* Now test that an interrupt function that is a leaf also works
+     correctly.  */
+  __asm__ ("push #CHECK2");
+  __asm__ ("push r2");
+  __asm__ ("br #isr_leaf");
+
+  __asm__ ("CHECK2:");
+  CHECK_REGS
+
+  /* The values in R4->R15 were successfully checked, now jump to FINISH to run
+     the prologue generated by the compiler.  */
+  __asm__ ("jmp FINISH");
+
+  /* CHECK_REGS will branch here if a register holds the wrong value.  */
+  __asm__ ("ABORT:");
+#ifdef __MSP430X_LARGE__ 
+  __asm__ ("calla #abort");
+#else
+  __asm__ ("call #abort");
+#endif
+
+  __asm__ ("FINISH:");
+  return 0;
+}
+