RE: [PATCH, MIPS] Support new interrupt handler options
Hi Catherine, This is now OK to commit. Catherine Committed as r225819. Robert
RE: [PATCH, MIPS] Support new interrupt handler options
Hi Catherine, I'm getting build errors with the current TOT and your patch. The first errors that I encounter are: gcc/config/mips/mips.c:1355:1: warning: 'mips_int_mask mips_interrupt_mask(tree)' defined but not used [-Wunused-function] gcc/config/mips/mips.c:1392:1: warning: 'mips_shadow_set mips_use_shadow_register_set(tree)' defined but not used [-Wunused-function] Removing these two functions results in further errors that I have not investigated. Will you try applying and building your patch again? I have no explanation why this could happen. My guess is that a part of the patch did not apply correctly. Those functions are used in mips_compute_frame_info(). I did notice and fixed warnings about unused variables/arguments. I have a couple of further comments on the existing patch, see below. Comments added. Please have a look at the attached revised patch. Tested against r225768. Regards, Robert gcc/ * config/mips/mips.c (mips_int_mask): New enum. (mips_shadow_set): Likewise. (int_mask): New variable. (use_shadow_register_set_p): Change type to enum mips_shadow_set. (machine_function): Add int_mask and use_shadow_register_set. (mips_attribute_table): Add attribute handlers for interrupt and use_shadow_register_set. (mips_interrupt_mask): New static function. (mips_handle_interrupt_attr): Likewise. (mips_handle_use_shadow_register_set_attr): Likewise. (mips_use_shadow_register_set): Change return type to enum mips_shadow_set. Add argument handling for use_shadow_register_set attribute. (mips_interrupt_extra_called_saved_reg_p): Update the conditional to compare with mips_shadow_set enum. (mips_compute_frame_info): Add interrupt mask and use_shadow_register_set to per-function information structure. Add a stack slot for EPC unconditionally. (mips_expand_prologue): Compare use_shadow_register_set value with mips_shadow_set enum. Save EPC always in K1, clobber only K1 for masked interrupt register but in EIC mode use K0 and save Cause in K0. EPC saved and restored unconditionally. Use PMODE_INSN macro when copying the stack pointer from the shadow register set. * config/mips/mips.h (SR_IM0): New define. * config/mips/mips.md (mips_rdpgpr): Rename to... (mips_rdpgpr_mode): ...this. Use the Pmode iterator. * doc/extend.texi (Declaring Attributes of Functions): Document optional arguments for interrupt and use_shadow_register_set attributes. gcc/testsuite/ * gcc.target/mips/interrupt_handler-4.c: New test. --- gcc/config/mips/mips.c | 286 + gcc/config/mips/mips.h | 3 + gcc/config/mips/mips.md| 10 +- gcc/doc/extend.texi| 22 +- .../gcc.target/mips/interrupt_handler-4.c | 31 +++ 5 files changed, 290 insertions(+), 62 deletions(-) create mode 100644 gcc/testsuite/gcc.target/mips/interrupt_handler-4.c diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 671fed8..70240f7 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -382,6 +382,30 @@ struct GTY(()) mips_frame_info { HOST_WIDE_INT hard_frame_pointer_offset; }; +/* Enumeration for masked vectored (VI) and non-masked (EIC) interrupts. */ +enum mips_int_mask +{ + INT_MASK_EIC = -1, + INT_MASK_SW0 = 0, + INT_MASK_SW1 = 1, + INT_MASK_HW0 = 2, + INT_MASK_HW1 = 3, + INT_MASK_HW2 = 4, + INT_MASK_HW3 = 5, + INT_MASK_HW4 = 6, + INT_MASK_HW5 = 7 +}; + +/* Enumeration to mark the existence of the shadow register set. + SHADOW_SET_INTSTACK indicates a shadow register set with a valid stack + pointer. */ +enum mips_shadow_set +{ + SHADOW_SET_NO, + SHADOW_SET_YES, + SHADOW_SET_INTSTACK +}; + struct GTY(()) machine_function { /* The next floating-point condition-code register to allocate for ISA_HAS_8CC targets, relative to ST_REG_FIRST. */ @@ -434,8 +458,12 @@ struct GTY(()) machine_function { /* True if this is an interrupt handler. */ bool interrupt_handler_p; - /* True if this is an interrupt handler that uses shadow registers. */ - bool use_shadow_register_set_p; + /* Records the way in which interrupts should be masked. Only used if + interrupts are not kept masked. */ + enum mips_int_mask int_mask; + + /* Records if this is an interrupt handler that uses shadow registers. */ + enum mips_shadow_set use_shadow_register_set; /* True if this is an interrupt handler that should keep interrupts masked. */ @@ -717,6 +745,10 @@ const enum reg_class mips_regno_to_class[FIRST_PSEUDO_REGISTER] = { ALL_REGS,ALL_REGS, ALL_REGS, ALL_REGS }; +static tree mips_handle_interrupt_attr (tree *, tree, tree, int, bool *); +static tree
RE: [PATCH, MIPS] Support new interrupt handler options
-Original Message- From: Robert Suchanek [mailto:robert.sucha...@imgtec.com] Sent: Tuesday, July 14, 2015 11:14 AM To: Moore, Catherine; Matthew Fortune; gcc-patches@gcc.gnu.org Subject: RE: [PATCH, MIPS] Support new interrupt handler options Hi Catherine, I'm getting build errors with the current TOT and your patch. The first errors that I encounter are: gcc/config/mips/mips.c:1355:1: warning: 'mips_int_mask mips_interrupt_mask(tree)' defined but not used [-Wunused-function] gcc/config/mips/mips.c:1392:1: warning: 'mips_shadow_set mips_use_shadow_register_set(tree)' defined but not used [-Wunused-function] Removing these two functions results in further errors that I have not investigated. Will you try applying and building your patch again? I have no explanation why this could happen. My guess is that a part of the patch did not apply correctly. Those functions are used in mips_compute_frame_info(). I did notice and fixed warnings about unused variables/arguments. I re-ran patch with the verbose option and found that it was silently discarding hunks (starting with #7) because it thought it was garbage. I have a couple of further comments on the existing patch, see below. Comments added. Please have a look at the attached revised patch. Tested against r225768. Regards, Robert gcc/ * config/mips/mips.c (mips_int_mask): New enum. (mips_shadow_set): Likewise. (int_mask): New variable. (use_shadow_register_set_p): Change type to enum mips_shadow_set. (machine_function): Add int_mask and use_shadow_register_set. (mips_attribute_table): Add attribute handlers for interrupt and use_shadow_register_set. (mips_interrupt_mask): New static function. (mips_handle_interrupt_attr): Likewise. (mips_handle_use_shadow_register_set_attr): Likewise. (mips_use_shadow_register_set): Change return type to enum mips_shadow_set. Add argument handling for use_shadow_register_set attribute. (mips_interrupt_extra_called_saved_reg_p): Update the conditional to compare with mips_shadow_set enum. (mips_compute_frame_info): Add interrupt mask and use_shadow_register_set to per-function information structure. Add a stack slot for EPC unconditionally. (mips_expand_prologue): Compare use_shadow_register_set value with mips_shadow_set enum. Save EPC always in K1, clobber only K1 for masked interrupt register but in EIC mode use K0 and save Cause in K0. EPC saved and restored unconditionally. Use PMODE_INSN macro when copying the stack pointer from the shadow register set. * config/mips/mips.h (SR_IM0): New define. * config/mips/mips.md (mips_rdpgpr): Rename to... (mips_rdpgpr_mode): ...this. Use the Pmode iterator. * doc/extend.texi (Declaring Attributes of Functions): Document optional arguments for interrupt and use_shadow_register_set attributes. gcc/testsuite/ * gcc.target/mips/interrupt_handler-4.c: New test. This is now OK to commit. Catherine
RE: [PATCH, MIPS] Support new interrupt handler options
-Original Message- From: Robert Suchanek [mailto:robert.sucha...@imgtec.com] Sent: Wednesday, July 08, 2015 6:43 AM To: Matthew Fortune; Moore, Catherine; gcc-patches@gcc.gnu.org Subject: [PATCH, MIPS] Support new interrupt handler options Hi, This patch adds support for optional arguments for interrupt and use_shadow_register_set attributes. The patch also fixes an ICE if both interrupt and use_shadow_register_set are enabled and compiled with - mips64r2 -mabi=64 discovered during testing of the attached test. The interrupt attribute accepts new arguments: eic and vector=[sw0|sw1|hw0|hw1|hw2|hw3|hw4|hw5]. The former is the default if no argument is given and the latter changes the behaviour of GCC and masks interrupts from sw0 up to and including the specified vector. As part of this change, the EPC is now saved and restored unconditionally to recover the state in nested interrupts. Only K1 register is clobbered for masked interrupts but for non-masked interrupts K0 is still used. The use_shadow_register_set attribute has a new option, intstack, to indicate that the shadow register set has a valid stack pointer. With this option rdpgpr $sp, $sp will not be generated for an ISR. Tested with mips-img-elf, mips-img-linux-gnu and mips64el-linux-gnu cross compilers. Ok to apply? Regards, Robert 2015-07-07 Matthew Fortune matthew.fort...@imgtec.com Robert Suchanek robert.sucha...@imgtec.com gcc/ * config/mips/mips.c (mips_int_mask): New enum. (mips_shadow_set): Likewise. (int_mask): New variable. (use_shadow_register_set_p): Change type to enum mips_shadow_set. (machine_function): Add int_mask and use_shadow_register_set. (mips_attribute_table): Add attribute handlers for interrupt and use_shadow_register_set. (mips_interrupt_mask): New static function. (mips_handle_interrupt_attr): Likewise. (mips_handle_use_shadow_register_set_attr): Likewise. (mips_use_shadow_register_set): Change return type to enum mips_shadow_set. Add argument handling for use_shadow_register_set attribute. (mips_interrupt_extra_called_saved_reg_p): Update the conditional to compare with mips_shadow_set enum. (mips_compute_frame_info): Add interrupt mask and use_shadow_register_set to per-function information structure. Add a stack slot for EPC unconditionally. (mips_expand_prologue): Compare use_shadow_register_set value with mips_shadow_set enum. Save EPC always in K1, clobber only K1 for masked interrupt register but in EIC mode use K0 and save Cause in K0. EPC saved and restored unconditionally. Use PMODE_INSN macro when copying the stack pointer from the shadow register set. * config/mips/mips.h (SR_IM0): New define. * config/mips/mips.md (mips_rdpgpr): Rename to... (mips_rdpgpr_mode): ...this. Use the Pmode iterator. * doc/extend.texi (Declaring Attributes of Functions): Document optional arguments for interrupt and use_shadow_register_set attributes. gcc/testsuite/ * gcc.target/mips/interrupt_handler-4.c: New test. Hi Robert, I'm getting build errors with the current TOT and your patch. The first errors that I encounter are: gcc/config/mips/mips.c:1355:1: warning: 'mips_int_mask mips_interrupt_mask(tree)' defined but not used [-Wunused-function] gcc/config/mips/mips.c:1392:1: warning: 'mips_shadow_set mips_use_shadow_register_set(tree)' defined but not used [-Wunused-function] Removing these two functions results in further errors that I have not investigated. Will you try applying and building your patch again? I have a couple of further comments on the existing patch, see below. Thanks, Catherine diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index ce21a0f..b6ad7db 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -1325,13 +1359,62 @@ mips_interrupt_type_p (tree type) return lookup_attribute (interrupt, TYPE_ATTRIBUTES (type)) != NULL; } +static enum mips_int_mask +mips_interrupt_mask (tree type) This function requires a comment. +static enum mips_shadow_set +mips_use_shadow_register_set (tree type) Likewise. { @@ -1537,6 +1620,87 @@ mips_can_inline_p (tree caller, tree callee) return false; return default_target_can_inline_p (caller, callee); } + +static tree +mips_handle_interrupt_attr (tree *node, tree name, tree args, + int flags ATTRIBUTE_UNUSED, bool *no_add_attrs) { Likewise. + +static tree +mips_handle_use_shadow_register_set_attr (tree *node, tree name, tree args, And here as well.