[patch] m68k: Don't use a sibcall from an interrupt handler (Take 2)

[patch] m68k: Don't use a sibcall from an interrupt handler (Take 2)

[Kazu Hirata]

Hi,

Attached is a revised patch to teach gcc not to use a sibcall from an
interrupt handler.

The previous iteration of this patch is at:

  http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01690.html

In this iteration, I have moved a part of the function comment about
nested functions inside the function.  Also, I've changed TREE_OPERAND
to CALL_EXPR_STATIC_CHAIN so that the code will be more self
explanatory.

Tested on m68k-elf.  OK to apply?

Kazu Hirata

gcc/
2007-08-25  Nathan Sidwell  <nathan@codesourcery.com>

	* config/m68k/m68k.c (m68k_get_function_kind): Assert we're never
	given a non-function.
	(m68k_ok_for_sibcall_p): Only sibcall functions of the same kind.

gcc/testsuite/
2007-08-25  Nathan Sidwell  <nathan@codesourcery.com>
	    Kazu Hirata  <kazu@codesourcery.com>

	* gcc.target/m68k/interrupt-1.c: New.

Index: gcc/config/m68k/m68k.c
===================================================================
--- gcc/config/m68k/m68k.c	(revision 127798)
+++ gcc/config/m68k/m68k.c	(working copy)
@@ -631,9 +631,8 @@ m68k_get_function_kind (tree func)
 {
   tree a;
 
-  if (TREE_CODE (func) != FUNCTION_DECL)
-    return false;
-
+  gcc_assert (TREE_CODE (func) == FUNCTION_DECL);
+  
   a = lookup_attribute ("interrupt", DECL_ATTRIBUTES (func));
   if (a != NULL_TREE)
     return m68k_fk_interrupt_handler;
@@ -1250,14 +1249,30 @@ flags_in_68881 (void)
   return cc_status.flags & CC_IN_68881;
 }
 
-/* Implement TARGET_FUNCTION_OK_FOR_SIBCALL_P.  We cannot use sibcalls
-   for nested functions because we use the static chain register for
-   indirect calls.  */
+/* Implement TARGET_FUNCTION_OK_FOR_SIBCALL_P.  */
 
 static bool
-m68k_ok_for_sibcall_p (tree decl ATTRIBUTE_UNUSED, tree exp)
+m68k_ok_for_sibcall_p (tree decl, tree exp)
 {
-  return TREE_OPERAND (exp, 2) == NULL;
+  enum m68k_function_kind kind;
+  
+  /* We cannot use sibcalls for nested functions because we use the
+     static chain register for indirect calls.  */
+  if (CALL_EXPR_STATIC_CHAIN (exp))
+    return false;
+
+  kind = m68k_get_function_kind (current_function_decl);
+  if (kind == m68k_fk_normal_function)
+    /* We can always sibcall from a normal function, because it's
+       undefined if it is calling an interrupt function.  */
+    return true;
+
+  /* Otherwise we can only sibcall if the function kind is known to be
+     the same.  */
+  if (decl && m68k_get_function_kind (decl) == kind)
+    return true;
+  
+  return false;
 }
 
 /* Convert X to a legitimate function call memory reference and return the
Index: gcc/testsuite/gcc.target/m68k/interrupt-1.c
===================================================================
--- gcc/testsuite/gcc.target/m68k/interrupt-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/m68k/interrupt-1.c	(revision 0)
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler "jra\[ \t\]*interrupt_sibcall" } } */
+/* { dg-final { scan-assembler "jbsr\[ \t\]*interrupt_call" } } */
+/* { dg-final { scan-assembler "jra\[ \t\]*normal_sibcall" } } */
+
+void normal_sibcall (void);
+void interrupt_call (void);
+void __attribute ((interrupt)) interrupt_sibcall (void);
+
+void normal (void)
+{
+  normal_sibcall ();
+}
+
+void __attribute ((interrupt)) interrupt (void)
+{
+  interrupt_call ();
+}
+
+void __attribute ((interrupt)) interrupt_2 (void)
+{
+  interrupt_sibcall ();
+}

Re: [patch] m68k: Don't use a sibcall from an interrupt handler (Take 2)

[Andreas Schwab]

Kazu Hirata <kazu@codesourcery.com> writes:

> Index: gcc/testsuite/gcc.target/m68k/interrupt-1.c
> ===================================================================
> --- gcc/testsuite/gcc.target/m68k/interrupt-1.c	(revision 0)
> +++ gcc/testsuite/gcc.target/m68k/interrupt-1.c	(revision 0)
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler "jra\[ \t\]*interrupt_sibcall" } } */
> +/* { dg-final { scan-assembler "jbsr\[ \t\]*interrupt_call" } } */
> +/* { dg-final { scan-assembler "jra\[ \t\]*normal_sibcall" } } */

That needs to handle the possible variants of m68k_symbolic_call and
m68k_symbolic_jump, and targets that use a label prefix.  Ok with that
change.

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."