Re: PR 56524: TREE_OPTIMIZATION_OPTABS vs. mips16

[Richard Sandiford <rdsandiford at googlemail dot com>]

Jakub Jelinek <jakub@redhat.com> writes:
> So I think the
>
>       if (TREE_OPTIMIZATION_OPTABS (optnode))
>         ggc_free (TREE_OPTIMIZATION_OPTABS (optnode));
>
> has to be removed (of course the second ggc_free, if memcmp returned 0, is
> desirable).

*blush*.  Thanks for catching that.

> Otherwise looks good.

OK, here's what I installed after retesting on x86_64-linux-gnu and
mips64-linux-gnu.

Thanks,
Richard


gcc/
	PR middle-end/56524
	* tree.h (tree_optimization_option): Rename target_optabs to optabs.
	Add base_optabs.
	(TREE_OPTIMIZATION_OPTABS): Update after previous field change.
	(TREE_OPTIMIZATION_BASE_OPTABS): New macro.
	(save_optabs_if_changed): Replace with...
	(init_tree_optimization_optabs): ...this.
	* optabs.c (save_optabs_if_changed): Rename to...
	(init_tree_optimization_optabs): ...this.  Take the optimization node
	as argument.  Do nothing if the base optabs are already correct.
	Reuse the existing TREE_OPTIMIZATION_OPTABS memory if we need
	to recompute optabs.
	* function.h (function): Remove optabs field.
	* function.c (invoke_set_current_function_hook): Call
	init_tree_optimization_optabs.  Use the result to initialize
	this_fn_optabs.

gcc/c-family/
	PR middle-end/56524
	* c-common.c (handle_optimize_attribute): Don't call
	save_optabs_if_changed.

gcc/testsuite/
	PR middle-end/56524
	* gcc.target/mips/pr56524.c: New test.

Index: gcc/tree.h
===================================================================
--- gcc/tree.h	2013-03-08 07:52:41.865887012 +0000
+++ gcc/tree.h	2013-03-08 07:54:18.122879783 +0000
@@ -3589,21 +3589,26 @@ struct GTY(()) tree_optimization_option
 
   /* Target optabs for this set of optimization options.  This is of
      type `struct target_optabs *'.  */
-  unsigned char *GTY ((atomic)) target_optabs;
+  unsigned char *GTY ((atomic)) optabs;
+
+  /* The value of this_target_optabs against which the optabs above were
+     generated.  */
+  struct target_optabs *GTY ((skip)) base_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
 #define TREE_OPTIMIZATION_OPTABS(NODE) \
-  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_optabs)
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.optabs)
+
+#define TREE_OPTIMIZATION_BASE_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.base_optabs)
 
 /* Return a tree node that encapsulates the current optimization options.  */
 extern tree build_optimization_node (void);
 
-/* Save a new set of target_optabs in a TREE_OPTIMIZATION node if the
-   current set of optabs has changed.  */
-extern void save_optabs_if_changed (tree);
+extern void init_tree_optimization_optabs (tree);
 
 /* Target options used by a function.  */
 
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	2013-03-08 07:54:02.938171541 +0000
+++ gcc/optabs.c	2013-03-08 07:54:29.833402743 +0000
@@ -6208,31 +6208,32 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
-/* Recompute the optabs and save them if they have changed.  */
+/* Use the current target and options to initialize
+   TREE_OPTIMIZATION_OPTABS (OPTNODE).  */
 
 void
-save_optabs_if_changed (tree fndecl)
+init_tree_optimization_optabs (tree optnode)
 {
-  /* ?? If this fails, we should temporarily restore the default
-     target first (set_cfun (NULL) ??), do the rest of this function,
-     and then restore it.  */
-  gcc_assert (this_target_optabs == &default_target_optabs);
+  /* Quick exit if we have already computed optabs for this target.  */
+  if (TREE_OPTIMIZATION_BASE_OPTABS (optnode) == this_target_optabs)
+    return;
 
+  /* Forget any previous information and set up for the current target.  */
+  TREE_OPTIMIZATION_BASE_OPTABS (optnode) = this_target_optabs;
   struct target_optabs *tmp_optabs = (struct target_optabs *)
-    ggc_alloc_atomic (sizeof (struct target_optabs));
-  tree optnode = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl);
+    TREE_OPTIMIZATION_OPTABS (optnode);
+  if (tmp_optabs)
+    memset (tmp_optabs, 0, sizeof (struct target_optabs));
+  else
+    tmp_optabs = (struct target_optabs *)
+      ggc_alloc_atomic (sizeof (struct target_optabs));
 
   /* Generate a new set of optabs into tmp_optabs.  */
   init_all_optabs (tmp_optabs);
 
   /* If the optabs changed, record it.  */
   if (memcmp (tmp_optabs, this_target_optabs, sizeof (struct target_optabs)))
-    {
-      if (TREE_OPTIMIZATION_OPTABS (optnode))
-	ggc_free (TREE_OPTIMIZATION_OPTABS (optnode));
-
-      TREE_OPTIMIZATION_OPTABS (optnode) = (unsigned char *) tmp_optabs;
-    }
+    TREE_OPTIMIZATION_OPTABS (optnode) = (unsigned char *) tmp_optabs;
   else
     {
       TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
Index: gcc/function.h
===================================================================
--- gcc/function.h	2013-03-08 07:52:41.865887012 +0000
+++ gcc/function.h	2013-03-08 07:54:18.118879603 +0000
@@ -580,9 +580,6 @@ struct GTY(()) function {
      a string describing the reason for failure.  */
   const char * GTY((skip)) cannot_be_copied_reason;
 
-  /* Optabs for this function.  This is of type `struct target_optabs *'.  */
-  unsigned char *GTY ((atomic)) optabs;
-
   /* Collected bit flags.  */
 
   /* Number of units of general registers that need saving in stdarg
Index: gcc/function.c
===================================================================
--- gcc/function.c	2013-03-08 07:52:41.865887012 +0000
+++ gcc/function.c	2013-03-08 07:54:18.117879558 +0000
@@ -4400,25 +4400,14 @@ invoke_set_current_function_hook (tree f
 	}
 
       targetm.set_current_function (fndecl);
+      this_fn_optabs = this_target_optabs;
 
-      if (opts == optimization_default_node)
-	this_fn_optabs = this_target_optabs;
-      else
+      if (opts != optimization_default_node)
 	{
-	  struct function *fn = DECL_STRUCT_FUNCTION (fndecl);
-	  if (fn->optabs == NULL)
-	    {
-	      if (this_target_optabs == &default_target_optabs)
-		fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
-	      else
-		{
-		  fn->optabs = (unsigned char *)
-		    ggc_alloc_atomic (sizeof (struct target_optabs));
-		  init_all_optabs ((struct target_optabs *) fn->optabs);
-		}
-	    }
-	  this_fn_optabs = fn->optabs ? (struct target_optabs *) fn->optabs
-	                              : this_target_optabs;
+	  init_tree_optimization_optabs (opts);
+	  if (TREE_OPTIMIZATION_OPTABS (opts))
+	    this_fn_optabs = (struct target_optabs *)
+	      TREE_OPTIMIZATION_OPTABS (opts);
 	}
     }
 }
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	2013-03-08 07:52:41.865887012 +0000
+++ gcc/c-family/c-common.c	2013-03-08 07:54:18.115879468 +0000
@@ -8947,8 +8947,6 @@ handle_optimize_attribute (tree *node, t
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
-      save_optabs_if_changed (*node);
-
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
Index: gcc/testsuite/gcc.target/mips/pr56524.c
===================================================================
--- /dev/null	2013-03-06 23:07:14.594799386 +0000
+++ gcc/testsuite/gcc.target/mips/pr56524.c	2013-03-08 07:54:18.123879828 +0000
@@ -0,0 +1,8 @@
+/* { dg-options "-mips16" } */
+
+void bar (void) {}
+
+void __attribute__((optimize("schedule-insns")))
+foo (void)
+{
+}