Patchwork Fix "naked" attribute (PR target/44290)

login
register
mail settings
Submitter Jie Zhang
Date July 22, 2010, 12:56 p.m.
Message ID <4C483FEE.5040605@codesourcery.com>
Download mbox | patch
Permalink /patch/59567/
State New
Headers show

Comments

Jie Zhang - July 22, 2010, 12:56 p.m.
The "naked" attribute should imply "noinline" and "noclone". This patch 
implements this for all the 5 targets which currently support the 
"naked" attribute: ARM, AVR, MCORE, RX, SPU.

IPA-SRA should not apply to functions with "noclone" attribute. This 
patch adds a check to tree_versionable_function_p in 
ipa_sra_preliminary_function_checks.

Bootstrapped and regression tested on x86_64. Testing for ARM NEON is 
OK, too.

cc1 builds well for AVR, MCORE, RX, SPU. The two new tests pass on AVR, 
MCORE, RX.

cc1 builds well for SPU, but the test cases cause ICE, even the patch is 
not applied. It seems that SPU target has a bug for the "naked" 
attribute somewhere. This is not a regression, I think.

Is it OK?


Regards,
Mark Mitchell - July 22, 2010, 9:59 p.m.
Jie Zhang wrote:

> The "naked" attribute should imply "noinline" and "noclone". This patch
> implements this for all the 5 targets which currently support the
> "naked" attribute: ARM, AVR, MCORE, RX, SPU.

I completely agree with the spirit of this patch.  "naked" functions are
very weird things, and the compiler should be very cautious in doing
things with them.  It shouldn't make copies of them (inline or clone)
because we really have no way of knowing what's in there, including (for
example) static data declared in assembly code.

But, I think the implementation could be more architecture-independent.
 I'm prepared to say that any architecture that supports the "naked"
attribute *must* implement it with the semantics on the architectures
you name above.  That means that the ${arch}_insert_attributes functions
you have could be architecture-independent.  Just put that code into the
architecture-independent attribute-processing function.

Thanks,

Patch



	PR target/44290
	* tree-sra.c (ipa_sra_preliminary_function_checks): Return
	false if ! tree_versionable_function_p.
	* config/arm/arm.c (arm_insert_attributes): New.
	(TARGET_INSERT_ATTRIBUTES): Define.
	* config/avr/avr.c (avr_insert_attributes): Insert "noinline"
	and "noclone" if "naked".
	* config/mcore/mcore.c (mcore_insert_attributes): New.
	(TARGET_INSERT_ATTRIBUTES): Define.
	* config/rx/rx.c (rx_insert_attributes): New.
	(TARGET_INSERT_ATTRIBUTES): Define.
	* config/spu/spu.c (spu_insert_attributes): New.
	(TARGET_INSERT_ATTRIBUTES): Define.
	* doc/extend.texi: Remove IP2K from the description of naked
	attribute.  Add MCORE instead.

	testsuite/
	PR target/44290
	* gcc.dg/pr44290-1.c: New test.
	* gcc.dg/pr44290-2.c: New test.



Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 162396)
+++ doc/extend.texi	(working copy)
@@ -2743,7 +2743,7 @@  and newer.
 
 @item naked
 @cindex function without a prologue/epilogue code
-Use this attribute on the ARM, AVR, IP2K, RX and SPU ports to indicate that
+Use this attribute on the ARM, AVR, MCORE, RX and SPU ports to indicate that
 the specified function does not need prologue/epilogue sequences generated by
 the compiler.  It is up to the programmer to provide these sequences. The 
 only statements that can be safely included in naked functions are 
Index: testsuite/gcc.dg/pr44290-1.c
===================================================================
--- testsuite/gcc.dg/pr44290-1.c	(revision 0)
+++ testsuite/gcc.dg/pr44290-1.c	(revision 0)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile { target arm*-*-* avr-*-* mcore-*-* rx-*-* spu-*-* } } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+static void __attribute__((naked))
+foo(void *from, void *to)
+{
+  asm volatile("dummy"::"r"(from), "r"(to));
+}
+
+unsigned int fie[2];
+
+void fum(void *to)
+{
+  foo(fie, to);
+}
+
+/* { dg-final { scan-tree-dump "foo \\\(void \\\* from, void \\\* to\\\)" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: testsuite/gcc.dg/pr44290-2.c
===================================================================
--- testsuite/gcc.dg/pr44290-2.c	(revision 0)
+++ testsuite/gcc.dg/pr44290-2.c	(revision 0)
@@ -0,0 +1,24 @@ 
+/* { dg-do compile { target arm*-*-* avr-*-* mcore-*-* rx-*-* spu-*-* } } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+static unsigned long __attribute__((naked))
+foo (unsigned long base)
+{
+  asm volatile ("dummy");
+}
+unsigned long
+bar (void)
+{
+  static int start, set;
+
+  if (!set)
+    {
+      set = 1;
+      start = foo (0);
+    }
+
+  return foo (start);
+}
+
+/* { dg-final { scan-tree-dump "foo \\\(long unsigned int base\\\)" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 162396)
+++ tree-sra.c	(working copy)
@@ -4285,6 +4285,13 @@  modify_function (struct cgraph_node *nod
 static bool
 ipa_sra_preliminary_function_checks (struct cgraph_node *node)
 {
+  if (!tree_versionable_function_p (current_function_decl))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Function isn't allowed to be versioned.\n");
+      return false;
+    }
+
   if (!cgraph_node_can_be_local_p (node))
     {
       if (dump_file)
Index: config/spu/spu.c
===================================================================
--- config/spu/spu.c	(revision 162396)
+++ config/spu/spu.c	(working copy)
@@ -182,6 +182,7 @@  static int spu_sched_reorder (FILE *, in
 static tree spu_handle_fndecl_attribute (tree * node, tree name, tree args,
 					 int flags,
 					 bool *no_add_attrs);
+static void spu_insert_attributes (tree, tree *);
 static tree spu_handle_vector_attribute (tree * node, tree name, tree args,
 					 int flags,
 					 bool *no_add_attrs);
@@ -371,6 +372,9 @@  static const struct attribute_spec spu_a
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE spu_attribute_table
 
+#undef  TARGET_INSERT_ATTRIBUTES
+#define TARGET_INSERT_ATTRIBUTES spu_insert_attributes
+
 #undef TARGET_ASM_INTEGER
 #define TARGET_ASM_INTEGER spu_assemble_integer
 
@@ -3922,6 +3926,24 @@  spu_naked_function_p (tree func)
   return a != NULL_TREE;
 }
 
+/* A "naked" attribute implies "noinline" and "noclone".  */
+
+static void
+spu_insert_attributes (tree node, tree *attributes)
+{
+  if (TREE_CODE (node) != FUNCTION_DECL
+      || lookup_attribute ("naked", *attributes) == NULL)
+    return;
+
+  if (lookup_attribute ("noinline", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noinline"),
+			     NULL, *attributes);
+
+  if (lookup_attribute ("noclone", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noclone"),
+			     NULL, *attributes);
+}
+
 int
 spu_initial_elimination_offset (int from, int to)
 {
Index: config/rx/rx.c
===================================================================
--- config/rx/rx.c	(revision 162396)
+++ config/rx/rx.c	(working copy)
@@ -2086,6 +2086,24 @@  rx_handle_func_attribute (tree * node,
   return NULL_TREE;
 }
 
+/* A "naked" attribute implies "noinline" and "noclone".  */
+
+static void
+rx_insert_attributes (tree node, tree *attributes)
+{
+  if (TREE_CODE (node) != FUNCTION_DECL
+      || lookup_attribute ("naked", *attributes) == NULL)
+    return;
+
+  if (lookup_attribute ("noinline", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noinline"),
+			     NULL, *attributes);
+
+  if (lookup_attribute ("noclone", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noclone"),
+			     NULL, *attributes);
+}
+
 /* Table of RX specific attributes.  */
 const struct attribute_spec rx_attribute_table[] =
 {
@@ -2726,6 +2744,9 @@  rx_memory_move_cost (enum machine_mode m
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE		rx_attribute_table
 
+#undef  TARGET_INSERT_ATTRIBUTES
+#define TARGET_INSERT_ATTRIBUTES	rx_insert_attributes
+
 #undef  TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START			rx_file_start
 
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 162396)
+++ config/avr/avr.c	(working copy)
@@ -4905,6 +4905,20 @@  avr_insert_attributes (tree node, tree *
 	 thing const in the first place?  */
       TREE_READONLY (node) = 1;
     }
+
+  /* A "naked" attribute implies "noinline" and "noclone".  */
+
+  if (TREE_CODE (node) != FUNCTION_DECL
+      || lookup_attribute ("naked", *attributes) == NULL)
+    return;
+
+  if (lookup_attribute ("noinline", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noinline"),
+			     NULL, *attributes);
+
+  if (lookup_attribute ("noclone", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noclone"),
+			     NULL, *attributes);
 }
 
 /* A get_unnamed_section callback for switching to progmem_section.  */
Index: config/mcore/mcore.c
===================================================================
--- config/mcore/mcore.c	(revision 162396)
+++ config/mcore/mcore.c	(working copy)
@@ -128,6 +128,7 @@  static void       mcore_mark_dllimport  
 static int        mcore_dllexport_p             (tree);
 static int        mcore_dllimport_p             (tree);
 static tree       mcore_handle_naked_attribute  (tree *, tree, tree, int, bool *);
+static void       mcore_insert_attributes       (tree, tree *);
 #ifdef OBJECT_FORMAT_ELF
 static void	  mcore_asm_named_section       (const char *,
 						 unsigned int, tree);
@@ -186,6 +187,8 @@  static const struct attribute_spec mcore
 
 #undef  TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE 		mcore_attribute_table
+#undef  TARGET_INSERT_ATTRIBUTES
+#define TARGET_INSERT_ATTRIBUTES	mcore_insert_attributes
 #undef  TARGET_ASM_UNIQUE_SECTION
 #define TARGET_ASM_UNIQUE_SECTION 	mcore_unique_section
 #undef  TARGET_ASM_FUNCTION_RODATA_SECTION
@@ -3058,6 +3061,24 @@  mcore_handle_naked_attribute (tree * nod
   return NULL_TREE;
 }
 
+/* A "naked" attribute implies "noinline" and "noclone".  */
+
+static void
+mcore_insert_attributes (tree node, tree *attributes)
+{
+  if (TREE_CODE (node) != FUNCTION_DECL
+      || lookup_attribute ("naked", *attributes) == NULL)
+    return;
+
+  if (lookup_attribute ("noinline", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noinline"),
+			     NULL, *attributes);
+
+  if (lookup_attribute ("noclone", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noclone"),
+			     NULL, *attributes);
+}
+
 /* ??? It looks like this is PE specific?  Oh well, this is what the
    old code did as well.  */
 
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 162396)
+++ config/arm/arm.c	(working copy)
@@ -116,6 +116,7 @@  static unsigned long arm_compute_save_re
 static unsigned long arm_compute_save_reg_mask (void);
 static unsigned long arm_isr_value (tree);
 static unsigned long arm_compute_func_type (void);
+static void arm_insert_attributes (tree, tree *);
 static tree arm_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
 static tree arm_handle_pcs_attribute (tree *, tree, tree, int, bool *);
 static tree arm_handle_isr_attribute (tree *, tree, tree, int, bool *);
@@ -315,6 +316,9 @@  static const struct attribute_spec arm_a
 #undef  TARGET_SET_DEFAULT_TYPE_ATTRIBUTES
 #define TARGET_SET_DEFAULT_TYPE_ATTRIBUTES arm_set_default_type_attributes
 
+#undef  TARGET_INSERT_ATTRIBUTES
+#define TARGET_INSERT_ATTRIBUTES arm_insert_attributes
+
 #undef  TARGET_SCHED_ADJUST_COST
 #define TARGET_SCHED_ADJUST_COST arm_adjust_cost
 
@@ -4592,6 +4596,24 @@  arm_pr_long_calls_off (struct cpp_reader
   arm_pragma_long_calls = OFF;
 }
 
+/* A "naked" attribute implies "noinline" and "noclone".  */
+
+static void
+arm_insert_attributes (tree node, tree *attributes)
+{
+  if (TREE_CODE (node) != FUNCTION_DECL
+      || lookup_attribute ("naked", *attributes) == NULL)
+    return;
+
+  if (lookup_attribute ("noinline", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noinline"),
+			     NULL, *attributes);
+
+  if (lookup_attribute ("noclone", *attributes) == NULL)
+    *attributes = tree_cons (get_identifier ("noclone"),
+			     NULL, *attributes);
+}
+
 /* Handle an attribute requiring a FUNCTION_DECL;
    arguments as in struct attribute_spec.handler.  */
 static tree