[patch] m68k.h: Add "interrupt_thread" attribute.

[patch] m68k.h: Add "interrupt_thread" attribute.

[Kazu Hirata]

Hi,

Attached is a patch to add "interrupt_thread" attribute.

A function with the interrupt_thread attribute is compiled just like
an ordinary function except that

- no register is saved or restored in prologue/epilogue.

- the 'rts' instruction is replaced with the 'sleep' instruction.

This attribute is intended to take advantage of fido's hardware
contexts, implemented as five sets of registers.  With this feature,
we can dedicate one context to handle a certain interrupt, presumably
time critical one where saving/restoring registers makes a big
difference.

As far as the implementation goes, we teach m68k_compute_frame_layout
to think that no register need saving/restoring when interrupt_thread
is specified.

Now that two different kinds of interrupt functions,
m68k_interrupt_function_p is renamed to m68k_get_function_kind,
categorizing functions into normal, interrupt_handler, and
interrupt_thread functions.

The output of the "*return" pattern and the attribute processing
should be self-explanatory.

Tested on fido-none-elf.  OK to apply?

Kazu Hirata

gcc/
2007-04-27  Kazu Hirata  <kazu@codesourcery.com>

	* config/m68k/m68k-protos.h: Rename m68k_interrupt_function_p
	to m68k_get_function_kind.  Update its prototype.
	* config/m68k/m68k.c (m68k_attribute_table): Add an entry for
	interrupt_thread.
	(m68k_interrupt_function_p): Return enum m68k_function_type
	instead of bool.  Rename to m68k_get_function_kind.
	(m68k_handle_fndecl_attribute): Reject interrupt_thread if the
	target is not fido.
	(m68k_compute_frame_layout): Don't mark any register for save
	if an interrupt_thread attribute is specified.
	(m68k_hard_regno_rename_ok): Update a use of
	m68k_interrupt_function_p.
	* config/m68k/m68k.h (EPILOGUE_USES): Update a use of
	m68k_interrupt_function_p.
	(m68k_function_type): New.
	* config/m68k/m68k.md (*return): Output a 'sleep' instruction
	for a function with an interrupt_thread attribute.
	* doc/extend.texi: Document the interrupt_thread attribute.

gcc/testsuite/
2007-04-27  Kazu Hirata  <kazu@codesourcery.com>

	* gcc.target/m68k/interrupt_thread-1.c,
	gcc.target/m68k/interrupt_thread-2.c,
	gcc.target/m68k/interrupt_thread-3.c: New.
	* gcc.target/m68k/m68k.exp: Accept fido.

Index: gcc/config/m68k/m68k-protos.h
===================================================================
--- gcc/config/m68k/m68k-protos.h	(revision 124112)
+++ gcc/config/m68k/m68k-protos.h	(working copy)
@@ -21,7 +21,7 @@ Boston, MA 02110-1301, USA.  */
 /* Define functions defined in aux-output.c and used in templates.  */
 
 #ifdef RTX_CODE
-extern bool m68k_interrupt_function_p (tree);
+extern enum m68k_function_kind m68k_get_function_kind (tree);
 extern HOST_WIDE_INT m68k_initial_elimination_offset (int from, int to);
 
 extern void split_di (rtx[], int, rtx[], rtx[]);
Index: gcc/config/m68k/m68k.c
===================================================================
--- gcc/config/m68k/m68k.c	(revision 124112)
+++ gcc/config/m68k/m68k.c	(working copy)
@@ -223,6 +223,7 @@ static const struct attribute_spec m68k_
 {
   /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler } */
   { "interrupt_handler", 0, 0, true,  false, false, m68k_handle_fndecl_attribute },
+  { "interrupt_thread", 0, 0, true,  false, false, m68k_handle_fndecl_attribute },
   { NULL,                0, 0, false, false, false, NULL }
 };
 
@@ -633,10 +634,12 @@ m68k_cpp_cpu_family (const char *prefix)
   return concat ("__m", prefix, "_family_", m68k_cpu_entry->family, NULL);
 }
 \f
-/* Return nonzero if FUNC is an interrupt function as specified by the
-   "interrupt_handler" attribute.  */
-bool
-m68k_interrupt_function_p (tree func)
+/* Return m68k_fk_interrupt_handler if FUNC has an "interrupt_handler"
+   attribute and interrupt_thread if FUNC has an "interrupt_thread"
+   attribute.  Otherwise, return m68k_fk_normal_function.  */
+
+enum m68k_function_kind
+m68k_get_function_kind (tree func)
 {
   tree a;
 
@@ -644,7 +647,14 @@ m68k_interrupt_function_p (tree func)
     return false;
 
   a = lookup_attribute ("interrupt_handler", DECL_ATTRIBUTES (func));
-  return (a != NULL_TREE);
+  if (a != NULL_TREE)
+    return m68k_fk_interrupt_handler;
+
+  a = lookup_attribute ("interrupt_thread", DECL_ATTRIBUTES (func));
+  if (a != NULL_TREE)
+    return m68k_fk_interrupt_thread;
+
+  return m68k_fk_normal_function;
 }
 
 /* Handle an attribute requiring a FUNCTION_DECL; arguments as in
@@ -662,6 +672,19 @@ m68k_handle_fndecl_attribute (tree *node
       *no_add_attrs = true;
     }
 
+  if (m68k_get_function_kind (*node) != m68k_fk_normal_function)
+    {
+      error ("multiple interrupt attributes not allowed");
+      *no_add_attrs = true;
+    }
+
+  if (!TARGET_FIDOA
+      && !strcmp (IDENTIFIER_POINTER (name), "interrupt_thread"))
+    {
+      error ("interrupt_thread is available only on fido");
+      *no_add_attrs = true;
+    }
+
   return NULL_TREE;
 }
 
@@ -670,7 +693,10 @@ m68k_compute_frame_layout (void)
 {
   int regno, saved;
   unsigned int mask;
-  bool interrupt_handler = m68k_interrupt_function_p (current_function_decl);
+  enum m68k_function_kind func_kind =
+    m68k_get_function_kind (current_function_decl);
+  bool interrupt_handler = func_kind == m68k_fk_interrupt_handler;
+  bool interrupt_thread = func_kind == m68k_fk_interrupt_thread;
 
   /* Only compute the frame once per function.
      Don't cache information until reload has been completed.  */
@@ -681,12 +707,15 @@ m68k_compute_frame_layout (void)
   current_frame.size = (get_frame_size () + 3) & -4;
 
   mask = saved = 0;
-  for (regno = 0; regno < 16; regno++)
-    if (m68k_save_reg (regno, interrupt_handler))
-      {
-	mask |= 1 << (regno - D0_REG);
-	saved++;
-      }
+
+  /* Interrupt thread does not need to save any register.  */
+  if (!interrupt_thread)
+    for (regno = 0; regno < 16; regno++)
+      if (m68k_save_reg (regno, interrupt_handler))
+	{
+	  mask |= 1 << (regno - D0_REG);
+	  saved++;
+	}
   current_frame.offset = saved * 4;
   current_frame.reg_no = saved;
   current_frame.reg_mask = mask;
@@ -695,12 +724,14 @@ m68k_compute_frame_layout (void)
   mask = saved = 0;
   if (TARGET_HARD_FLOAT)
     {
-      for (regno = 16; regno < 24; regno++)
-	if (m68k_save_reg (regno, interrupt_handler))
-	  {
-	    mask |= 1 << (regno - FP0_REG);
-	    saved++;
-	  }
+      /* Interrupt thread does not need to save any register.  */
+      if (!interrupt_thread)
+	for (regno = 16; regno < 24; regno++)
+	  if (m68k_save_reg (regno, interrupt_handler))
+	    {
+	      mask |= 1 << (regno - FP0_REG);
+	      saved++;
+	    }
       current_frame.foffset = saved * TARGET_FP_REG_SIZE;
       current_frame.offset += current_frame.foffset;
     }
@@ -4212,7 +4243,8 @@ m68k_hard_regno_rename_ok (unsigned int 
      saved by the prologue, even if they would normally be
      call-clobbered.  */
 
-  if (m68k_interrupt_function_p (current_function_decl)
+  if ((m68k_get_function_kind (current_function_decl)
+       == m68k_fk_interrupt_handler)
       && !regs_ever_live[new_reg])
     return 0;
 
Index: gcc/config/m68k/m68k.h
===================================================================
--- gcc/config/m68k/m68k.h	(revision 124112)
+++ gcc/config/m68k/m68k.h	(working copy)
@@ -971,8 +971,10 @@ do { if (cc_prev_status.flags & CC_IN_68
 #define INCOMING_FRAME_SP_OFFSET 4
 
 /* All registers are live on exit from an interrupt routine.  */
-#define EPILOGUE_USES(REGNO) \
-  (reload_completed && m68k_interrupt_function_p (current_function_decl))
+#define EPILOGUE_USES(REGNO)					\
+  (reload_completed						\
+   && (m68k_get_function_kind (current_function_decl)	\
+       == m68k_fk_interrupt_handler))
 
 /* Describe how we implement __builtin_eh_return.  */
 #define EH_RETURN_DATA_REGNO(N) \
@@ -1166,6 +1168,13 @@ enum fpu_type
   FPUTYPE_COLDFIRE
 };
 
+enum m68k_function_kind
+{
+  m68k_fk_normal_function,
+  m68k_fk_interrupt_handler,
+  m68k_fk_interrupt_thread
+};
+
 /* Variables in m68k.c; see there for details.  */
 extern const char *m68k_library_id_string;
 extern int m68k_last_compare_had_fp_operands;
Index: gcc/config/m68k/m68k.md
===================================================================
--- gcc/config/m68k/m68k.md	(revision 124112)
+++ gcc/config/m68k/m68k.md	(working copy)
@@ -6832,15 +6832,23 @@ (define_insn "*return"
   [(return)]
   ""
 {
-  if (m68k_interrupt_function_p (current_function_decl))
-    return "rte";
-  else if (current_function_pops_args)
+  switch (m68k_get_function_kind (current_function_decl))
     {
-      operands[0] = GEN_INT (current_function_pops_args);
-      return "rtd %0";
+    case m68k_fk_interrupt_handler:
+      return "rte";
+
+    case m68k_fk_interrupt_thread:
+      return "sleep";
+
+    default:
+      if (current_function_pops_args)
+	{
+	  operands[0] = GEN_INT (current_function_pops_args);
+	  return "rtd %0";
+	}
+      else
+	return "rts";
     }
-  else
-    return "rts";
 })
 
 (define_insn "*m68k_store_multiple"
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 124112)
+++ gcc/doc/extend.texi	(working copy)
@@ -2013,6 +2013,14 @@ indicate that the specified function is 
 will generate function entry and exit sequences suitable for use in an
 interrupt handler when this attribute is present.
 
+@item interrupt_thread
+@cindex interrupt thread functions on fido
+Use this attribute on fido, a subarchitecture of the m68k, to indicate
+that the specified function is an interrupt handler that is designed
+to run as a thread.  The compiler omits generate prologue/epilogue
+sequences and replaces the return instruction with a @code{sleep}
+instruction.  This attribute is available only on fido.
+
 @item kspisusp
 @cindex User stack pointer in interrupts on the Blackfin
 When used together with @code{interrupt_handler}, @code{exception_handler}
Index: gcc/testsuite/gcc.target/m68k/interrupt_thread-1.c
===================================================================
--- gcc/testsuite/gcc.target/m68k/interrupt_thread-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/m68k/interrupt_thread-1.c	(revision 0)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-mcpu=fidoa -O2 -fomit-frame-pointer" } */
+
+/* Check that interrupt_thread attribute works.  */
+
+#ifdef __mfido__
+extern void foo (void) __attribute__ ((interrupt_thread));
+
+int a, b, c, d;
+
+void bar (void);
+
+void
+foo (void)
+{
+  int w, x, y, z;
+
+  w = a;
+  x = b;
+  y = c;
+  z = d;
+
+  bar ();
+
+  a = w;
+  b = x;
+  c = y;
+  d = z;
+}
+#else
+/* If the current mutilib is, say, -mcpu=5485, the compiler gets
+   -mcpu=fidoa -mcpu=5485, where -mcpu=fidoa is overridden.  In that
+   case, we just print out "sleep" in the assembly file and pretend
+   that everything is all right.  */
+asm ("sleep");
+#endif
+
+/* "sleep" should be generated in place of "rts".  */
+/* { dg-final { scan-assembler-times "sleep" 1 } } */
+/* { dg-final { scan-assembler-times "rts" 0 } } */
+
+/* There should be no stack adjustment.  */
+/* { dg-final { scan-assembler-times "sp" 0 } } */
Index: gcc/testsuite/gcc.target/m68k/interrupt_thread-2.c
===================================================================
--- gcc/testsuite/gcc.target/m68k/interrupt_thread-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/m68k/interrupt_thread-2.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-mcpu=fidoa" } */
+
+/* Check that an error is issued for using more than one
+   interrupt_attribute at the same time.  */
+
+/* If the current mutilib is, say, -mcpu=5485, the compiler gets
+   -mcpu=fidoa -mcpu=5485, where -mcpu=fidoa is overridden.  In that
+   case, we just use two interrupt_handler attributes and expect the
+   same error.  */
+#ifdef __mfido___
+#define IH interrupt_thread
+#else
+#define IH interrupt_handler
+#endif
+
+extern void f1 (void) __attribute__((interrupt_handler, interrupt_handler)); /* { dg-error "error: multiple interrupt attributes not allowed" } */
+
+extern void f2 (void) __attribute__((interrupt_handler, IH)); /* { dg-error "error: multiple interrupt attributes not allowed" } */
+
+extern void f3 (void) __attribute__((IH, interrupt_handler)); /* { dg-error "error: multiple interrupt attributes not allowed" } */
+
+extern void f4 (void) __attribute__((IH, IH)); /* { dg-error "error: multiple interrupt attributes not allowed" } */
Index: gcc/testsuite/gcc.target/m68k/interrupt_thread-3.c
===================================================================
--- gcc/testsuite/gcc.target/m68k/interrupt_thread-3.c	(revision 0)
+++ gcc/testsuite/gcc.target/m68k/interrupt_thread-3.c	(revision 0)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-mcpu=cpu32" } */
+
+/* Check that interrupt_thread is rejected on CPUs other than
+   fido.  */
+
+extern void foo (void) __attribute__((interrupt_thread)); /* { dg-error "error: interrupt_thread is available only on fido" } */
Index: gcc/testsuite/gcc.target/m68k/m68k.exp
===================================================================
--- gcc/testsuite/gcc.target/m68k/m68k.exp	(revision 124112)
+++ gcc/testsuite/gcc.target/m68k/m68k.exp	(working copy)
@@ -17,7 +17,7 @@
 # GCC testsuite that uses the `dg.exp' driver.
 
 # Exit immediately if this isn't an m68k target.
-if ![istarget m68k*-*-*] then {
+if { ![istarget m68k*-*-*] && ![istarget fido*-*-*] } then {
   return
 }
 

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Roman Zippel]

Hi,

On Fri, 27 Apr 2007, Kazu Hirata wrote:

> Attached is a patch to add "interrupt_thread" attribute.
> 
> A function with the interrupt_thread attribute is compiled just like
> an ordinary function except that
> 
> - no register is saved or restored in prologue/epilogue.
> 
> - the 'rts' instruction is replaced with the 'sleep' instruction.

This looks rather independent, so it might be better to do this with two 
attributes to keep it more general.

bye, Roman

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Kazu Hirata]

Hi Roman,

>> Attached is a patch to add "interrupt_thread" attribute.
>>
>> A function with the interrupt_thread attribute is compiled just like
>> an ordinary function except that
>>
>> - no register is saved or restored in prologue/epilogue.
>>
>> - the 'rts' instruction is replaced with the 'sleep' instruction.
> 
> This looks rather independent, so it might be better to do this with two 
> attributes to keep it more general.

Hmm.  From usability point of view, I wonder if we want to keep things that 
flexible.  I find it nice to specify exactly one attribute whose name 
matches its purpose, "interrupt_handler" for "interrupt handler" for example.

For register save/store, we have three kinds:

A. normal functions
B. normal + CALL_USED_REGS (interrupt_handler style)
C. no registers (interrupt_thread style)

For the return instruction, we have:

1. rts
2. rte
3. sleep

We have the following combinations:

A1 normal functions, no attribute needed
A2 ??? D0, D1, A0, and A1 would be clobbered if used for interrupts
A3 ???
B1 ??? why save more registers than necessary?
B2 interrupt_handler
B3 ???
C1 ???
C2 ???
C3 interrupt_thread

Do any of these other than A1, B2, and C3 make sense in real world 
applications?

Thanks,

Kazu Hirata

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Roman Zippel]

Hi,

On Fri, 27 Apr 2007, Kazu Hirata wrote:

> +      /* Interrupt thread does not need to save any register.  */
> +      if (!interrupt_thread)
> +	for (regno = 16; regno < 24; regno++)
> +	  if (m68k_save_reg (regno, interrupt_handler))
> +	    {
> +	      mask |= 1 << (regno - FP0_REG);
> +	      saved++;
> +	    }

Independent of how this is exported via attributes, this should really be 
done via a single frame type, which specifies how registers are saved.

bye, Roman

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Kazu Hirata]

Hi Roman,

>> +      /* Interrupt thread does not need to save any register.  */
>> +      if (!interrupt_thread)
>> +	for (regno = 16; regno < 24; regno++)
>> +	  if (m68k_save_reg (regno, interrupt_handler))
>> +	    {
>> +	      mask |= 1 << (regno - FP0_REG);
>> +	      saved++;
>> +	    }
> 
> Independent of how this is exported via attributes, this should really be 
> done via a single frame type, which specifies how registers are saved.

Err, could you elaborate a little bit?  What do you mean by a single frame 
type?

Thanks,

Kazu Hirata

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Roman Zippel]

Hi,

On Fri, 27 Apr 2007, Kazu Hirata wrote:

> Hmm.  From usability point of view, I wonder if we want to keep things that
> flexible.  I find it nice to specify exactly one attribute whose name matches
> its purpose, "interrupt_handler" for "interrupt handler" for example.

Making it too specific isn't always good if we end up with lots of special 
cases.

> For register save/store, we have three kinds:
> 
> A. normal functions
> B. normal + CALL_USED_REGS (interrupt_handler style)
> C. no registers (interrupt_thread style)
> 
> For the return instruction, we have:
> 
> 1. rts
> 2. rte
> 3. sleep
> 
> We have the following combinations:
> 
> A1 normal functions, no attribute needed
> A2 ??? D0, D1, A0, and A1 would be clobbered if used for interrupts
> A3 ???
> B1 ??? why save more registers than necessary?
> B2 interrupt_handler
> B3 ???
> C1 ???
> C2 ???
> C3 interrupt_thread
> 
> Do any of these other than A1, B2, and C3 make sense in real world
> applications?

Saving all registers can be quite useful (e.g. to make sure they are on 
the stack for garbage collection).
It's more the return instructions that dictate the frame information 
usage, but the different register save models could be used independently 
(e.g. a short assembler entry/exit which calls one or more C functions).

bye, Roman

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Roman Zippel]

Hi,

On Fri, 27 Apr 2007, Kazu Hirata wrote:

> > Independent of how this is exported via attributes, this should really be
> > done via a single frame type, which specifies how registers are saved.
> 
> Err, could you elaborate a little bit?  What do you mean by a single frame
> type?

A single variable for just this information.

bye, Roman

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Kazu Hirata]

Hi Roman,

>>> Independent of how this is exported via attributes, this should really be
>>> done via a single frame type, which specifies how registers are saved.
>> Err, could you elaborate a little bit?  What do you mean by a single frame
>> type?
> 
> A single variable for just this information.

I'm afraid I still don't understand what you are suggesting here.  Are you 
suggesting to consolidate two variables, interrupt_thread and 
interrupt_handler, into one with bit fields or something?  Would you care 
to write pseudo code maybe?

Thanks,

Kazu Hirata

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Roman Zippel]

Hi,

On Fri, 27 Apr 2007, Kazu Hirata wrote:

> I'm afraid I still don't understand what you are suggesting here.  Are you
> suggesting to consolidate two variables, interrupt_thread and
> interrupt_handler, into one with bit fields or something?  Would you care to
> write pseudo code maybe?

These two variables:

	if (!interrupt_thread)
		...
		if (m68k_save_reg (regno, interrupt_handler))

should be merged into one and the whole logic should be in 
m68k_save_reg().

bye, Roman

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Kazu Hirata]

Hi Roman,

>> For register save/store, we have three kinds:
>>
>> A. normal functions
>> B. normal + CALL_USED_REGS (interrupt_handler style)
>> C. no registers (interrupt_thread style)
>>
>> For the return instruction, we have:
>>
>> 1. rts
>> 2. rte
>> 3. sleep
>>
>> We have the following combinations:
>>
>> A1 normal functions, no attribute needed
>> A2 ??? D0, D1, A0, and A1 would be clobbered if used for interrupts
>> A3 ???
>> B1 ??? why save more registers than necessary?
>> B2 interrupt_handler
>> B3 ???
>> C1 ???
>> C2 ???
>> C3 interrupt_thread
>>
>> Do any of these other than A1, B2, and C3 make sense in real world
>> applications?
> 
> Saving all registers can be quite useful (e.g. to make sure they are on 
> the stack for garbage collection).
> It's more the return instructions that dictate the frame information 
> usage, but the different register save models could be used independently 
> (e.g. a short assembler entry/exit which calls one or more C functions).

What do you mean by "garbage collection" and "short assember entry/exit"?

Kazu Hirata

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Kazu Hirata]

Hi Jeff and Andreas,

> gcc/
> 2007-04-27  Kazu Hirata  <kazu@codesourcery.com>
> 
> 	* config/m68k/m68k-protos.h: Rename m68k_interrupt_function_p
> 	to m68k_get_function_kind.  Update its prototype.
> 	* config/m68k/m68k.c (m68k_attribute_table): Add an entry for
> 	interrupt_thread.
> 	(m68k_interrupt_function_p): Return enum m68k_function_type
> 	instead of bool.  Rename to m68k_get_function_kind.
> 	(m68k_handle_fndecl_attribute): Reject interrupt_thread if the
> 	target is not fido.
> 	(m68k_compute_frame_layout): Don't mark any register for save
> 	if an interrupt_thread attribute is specified.
> 	(m68k_hard_regno_rename_ok): Update a use of
> 	m68k_interrupt_function_p.
> 	* config/m68k/m68k.h (EPILOGUE_USES): Update a use of
> 	m68k_interrupt_function_p.
> 	(m68k_function_type): New.
> 	* config/m68k/m68k.md (*return): Output a 'sleep' instruction
> 	for a function with an interrupt_thread attribute.
> 	* doc/extend.texi: Document the interrupt_thread attribute.
> 
> gcc/testsuite/
> 2007-04-27  Kazu Hirata  <kazu@codesourcery.com>
> 
> 	* gcc.target/m68k/interrupt_thread-1.c,
> 	gcc.target/m68k/interrupt_thread-2.c,
> 	gcc.target/m68k/interrupt_thread-3.c: New.
> 	* gcc.target/m68k/m68k.exp: Accept fido.

Do you have a comment about this patch?  Roman Zippel expressed some 
implementation ideas, but is the feature itself OK?

The original post is at:

http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01880.html

Thanks,

Kazu Hirata

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Andreas Schwab]

Kazu Hirata <kazu@codesourcery.com> writes:

> Hi Jeff and Andreas,
>
>> gcc/
>> 2007-04-27  Kazu Hirata  <kazu@codesourcery.com>
>>
>> 	* config/m68k/m68k-protos.h: Rename m68k_interrupt_function_p
>> 	to m68k_get_function_kind.  Update its prototype.
>> 	* config/m68k/m68k.c (m68k_attribute_table): Add an entry for
>> 	interrupt_thread.
>> 	(m68k_interrupt_function_p): Return enum m68k_function_type
>> 	instead of bool.  Rename to m68k_get_function_kind.
>> 	(m68k_handle_fndecl_attribute): Reject interrupt_thread if the
>> 	target is not fido.
>> 	(m68k_compute_frame_layout): Don't mark any register for save
>> 	if an interrupt_thread attribute is specified.
>> 	(m68k_hard_regno_rename_ok): Update a use of
>> 	m68k_interrupt_function_p.
>> 	* config/m68k/m68k.h (EPILOGUE_USES): Update a use of
>> 	m68k_interrupt_function_p.
>> 	(m68k_function_type): New.
>> 	* config/m68k/m68k.md (*return): Output a 'sleep' instruction
>> 	for a function with an interrupt_thread attribute.
>> 	* doc/extend.texi: Document the interrupt_thread attribute.
>>
>> gcc/testsuite/
>> 2007-04-27  Kazu Hirata  <kazu@codesourcery.com>
>>
>> 	* gcc.target/m68k/interrupt_thread-1.c,
>> 	gcc.target/m68k/interrupt_thread-2.c,
>> 	gcc.target/m68k/interrupt_thread-3.c: New.
>> 	* gcc.target/m68k/m68k.exp: Accept fido.
>
> Do you have a comment about this patch?  Roman Zippel expressed some
> implementation ideas, but is the feature itself OK?
>
> The original post is at:
>
> http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01880.html

I think the patch is OK as is.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] m68k.h: Add "interrupt_thread" attribute.

[Roman Zippel]

Hi,

On Sun, 13 May 2007, Andreas Schwab wrote:

> > http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01880.html
> 
> I think the patch is OK as is.

I still disagree, external API is one thing, but at least the 
implementation should be cleaned up.
It would add the following to m68k_compute_frame_layout():

+  bool interrupt_handler = func_kind == m68k_fk_interrupt_handler;
+  bool interrupt_thread = func_kind == m68k_fk_interrupt_thread;

which really belong in m68k_save_reg().

bye, Roman