Hi Michael, > -----Original Message----- > From: Michael Eager [mailto:ea...@eagerm.com] > Sent: Wednesday, 27 February 2013 4:12 am > To: David Holsgrove > Cc: gcc-patches@gcc.gnu.org; Michael Eager (ea...@eagercon.com); John > Williams; Edgar E. Iglesias (edgar.igles...@gmail.com); Vinod Kathail; > Vidhumouli > Hunsigida; Nagaraju Mekala; Tom Shui > Subject: Re: [Patch, microblaze]: Added fast_interrupt controller > > On 02/10/2013 10:39 PM, David Holsgrove wrote: > > Added fast_interrupt controller > > > > Changelog > > > > 2013-02-11 Nagaraju Mekala <nmek...@xilinx.com> > > > > * config/microblaze/microblaze-protos.h: microblaze_is_fast_interrupt. > > * config/microblaze/microblaze.c (microblaze_attribute_table): Add > > microblaze_is_fast_interrupt. > > (microblaze_fast_interrupt_function_p): New function. > > (microblaze_is_fast_interrupt check): New function. > > (microblaze_must_save_register): Account for fast_interrupt. > > (save_restore_insns): Likewise. > > (compute_frame_size): Likewise. > > (microblaze_globalize_label): Add FAST_INTERRUPT_NAME. > > * config/microblaze/microblaze.h: Define FAST_INTERRUPT_NAME as > > fast_interrupt. > > * config/microblaze/microblaze.md (movsi_status): Can be > > fast_interrupt > > (return): Add microblaze_is_fast_interrupt. > > (return_internal): Likewise. > > +int > +microblaze_is_fast_interrupt (void) > +{ > + return fast_interrupt; > +} > > + if (fast_interrupt) > + { > > Use wrapper functions consistently. Either reference the flag everywhere > or use the wrapper everywhere.
I've repurposed the existing 'microblaze_is_interrupt_handler' wrapper, (which
was
only used in the machine description), to be 'microblaze_is_interrupt_variant'
- true
if the function's attribute is either interrupt_handler or fast_interrupt.
>
> + if (interrupt_handler || fast_interrupt)
>
> + if (microblaze_is_interrupt_handler () || microblaze_is_fast_interrupt())
>
> There are many places in the patch where both interrupt_handler and
> fast_interrupt
> are tested. These can be eliminated by setting the interrupt_handler flag
> when
> you see fast_interrupt and checking for the correct registers to be saved in
> microblaze_must_save_register().
I've used this microblaze_is_interrupt_variant wrapper throughout, checking
specifically for the interrupt_handler or fast_interrupt flag only where it was
necessary to handle them differently.
Please let me know if the patch attached is acceptable, or if you would prefer
I refactor all the existing interrupt_handler functionality to accommodate the
fast_interrupt.
Updated Changelog;
2013-03-05 David Holsgrove <david.holsgr...@xilinx.com>
* gcc/config/microblaze/microblaze-protos.h: Rename
microblaze_is_interrupt_handler to microblaze_is_interrupt_variant.
* gcc/config/microblaze/microblaze.c (microblaze_attribute_table): Add
fast_interrupt.
(microblaze_fast_interrupt_function_p): New function.
(microblaze_is_interrupt_handler): Rename to
microblaze_is_interrupt_variant and add fast_interrupt check.
(microblaze_must_save_register): Use microblaze_is_interrupt_variant.
(save_restore_insns): Likewise.
(compute_frame_size): Likewise.
(microblaze_function_prologue): Add FAST_INTERRUPT_NAME.
(microblaze_globalize_label): Likewise.
* gcc/config/microblaze/microblaze.h: Define FAST_INTERRUPT_NAME.
* gcc/config/microblaze/microblaze.md: Use wrapper
microblaze_is_interrupt_variant.
thanks again for the reviews,
David
>
> + if ((interrupt_handler && !prologue) ||( fast_interrupt && !prologue) )
>
> + if ((interrupt_handler && prologue) || (fast_interrupt && prologue))
>
> Refactor. Fix spacing around parens.
>
> --
> Michael Eager ea...@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
0002-Gcc-Added-fast_interrupt-controller.patch
Description: 0002-Gcc-Added-fast_interrupt-controller.patch
