From 310b5fb79316511295c04f956f4c6630437ba204 Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz Date: Mon, 22 Jul 2019 16:02:29 +0000 Subject: [PATCH] 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 * 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 * 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 --- gcc/ChangeLog | 6 + gcc/config/msp430/msp430.c | 18 ++- gcc/testsuite/ChangeLog | 8 ++ .../gcc.target/msp430/isr-push-pop-isr-430.c | 13 ++ .../gcc.target/msp430/isr-push-pop-isr-430x.c | 12 ++ .../msp430/isr-push-pop-leaf-isr-430.c | 27 ++++ .../msp430/isr-push-pop-leaf-isr-430x.c | 24 ++++ .../gcc.target/msp430/isr-push-pop-main.c | 120 ++++++++++++++++++ 8 files changed, 223 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430.c create mode 100644 gcc/testsuite/gcc.target/msp430/isr-push-pop-isr-430x.c create mode 100644 gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430.c create mode 100644 gcc/testsuite/gcc.target/msp430/isr-push-pop-leaf-isr-430x.c create mode 100644 gcc/testsuite/gcc.target/msp430/isr-push-pop-main.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index eb786a072ec..687ae89cdc8 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2019-07-22 Jozef Lawrynowicz + + * 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 * config/rs6000/smmintrin.h (_mm_blend_epi16): New. diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c index 365e9eba747..265c2f642d8 100644 --- 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 d45620e4817..ecd2a9e5554 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2019-07-22 Jozef Lawrynowicz + + * 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 * 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 index 00000000000..a2bf8433ebd --- /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 index 00000000000..2d65186bdf9 --- /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 index 00000000000..cbb45974c4a --- /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 index 00000000000..872a40ef755 --- /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 index 00000000000..5c7b594b50b --- /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; +} + -- 2.30.2