[patch] m68k: Don't use a sibcall from an interrupt handler.

[patch] m68k: Don't use a sibcall from an interrupt handler.

[Kazu Hirata]

Hi,

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

Changes in m68k_ok_for_sibcall_p should be straightforward.  If we
don't have a prototype, we cannot use a sibcall.  If we are normal
function, we can sibcall anyone.  Otherwise, the "function kind" must
match between the caller and callee.

Tested on m68k-elf.  OK to apply?

Kazu Hirata

gcc/
2007-08-24  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-24  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 127787)
+++ 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;
@@ -1255,9 +1254,25 @@ flags_in_68881 (void)
    indirect calls.  */
 
 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;
+  
+  if (TREE_OPERAND (exp, 2))
+    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.

[Richard Sandiford]

Kazu Hirata <kazu@codesourcery.com> writes:
> @@ -1255,9 +1254,25 @@ flags_in_68881 (void)
>     indirect calls.  */
>  
>  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;
> +  
> +  if (TREE_OPERAND (exp, 2))
> +    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;

Minor nit, but the comment for this function is:

/* Implement TARGET_FUNCTION_OK_FOR_SIBCALL_P.  We cannot use sibcalls
   for nested functions because we use the static chain register for
   indirect calls.  */

Now that the function is checking for more than nested functions,
the second sentence seems out of place.  I think you should move it
above the TREE_OPERAND check instead.  (I'm sure I'm not the only one
who has no idea what TREE_OPERAND (exp, 2) is testing without that
comment to explain things.)

Richard

Re: [patch] m68k: Don't use a sibcall from an interrupt handler.

[Kazu Hirata]

Hi Richard,

> Minor nit, but the comment for this function is:
> 
> /* Implement TARGET_FUNCTION_OK_FOR_SIBCALL_P.  We cannot use sibcalls
>    for nested functions because we use the static chain register for
>    indirect calls.  */
> 
> Now that the function is checking for more than nested functions,
> the second sentence seems out of place.  I think you should move it
> above the TREE_OPERAND check instead.  (I'm sure I'm not the only one
> who has no idea what TREE_OPERAND (exp, 2) is testing without that
> comment to explain things.)

Thank you for spotting this.  Upon looking into tree.h, I found 
CALL_EXPR_STATIC_CHAIN.  I'll post a new patch with your comment suggestion 
and use of CALL_EXPR_STATIC_CHAIN incorporated.

Thanks,

Kazu Hirata