On 03/05/2013 07:09 AM, David Holsgrove wrote:
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.


Committed revision 196474.



--
Michael Eager    ea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Reply via email to