PR target/52555: attribute optimize is overriding command line options

PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

The problem here is that -ffast-math is overridden when switching 
optimization options on a per function basis with 
__attribute__((optimize("O"))).

The x86 ceilf* instructions depend on unsafe math optimizations, but the 
optabs are created at the beginning of the compilation.  When fast math 
becomes unavailable, the target can no longer find the instructions.

Fixed by recalculating the optabs when creating new optimization nodes, 
and switching to these (if appropriate) at cfun switching time.

How does this look?

Jakub, what's this you mention in the PR about caching 
__optimize__((3))?  You also mention I shouldn't compare against 
this_target_optabs, but default_target_optabs.  But what if 
this_target_optabs has changed?  (See patch).

Tested on x86-64 Linux.  It would be nice if someone with access to a 
SWITCHABLE_TARGET platform (mips) could also test this.

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 5413 bytes --]

commit fcac0360de909e6d96fe005a3012139d487d2db8
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Feb 11 15:51:24 2013 -0600

    	PR target/52555
    	* tree.h (struct tree_optimization_option): New field
    	target_optabs.
    	(TREE_OPTIMIZATION_OPTABS): New.
    	(save_optabs_if_changed): Protoize.
    	* optabs.h: Always declare this_target_optabs.
    	* optabs.c (save_optabs_if_changed): New.
    	Always declare this_target_optabs.
    	* function.c (invoke_set_current_function_hook): Set
    	this_target_optabs if there is one in the optimization node.
    c-family/
    	* c-common.c (handle_optimize_attribute): Call
    	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..3711e69 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node));
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..f37e91f 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4397,6 +4397,13 @@ invoke_set_current_function_hook (tree fndecl)
 	{
 	  optimization_current_node = opts;
 	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
+
+	  /* Change optabs if needed.  */
+	  if (TREE_OPTIMIZATION_OPTABS (opts))
+	    this_target_optabs
+	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
+	  else
+	    this_target_optabs = &default_target_optabs;
 	}
 
       targetm.set_current_function (fndecl);
diff --git a/gcc/optabs.c b/gcc/optabs.c
index 8a3d3a9..3ce1701 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,8 +44,8 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
-#if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
+#if SWITCHABLE_TARGET
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
 #endif
 
@@ -6184,6 +6184,41 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs.  If they have changed, save the new set of
+   optabs in the optimization node OPTNODE.  */
+
+void
+save_optabs_if_changed (tree optnode)
+{
+  struct target_optabs *save_target_optabs = this_target_optabs;
+  struct target_optabs *tmp_target_optabs = XNEW (struct target_optabs);
+
+  /* Generate a new set of optabs into tmp_target_optabs.  */
+  memset (tmp_target_optabs, 0, sizeof (struct target_optabs));
+  this_target_optabs = tmp_target_optabs;
+  init_all_optabs ();
+  this_target_optabs = save_target_optabs;
+
+  /* If the optabs changed, record it in the node.  */
+  if (memcmp (tmp_target_optabs, this_target_optabs,
+	      sizeof (struct target_optabs)))
+    {
+      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
+	 multiple ((optimize)) attributes for the same function.  Is
+	 this even valid?  For now, just clobber the existing entry
+	 with the new optabs.  */
+      if (TREE_OPTIMIZATION_OPTABS (optnode))
+	free (TREE_OPTIMIZATION_OPTABS (optnode));
+
+      TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      free (tmp_target_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..2e8b6ec 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,11 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
-#if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
-#else
-#define this_target_optabs (&default_target_optabs)
-#endif
 \f
 /* Define functions given in optabs.c.  */
 
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..eddbca8 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  void *GTY ((skip)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Mon, Feb 11, 2013 at 06:15:05PM -0600, Aldy Hernandez wrote:
> How does this look?

Looks good to me.

> Jakub, what's this you mention in the PR about caching
> __optimize__((3))?  You also mention I shouldn't compare against
> this_target_optabs, but default_target_optabs.  But what if
> this_target_optabs has changed?  (See patch).

The reason for that is that this_target_optabs could at that point be
simply whatever optabs used the last parsed function.
this_target_optabs changes only either because of optimize attribute
(not sure if MIPS as the only switchable target? supports that), or
because of mips_set_mips16_mode.  I think invoke_set_current_function_hook
invokes the target hook after the code you've changed, so I'd say it should
work fine even on MIPS.  CCing Richard for that anyway.
> 
> Tested on x86-64 Linux.  It would be nice if someone with access to
> a SWITCHABLE_TARGET platform (mips) could also test this.

A few nits below.  I'm not sure about the behavior of multiple optimize
attributes either, let's try it and see how it is handled right now
(ignoring optabs).

> @@ -6184,6 +6184,41 @@ init_optabs (void)
>    targetm.init_libfuncs ();
>  }
>  
> +/* Recompute the optabs.  If they have changed, save the new set of
> +   optabs in the optimization node OPTNODE.  */
> +
> +void
> +save_optabs_if_changed (tree optnode)
> +{
> +  struct target_optabs *save_target_optabs = this_target_optabs;
> +  struct target_optabs *tmp_target_optabs = XNEW (struct target_optabs);
> +
> +  /* Generate a new set of optabs into tmp_target_optabs.  */
> +  memset (tmp_target_optabs, 0, sizeof (struct target_optabs));

I think you should just use XCNEW and drop the memset.

> +  this_target_optabs = tmp_target_optabs;
> +  init_all_optabs ();
> +  this_target_optabs = save_target_optabs;
> +
> +  /* If the optabs changed, record it in the node.  */
> +  if (memcmp (tmp_target_optabs, this_target_optabs,
> +	      sizeof (struct target_optabs)))
> +    {
> +      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
> +	 multiple ((optimize)) attributes for the same function.  Is
> +	 this even valid?  For now, just clobber the existing entry
> +	 with the new optabs.  */
> +      if (TREE_OPTIMIZATION_OPTABS (optnode))
> +	free (TREE_OPTIMIZATION_OPTABS (optnode));
> +
> +      TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs;
> +    }
> +  else
> +    {
> +      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
> +      free (tmp_target_optabs);

Shouldn't this (and above) be XDELETE to match the allocation style?

	Jakub

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]


>> Jakub, what's this you mention in the PR about caching
>> __optimize__((3))?  You also mention I shouldn't compare against
>> this_target_optabs, but default_target_optabs.  But what if
>> this_target_optabs has changed?  (See patch).
>
> The reason for that is that this_target_optabs could at that point be
> simply whatever optabs used the last parsed function.
> this_target_optabs changes only either because of optimize attribute
> (not sure if MIPS as the only switchable target? supports that), or
> because of mips_set_mips16_mode.  I think invoke_set_current_function_hook
> invokes the target hook after the code you've changed, so I'd say it should
> work fine even on MIPS.  CCing Richard for that anyway.

Ok, fixed.

> I think you should just use XCNEW and drop the memset.

Perfect.  Done.

> Shouldn't this (and above) be XDELETE to match the allocation style?

Absolutely.  Done.

OK for trunk?


[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 5359 bytes --]

commit ee1f2aebe23fe0d5cecfdfde9822ef681bf6ef0c
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Feb 11 15:51:24 2013 -0600

    	PR target/52555
    	* tree.h (struct tree_optimization_option): New field
    	target_optabs.
    	(TREE_OPTIMIZATION_OPTABS): New.
    	(save_optabs_if_changed): Protoize.
    	* optabs.h: Always declare this_target_optabs.
    	* optabs.c (save_optabs_if_changed): New.
    	Always declare this_target_optabs.
    	* function.c (invoke_set_current_function_hook): Set
    	this_target_optabs if there is one in the optimization node.
    c-family/
    	* c-common.c (handle_optimize_attribute): Call
    	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..3711e69 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node));
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..f37e91f 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4397,6 +4397,13 @@ invoke_set_current_function_hook (tree fndecl)
 	{
 	  optimization_current_node = opts;
 	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
+
+	  /* Change optabs if needed.  */
+	  if (TREE_OPTIMIZATION_OPTABS (opts))
+	    this_target_optabs
+	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
+	  else
+	    this_target_optabs = &default_target_optabs;
 	}
 
       targetm.set_current_function (fndecl);
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..11153aa 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,8 +44,8 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
-#if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
+#if SWITCHABLE_TARGET
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
 #endif
 
@@ -6207,6 +6207,40 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs.  If they have changed, save the new set of
+   optabs in the optimization node OPTNODE.  */
+
+void
+save_optabs_if_changed (tree optnode)
+{
+  struct target_optabs *save_target_optabs = this_target_optabs;
+  struct target_optabs *tmp_target_optabs = XCNEW (struct target_optabs);
+
+  /* Generate a new set of optabs into tmp_target_optabs.  */
+  this_target_optabs = tmp_target_optabs;
+  init_all_optabs ();
+  this_target_optabs = save_target_optabs;
+
+  /* If the optabs changed, record it in the node.  */
+  if (memcmp (tmp_target_optabs, &default_target_optabs,
+	      sizeof (struct target_optabs)))
+    {
+      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
+	 multiple ((optimize)) attributes for the same function.  Is
+	 this even valid?  For now, just clobber the existing entry
+	 with the new optabs.  */
+      if (TREE_OPTIMIZATION_OPTABS (optnode))
+	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));
+
+      TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      XDELETE (tmp_target_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..2e8b6ec 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,11 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
-#if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
-#else
-#define this_target_optabs (&default_target_optabs)
-#endif
 \f
 /* Define functions given in optabs.c.  */
 
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..eddbca8 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  void *GTY ((skip)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Tue, Feb 12, 2013 at 09:58:38AM -0600, Aldy Hernandez wrote:
> OK for trunk?

I'd still prefer Richard to chime in, I'm really not familiar enough
with MIPS switchable target stuff.

> +/* Recompute the optabs.  If they have changed, save the new set of
> +   optabs in the optimization node OPTNODE.  */
> +
> +void
> +save_optabs_if_changed (tree optnode)
> +{
> +  struct target_optabs *save_target_optabs = this_target_optabs;
> +  struct target_optabs *tmp_target_optabs = XCNEW (struct target_optabs);
> +
> +  /* Generate a new set of optabs into tmp_target_optabs.  */
> +  this_target_optabs = tmp_target_optabs;
> +  init_all_optabs ();
> +  this_target_optabs = save_target_optabs;
> +
> +  /* If the optabs changed, record it in the node.  */
> +  if (memcmp (tmp_target_optabs, &default_target_optabs,
> +	      sizeof (struct target_optabs)))
> +    {
> +      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
> +	 multiple ((optimize)) attributes for the same function.  Is
> +	 this even valid?  For now, just clobber the existing entry
> +	 with the new optabs.  */
> +      if (TREE_OPTIMIZATION_OPTABS (optnode))
> +	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));

I wonder if this necessarily won't mean that if TREE_OPTIMIZATION_OPTABS
is non-NULL, then memcmp (tmp_target_optabs, TREE_OPTIMIZATION_OPTABS
(optnode), sizeof (*tmp_target_optabs)) == 0.  Because, optimization nodes
are only shared if they contain the same set of all options.

Multiple optimize attributes seems to be valid, see what
handle_optimize_attribute says on them, but they together either create
a fresh optimization node which certainly won't have
TREE_OPTIMIZATION_OPTABS set, or they return one older, but that should be
already initialized to the same thing.
So perhaps start the function with
  if (TREE_OPTIMIZATION_OPTABS (optnode))
    return;
?  I.e., if you have multiple functions with the same optimization node,
there is no need to do init_all_optabs again and again.
Perhaps we want to have a flag whether TREE_OPTIMIZATION_OPTABS has been
computed already, and don't call save_optabs_if_changed if already set,
or add some magic value for TREE_OPTIMIZATION_OPTABS, where e.g. NULL
would mean not computed yet, the special magic value would mean the default
and other values the special optabs?  Perhaps the magic value could be
just &default_target_optabs...

	Jakub

Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford]

Jakub Jelinek <jakub@redhat.com> writes:
> On Mon, Feb 11, 2013 at 06:15:05PM -0600, Aldy Hernandez wrote:
>> How does this look?
>
> Looks good to me.
>
>> Jakub, what's this you mention in the PR about caching
>> __optimize__((3))?  You also mention I shouldn't compare against
>> this_target_optabs, but default_target_optabs.  But what if
>> this_target_optabs has changed?  (See patch).
>
> The reason for that is that this_target_optabs could at that point be
> simply whatever optabs used the last parsed function.
> this_target_optabs changes only either because of optimize attribute
> (not sure if MIPS as the only switchable target? supports that), or
> because of mips_set_mips16_mode.  I think invoke_set_current_function_hook
> invokes the target hook after the code you've changed, so I'd say it should
> work fine even on MIPS.  CCing Richard for that anyway.

The target hook won't do anything for consecutive functions that
have the same mode though.  It expects this_target_optabs (and other
this_target_* stuff) to stay the same.

Rather than:

	  /* Change optabs if needed.  */
	  if (TREE_OPTIMIZATION_OPTABS (opts))
	    this_target_optabs
	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
	  else
	    this_target_optabs = &default_target_optabs;

I think it'd be better to have:

	  /* Change optabs if needed.  */
	  if (TREE_OPTIMIZATION_OPTABS (opts))
	    this_fn_optabs
	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
	  else
	    this_fn_optabs = this_target_optabs;

with genopinit.c updated to use this_fn_optabs instead of this_target_optabs.

Richard

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]


> Rather than:
>
> 	  /* Change optabs if needed.  */
> 	  if (TREE_OPTIMIZATION_OPTABS (opts))
> 	    this_target_optabs
> 	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
> 	  else
> 	    this_target_optabs = &default_target_optabs;
>
> I think it'd be better to have:
>
> 	  /* Change optabs if needed.  */
> 	  if (TREE_OPTIMIZATION_OPTABS (opts))
> 	    this_fn_optabs
> 	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
> 	  else
> 	    this_fn_optabs = this_target_optabs;
>
> with genopinit.c updated to use this_fn_optabs instead of this_target_optabs.

Hmmm, ok.

I also added a default case setting this_fn_optabs = 
&default_target_optabs when the optimizations haven't changed.  I can 
remove this if redundant.

Jakub also recommended bailing if TREE_OPTIMIZATION_OPTABS already set, 
thus avoiding recomputing init_all_optabs() in save_optabs_if_changed:

+ if (TREE_OPTIMIZATION_OPTABS (optnode))
+    return;

Is this still part of the plan?

How is this revision?


[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 6192 bytes --]

commit 48c1f4d243f81ca975b2aae34773b91ef6cbd8ca
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Feb 11 15:51:24 2013 -0600

    	PR target/52555
    	* genopinit.c (main): Use this_fn_optabs in generated
    	init_all_optabs.
    	* tree.h (struct tree_optimization_option): New field
    	target_optabs.
    	(TREE_OPTIMIZATION_OPTABS): New.
    	(save_optabs_if_changed): Protoize.
    	* optabs.h: Always declare this_target_optabs.
    	Declare this_fn_optabs.
    	* optabs.c (save_optabs_if_changed): New.
    	Always declare this_target_optabs.
    	Declare this_fn_optabs.
    	* function.c (invoke_set_current_function_hook): Set
    	this_fn_optabs if there is one in the optimization node.
    c-family/
    	* c-common.c (handle_optimize_attribute): Call
    	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..3711e69 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node));
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..b177a98 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4397,7 +4397,16 @@ invoke_set_current_function_hook (tree fndecl)
 	{
 	  optimization_current_node = opts;
 	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
+
+	  /* Change optabs if needed.  */
+	  if (TREE_OPTIMIZATION_OPTABS (opts))
+	    this_fn_optabs
+	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
+	  else
+	    this_fn_optabs = this_target_optabs;
 	}
+      else
+	this_fn_optabs = &default_target_optabs;
 
       targetm.set_current_function (fndecl);
     }
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 1bb2f77..2a4b3e2 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -423,7 +423,7 @@ main (int argc, char **argv)
   fprintf (s_file, "};\n\n");
 
   fprintf (s_file, "void\ninit_all_optabs (void)\n{\n");
-  fprintf (s_file, "  bool *ena = this_target_optabs->pat_enable;\n");
+  fprintf (s_file, "  bool *ena = this_fn_optabs->pat_enable;\n");
   for (i = 0; patterns.iterate (i, &p); ++i)
     fprintf (s_file, "  ena[%u] = HAVE_%s;\n", i, p->name);
   fprintf (s_file, "}\n\n");
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..e40bb5f 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,8 +44,9 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
-#if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
+#if SWITCHABLE_TARGET
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
 #endif
 
@@ -6207,6 +6208,40 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs.  If they have changed, save the new set of
+   optabs in the optimization node OPTNODE.  */
+
+void
+save_optabs_if_changed (tree optnode)
+{
+  struct target_optabs *save_target_optabs = this_target_optabs;
+  struct target_optabs *tmp_target_optabs = XCNEW (struct target_optabs);
+
+  /* Generate a new set of optabs into tmp_target_optabs.  */
+  this_target_optabs = tmp_target_optabs;
+  init_all_optabs ();
+  this_target_optabs = save_target_optabs;
+
+  /* If the optabs changed, record it in the node.  */
+  if (memcmp (tmp_target_optabs, &default_target_optabs,
+	      sizeof (struct target_optabs)))
+    {
+      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
+	 multiple ((optimize)) attributes for the same function.  Is
+	 this even valid?  For now, just clobber the existing entry
+	 with the new optabs.  */
+      if (TREE_OPTIMIZATION_OPTABS (optnode))
+	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));
+
+      TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      XDELETE (tmp_target_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..49908d4 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,11 +76,8 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
-#if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
-#else
-#define this_target_optabs (&default_target_optabs)
-#endif
+extern struct target_optabs *this_fn_optabs;
 \f
 /* Define functions given in optabs.c.  */
 
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..eddbca8 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  void *GTY ((skip)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Aldy Hernandez <aldyh@redhat.com> writes:
>>> Rather than:
>>>
>>> 	  /* Change optabs if needed.  */
>>> 	  if (TREE_OPTIMIZATION_OPTABS (opts))
>>> 	    this_target_optabs
>>> 	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
>>> 	  else
>>> 	    this_target_optabs = &default_target_optabs;
>>>
>>> I think it'd be better to have:
>>>
>>> 	  /* Change optabs if needed.  */
>>> 	  if (TREE_OPTIMIZATION_OPTABS (opts))
>>> 	    this_fn_optabs
>>> 	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
>>> 	  else
>>> 	    this_fn_optabs = this_target_optabs;
>>>
>>> with genopinit.c updated to use this_fn_optabs instead of this_target_optabs.
>>
>> Hmmm, ok.
>>
>> I also added a default case setting this_fn_optabs = 
>> &default_target_optabs when the optimizations haven't changed.  I can 
>> remove this if redundant.
>
> No, sounds like a good plan, but I think it should be this_target_optabs
> rather than &default_target_optabs.

Gah, just realised after sending that it would be better to have:

static void
invoke_set_current_function_hook (tree fndecl)
{
  this_fn_optabs = this_target_optabs;
  if (!in_dummy_function)
    {
      tree opts = ((fndecl)
		   ? DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl)
		   : optimization_default_node);

      if (!opts)
	opts = optimization_default_node;

      /* Change optimization options if needed.  */
      if (optimization_current_node != opts)
	{
	  optimization_current_node = opts;
	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));

	  /* Change optabs if needed.  */
	  if (TREE_OPTIMIZATION_OPTABS (opts))
	    this_fn_optabs
	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
	}

      targetm.set_current_function (fndecl);
    }
}

Richard

Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford]

Aldy Hernandez <aldyh@redhat.com> writes:
>> Rather than:
>>
>> 	  /* Change optabs if needed.  */
>> 	  if (TREE_OPTIMIZATION_OPTABS (opts))
>> 	    this_target_optabs
>> 	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
>> 	  else
>> 	    this_target_optabs = &default_target_optabs;
>>
>> I think it'd be better to have:
>>
>> 	  /* Change optabs if needed.  */
>> 	  if (TREE_OPTIMIZATION_OPTABS (opts))
>> 	    this_fn_optabs
>> 	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
>> 	  else
>> 	    this_fn_optabs = this_target_optabs;
>>
>> with genopinit.c updated to use this_fn_optabs instead of this_target_optabs.
>
> Hmmm, ok.
>
> I also added a default case setting this_fn_optabs = 
> &default_target_optabs when the optimizations haven't changed.  I can 
> remove this if redundant.

No, sounds like a good plan, but I think it should be this_target_optabs
rather than &default_target_optabs.  Also:

@@ -76,11 +76,8 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
-#if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
-#else
-#define this_target_optabs (&default_target_optabs)
-#endif

This shouldn't be needed now, and:

@@ -44,8 +44,9 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
-#if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
+#if SWITCHABLE_TARGET
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
 #endif

I think this should be:

 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
 #if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
 #endif

Looks good to me otherwise as far as switchable targets go.

Richard

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

Richard.

I made all the changes you suggested.

I also changed other instances of s/this_target_optabs/this_fn_optabs/ 
which I forgot in the previous iteration.  And I also changed 
save_optabs_if_changed() to use this_fn_optabs, since init_all_optabs() 
will generate the optabs into this_fn_optabs.

Tested on x86-64 Linux.

OK?

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 6694 bytes --]

+	PR target/52555
+	* genopinit.c (main): Use this_fn_optabs in generated
+	init_all_optabs, raw_optab_handler, and swap_optab_enable.
+	* tree.h (struct tree_optimization_option): New field
+	target_optabs.
+	(TREE_OPTIMIZATION_OPTABS): New.
+	(save_optabs_if_changed): Protoize.
+	* optabs.h: Declare this_fn_optabs.
+	* optabs.c (save_optabs_if_changed): New.
+	Declare this_fn_optabs.
+	* function.c (invoke_set_current_function_hook): Set
+	this_fn_optabs if there is one in the optimization node.
c-family/
+	PR target/52555
+	* c-common.c (handle_optimize_attribute): Call
+	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..3711e69 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node));
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..688242a 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4383,6 +4383,7 @@ static bool in_dummy_function;
 static void
 invoke_set_current_function_hook (tree fndecl)
 {
+  this_fn_optabs = this_target_optabs;
   if (!in_dummy_function)
     {
       tree opts = ((fndecl)
@@ -4397,6 +4398,11 @@ invoke_set_current_function_hook (tree fndecl)
 	{
 	  optimization_current_node = opts;
 	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
+
+	  /* Change optabs if needed.  */
+	  if (TREE_OPTIMIZATION_OPTABS (opts))
+	    this_fn_optabs
+	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
 	}
 
       targetm.set_current_function (fndecl);
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 1bb2f77..13ebdc5 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -423,7 +423,7 @@ main (int argc, char **argv)
   fprintf (s_file, "};\n\n");
 
   fprintf (s_file, "void\ninit_all_optabs (void)\n{\n");
-  fprintf (s_file, "  bool *ena = this_target_optabs->pat_enable;\n");
+  fprintf (s_file, "  bool *ena = this_fn_optabs->pat_enable;\n");
   for (i = 0; patterns.iterate (i, &p); ++i)
     fprintf (s_file, "  ena[%u] = HAVE_%s;\n", i, p->name);
   fprintf (s_file, "}\n\n");
@@ -456,7 +456,7 @@ main (int argc, char **argv)
 	   "raw_optab_handler (unsigned scode)\n"
 	   "{\n"
 	   "  int i = lookup_handler (scode);\n"
-	   "  return (i >= 0 && this_target_optabs->pat_enable[i]\n"
+	   "  return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
 	   "          ? pats[i].icode : CODE_FOR_nothing);\n"
 	   "}\n\n");
 
@@ -468,8 +468,8 @@ main (int argc, char **argv)
 	   "  int i = lookup_handler (scode);\n"
 	   "  if (i >= 0)\n"
 	   "    {\n"
-	   "      bool ret = this_target_optabs->pat_enable[i];\n"
-	   "      this_target_optabs->pat_enable[i] = set;\n"
+	   "      bool ret = this_fn_optabs->pat_enable[i];\n"
+	   "      this_fn_optabs->pat_enable[i] = set;\n"
 	   "      return ret;\n"
 	   "    }\n"
 	   "  else\n"
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..145930f 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
 #if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
@@ -6207,6 +6208,40 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs.  If they have changed, save the new set of
+   optabs in the optimization node OPTNODE.  */
+
+void
+save_optabs_if_changed (tree optnode)
+{
+  struct target_optabs *save_fn_optabs = this_fn_optabs;
+  struct target_optabs *tmp_target_optabs = XCNEW (struct target_optabs);
+
+  /* Generate a new set of optabs into tmp_target_optabs.  */
+  this_fn_optabs = tmp_target_optabs;
+  init_all_optabs ();
+  this_fn_optabs = save_fn_optabs;
+
+  /* If the optabs changed, record it in the node.  */
+  if (memcmp (tmp_target_optabs, &default_target_optabs,
+	      sizeof (struct target_optabs)))
+    {
+      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
+	 multiple ((optimize)) attributes for the same function.  Is
+	 this even valid?  For now, just clobber the existing entry
+	 with the new optabs.  */
+      if (TREE_OPTIMIZATION_OPTABS (optnode))
+	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));
+
+      TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      XDELETE (tmp_target_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..4de4409 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,6 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
+extern struct target_optabs *this_fn_optabs;
 #if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
 #else
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..eddbca8 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  void *GTY ((skip)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford]

Aldy Hernandez <aldyh@redhat.com> writes:
> Richard.
>
> I made all the changes you suggested.
>
> I also changed other instances of s/this_target_optabs/this_fn_optabs/ 
> which I forgot in the previous iteration.  And I also changed 
> save_optabs_if_changed() to use this_fn_optabs, since init_all_optabs() 
> will generate the optabs into this_fn_optabs.

Sorry, just noticed:

> +  /* If the optabs changed, record it in the node.  */
> +  if (memcmp (tmp_target_optabs, &default_target_optabs,
> +	      sizeof (struct target_optabs)))

This should be this_target_optabs rather than &default_target_optabs.
Nothing but target code and initialisers should use &default_target_optabs
directly.

I don't think that needs a retest though.  It only makes a difference
on MIPS.

Looks good to me otherwise.  If there's any fallout on MIPS (hopefully not)
I'll fix it up after the commit.

Thanks,
Richard

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]


> Sorry, just noticed:
>
>> +  /* If the optabs changed, record it in the node.  */
>> +  if (memcmp (tmp_target_optabs, &default_target_optabs,
>> +	      sizeof (struct target_optabs)))
>
> This should be this_target_optabs rather than &default_target_optabs.
> Nothing but target code and initialisers should use &default_target_optabs
> directly.

Fixed.

>
> I don't think that needs a retest though.  It only makes a difference
> on MIPS.

I verified that the testcase is still fixed by this patch (just in 
case).  Thanks so much for the review.

Final patch attached.

Jakub, OK?


[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 6690 bytes --]

+	PR target/52555
+	* genopinit.c (main): Use this_fn_optabs in generated
+	init_all_optabs, raw_optab_handler, and swap_optab_enable.
+	* tree.h (struct tree_optimization_option): New field
+	target_optabs.
+	(TREE_OPTIMIZATION_OPTABS): New.
+	(save_optabs_if_changed): Protoize.
+	* optabs.h: Declare this_fn_optabs.
+	* optabs.c (save_optabs_if_changed): New.
+	Declare this_fn_optabs.
+	* function.c (invoke_set_current_function_hook): Set
+	this_fn_optabs if there is one in the optimization node.
c-family/
+	PR target/52555
+	* c-common.c (handle_optimize_attribute): Call
+	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..3711e69 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node));
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..688242a 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4383,6 +4383,7 @@ static bool in_dummy_function;
 static void
 invoke_set_current_function_hook (tree fndecl)
 {
+  this_fn_optabs = this_target_optabs;
   if (!in_dummy_function)
     {
       tree opts = ((fndecl)
@@ -4397,6 +4398,11 @@ invoke_set_current_function_hook (tree fndecl)
 	{
 	  optimization_current_node = opts;
 	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
+
+	  /* Change optabs if needed.  */
+	  if (TREE_OPTIMIZATION_OPTABS (opts))
+	    this_fn_optabs
+	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
 	}
 
       targetm.set_current_function (fndecl);
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 1bb2f77..13ebdc5 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -423,7 +423,7 @@ main (int argc, char **argv)
   fprintf (s_file, "};\n\n");
 
   fprintf (s_file, "void\ninit_all_optabs (void)\n{\n");
-  fprintf (s_file, "  bool *ena = this_target_optabs->pat_enable;\n");
+  fprintf (s_file, "  bool *ena = this_fn_optabs->pat_enable;\n");
   for (i = 0; patterns.iterate (i, &p); ++i)
     fprintf (s_file, "  ena[%u] = HAVE_%s;\n", i, p->name);
   fprintf (s_file, "}\n\n");
@@ -456,7 +456,7 @@ main (int argc, char **argv)
 	   "raw_optab_handler (unsigned scode)\n"
 	   "{\n"
 	   "  int i = lookup_handler (scode);\n"
-	   "  return (i >= 0 && this_target_optabs->pat_enable[i]\n"
+	   "  return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
 	   "          ? pats[i].icode : CODE_FOR_nothing);\n"
 	   "}\n\n");
 
@@ -468,8 +468,8 @@ main (int argc, char **argv)
 	   "  int i = lookup_handler (scode);\n"
 	   "  if (i >= 0)\n"
 	   "    {\n"
-	   "      bool ret = this_target_optabs->pat_enable[i];\n"
-	   "      this_target_optabs->pat_enable[i] = set;\n"
+	   "      bool ret = this_fn_optabs->pat_enable[i];\n"
+	   "      this_fn_optabs->pat_enable[i] = set;\n"
 	   "      return ret;\n"
 	   "    }\n"
 	   "  else\n"
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..00a13da 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
 #if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
@@ -6207,6 +6208,40 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs.  If they have changed, save the new set of
+   optabs in the optimization node OPTNODE.  */
+
+void
+save_optabs_if_changed (tree optnode)
+{
+  struct target_optabs *save_fn_optabs = this_fn_optabs;
+  struct target_optabs *tmp_target_optabs = XCNEW (struct target_optabs);
+
+  /* Generate a new set of optabs into tmp_target_optabs.  */
+  this_fn_optabs = tmp_target_optabs;
+  init_all_optabs ();
+  this_fn_optabs = save_fn_optabs;
+
+  /* If the optabs changed, record it in the node.  */
+  if (memcmp (tmp_target_optabs, this_target_optabs,
+	      sizeof (struct target_optabs)))
+    {
+      /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates
+	 multiple ((optimize)) attributes for the same function.  Is
+	 this even valid?  For now, just clobber the existing entry
+	 with the new optabs.  */
+      if (TREE_OPTIMIZATION_OPTABS (optnode))
+	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));
+
+      TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      XDELETE (tmp_target_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..4de4409 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,6 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
+extern struct target_optabs *this_fn_optabs;
 #if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
 #else
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..eddbca8 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  void *GTY ((skip)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Wed, Feb 13, 2013 at 12:07:52PM -0600, Aldy Hernandez wrote:
> >Sorry, just noticed:
> >
> >>+  /* If the optabs changed, record it in the node.  */
> >>+  if (memcmp (tmp_target_optabs, &default_target_optabs,
> >>+	      sizeof (struct target_optabs)))
> >
> >This should be this_target_optabs rather than &default_target_optabs.
> >Nothing but target code and initialisers should use &default_target_optabs
> >directly.
> 
> Fixed.
> 
> >
> >I don't think that needs a retest though.  It only makes a difference
> >on MIPS.
> 
> I verified that the testcase is still fixed by this patch (just in
> case).  Thanks so much for the review.
> 
> Final patch attached.

Actually, thinking more about SWITCHABLE_TARGETS, I don't think this works
at all on those targets.  this_target_optab is some random optab from
whatever has been left there by previous function, it can be either the
same, or different target (mips vs. mips16?).  So, for SWITCHABLE_TARGETS,
I'm afraid you really can't save the optabs in the optimization node
(because, there could be different optabs for say all of
-Ofast + mips, -Ofast + mips16, -O2 + mips and -O2 + mips16 combinations),
but you probably can't save it even from within the target hook, because
some non-default optimization node might be in effect.
So, for SWITCHABLE_TARGETS, I think you can only save it in
cfun->this_fn_optab or similar, and perhaps cache the default for mips16
only when you know the default optimization node is in effect (or force it
into effect temporarily).
For !SWITCHABLE_TARGETS, you can easily remember it in the optimization
nodes and just copy the pointer over to cfun->this_fn_optab.
And unfortunately it seems optimize attribute is supported everywhere, just
the target attribute is not.

But, as soon as it is reachable from cfun->this_fn_optab, we might need to
GC allocate it, while optimization nodes are never freed, cfun is freed I
think.

	Jakub

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]


> Actually, thinking more about SWITCHABLE_TARGETS, I don't think this works
> at all on those targets.  this_target_optab is some random optab from
> whatever has been left there by previous function, it can be either the

Eeech... this significantly complicates things.

After some further interaction off-list... attached is a patch using 
cfun for SWITCHABLE_TARGETs, caching the default optimization node, and 
using GC for the pointers.

Jakub, I got rid of the suggested ifdef's because I'd prefer the 
compiler to compile/stress the non-common SWITCHABLE_TARGET case.  All 
targets define SWITCHABLE_TARGET to 0/1, so there should never be an 
undefined symbol.  If you'd prefer the ifdef, I can grudgingly change it 
back :).

[Richard, unfortunately there are some mips specific changes.]

How does this look y'all?


[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 9980 bytes --]

+2013-02-15  Aldy Hernandez  <aldyh@redhat.com>
+	    Jakub Jelinek  <jakub@redhat.com>
+
+	PR target/52555
+	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
+	(swap_optab_enable): Same.
+	(init_all_optabs): Use argument instead of global.
+	* tree.h (struct tree_optimization_option): New field
+	target_optabs.
+	* expr.h (init_all_optabs): Add argument to prototype.
+	(TREE_OPTIMIZATION_OPTABS): New.
+	(save_optabs_if_changed): Protoize.
+	* optabs.h: Declare this_fn_optabs.
+	* optabs.c (save_optabs_if_changed): New.
+	Declare this_fn_optabs.
+	(init_optabs): Add argument to init_all_optabs() call.
+	* function.c (invoke_set_current_function_hook): Handle per
+	function optabs.
+	* function.h (struct function): New field optabs.
+	* config/mips/mips.c (mips_set_mips16_mode): Handle when
+	optimization_current_node has changed.
c/family
+	PR target/52555
+	* c-common.c (handle_optimize_attribute): Call
+	save_optabs_if_changed.
+

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..a1d47a6 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (*node);
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index b203cdd..5e98485 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16313,7 +16313,26 @@ mips_set_mips16_mode (int mips16_p)
   if (mips16_p)
     {
       if (!mips16_globals)
-	mips16_globals = save_target_globals ();
+	{
+	  if (optimization_current_node != optimization_default_node)
+	    {
+	      tree opts = optimization_current_node;
+	      /* Temporarily switch to the default optimization node,
+		 so that *this_target_optabs is set to the default,
+		 not reflecting whatever the first mips16 function
+		 uses for the optimize attribute.  */
+	      optimization_current_node = optimization_default_node;
+	      cl_optimization_restore
+		(&global_options,
+		 TREE_OPTIMIZATION (optimization_default_node));
+	      mips16_globals = save_target_globals ();
+	      optimization_current_node = opts;
+	      cl_optimization_restore (&global_options,
+				       TREE_OPTIMIZATION (opts));
+	    }
+	  else
+	    mips16_globals = save_target_globals ();	  
+	}
       else
 	restore_target_globals (mips16_globals);
     }
diff --git a/gcc/expr.h b/gcc/expr.h
index f5063b4..15fcb47 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -718,7 +718,7 @@ extern bool split_comparison (enum rtx_code, enum machine_mode,
 /* Call this once to initialize the contents of the optabs
    appropriately for the current target machine.  */
 extern void init_optabs (void);
-extern void init_all_optabs (void);
+extern void init_all_optabs (struct target_optabs *);
 
 /* Call this to initialize an optab function entry.  */
 extern rtx init_one_libfunc (const char *);
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..c5eea2e 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4400,6 +4400,31 @@ invoke_set_current_function_hook (tree fndecl)
 	}
 
       targetm.set_current_function (fndecl);
+
+      if (opts == optimization_default_node)
+	this_fn_optabs = this_target_optabs;
+      else
+	{
+	  struct function *fn = DECL_STRUCT_FUNCTION (fndecl);
+	  if (fn->optabs == NULL)
+	    {
+	      if (!SWITCHABLE_TARGET)
+		fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
+	      else
+		{
+		  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;
+	}
     }
 }
 
diff --git a/gcc/function.h b/gcc/function.h
index 89d71e5..53e28b7 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -580,6 +580,9 @@ 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
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 1bb2f77..fb80717 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -422,8 +422,8 @@ main (int argc, char **argv)
     fprintf (s_file, "  { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
   fprintf (s_file, "};\n\n");
 
-  fprintf (s_file, "void\ninit_all_optabs (void)\n{\n");
-  fprintf (s_file, "  bool *ena = this_target_optabs->pat_enable;\n");
+  fprintf (s_file, "void\ninit_all_optabs (struct target_optabs *optabs)\n{\n");
+  fprintf (s_file, "  bool *ena = optabs->pat_enable;\n");
   for (i = 0; patterns.iterate (i, &p); ++i)
     fprintf (s_file, "  ena[%u] = HAVE_%s;\n", i, p->name);
   fprintf (s_file, "}\n\n");
@@ -456,7 +456,7 @@ main (int argc, char **argv)
 	   "raw_optab_handler (unsigned scode)\n"
 	   "{\n"
 	   "  int i = lookup_handler (scode);\n"
-	   "  return (i >= 0 && this_target_optabs->pat_enable[i]\n"
+	   "  return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
 	   "          ? pats[i].icode : CODE_FOR_nothing);\n"
 	   "}\n\n");
 
@@ -468,8 +468,8 @@ main (int argc, char **argv)
 	   "  int i = lookup_handler (scode);\n"
 	   "  if (i >= 0)\n"
 	   "    {\n"
-	   "      bool ret = this_target_optabs->pat_enable[i];\n"
-	   "      this_target_optabs->pat_enable[i] = set;\n"
+	   "      bool ret = this_fn_optabs->pat_enable[i];\n"
+	   "      this_fn_optabs->pat_enable[i] = set;\n"
 	   "      return ret;\n"
 	   "    }\n"
 	   "  else\n"
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..dd621c3 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
 #if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
@@ -6150,7 +6151,7 @@ init_optabs (void)
     libfunc_hash = htab_create_ggc (10, hash_libfunc, eq_libfunc, NULL);
 
   /* Fill in the optabs with the insns we support.  */
-  init_all_optabs ();
+  init_all_optabs (this_fn_optabs);
 
   /* The ffs function operates on `int'.  Fall back on it if we do not
      have a libgcc2 function for that width.  */
@@ -6207,6 +6208,41 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs and save them if they have changed.  */
+
+void
+save_optabs_if_changed (tree fndecl)
+{
+  /* ?? 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);
+
+  struct target_optabs *tmp_optabs = (struct target_optabs *)
+    ggc_alloc_atomic (sizeof (struct target_optabs));
+  tree optnode = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl);
+
+  /* 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)))
+    {
+      /* ?? An existing optabs indicates multiple ((optimize))
+	 attributes for the same function.  Is this even valid?  For
+	 now, just clobber the existing entry with the new optabs.  */
+      if (TREE_OPTIMIZATION_OPTABS (optnode))
+	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));
+
+      TREE_OPTIMIZATION_OPTABS (optnode) = (unsigned char *) tmp_optabs;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      ggc_free (tmp_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..4de4409 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,6 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
+extern struct target_optabs *this_fn_optabs;
 #if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
 #else
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..740d438 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  unsigned char *GTY ((atomic)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Fri, Feb 15, 2013 at 11:23:01AM -0600, Aldy Hernandez wrote:
> +2013-02-15  Aldy Hernandez  <aldyh@redhat.com>
> +	    Jakub Jelinek  <jakub@redhat.com>
> +
> +	PR target/52555
> +	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
> +	(swap_optab_enable): Same.
> +	(init_all_optabs): Use argument instead of global.
> +	* tree.h (struct tree_optimization_option): New field
> +	target_optabs.
> +	* expr.h (init_all_optabs): Add argument to prototype.
> +	(TREE_OPTIMIZATION_OPTABS): New.
> +	(save_optabs_if_changed): Protoize.
> +	* optabs.h: Declare this_fn_optabs.
> +	* optabs.c (save_optabs_if_changed): New.
> +	Declare this_fn_optabs.
> +	(init_optabs): Add argument to init_all_optabs() call.
> +	* function.c (invoke_set_current_function_hook): Handle per
> +	function optabs.
> +	* function.h (struct function): New field optabs.
> +	* config/mips/mips.c (mips_set_mips16_mode): Handle when
> +	optimization_current_node has changed.
> c/family
> +	PR target/52555
> +	* c-common.c (handle_optimize_attribute): Call
> +	save_optabs_if_changed.

Looks good, just a few nits.  But please wait for Richard's feedback on it.

> +	      if (!SWITCHABLE_TARGET)
> +		fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
> +	      else
> +		{
> +		  if (this_target_optabs == &default_target_optabs)
> +		    fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

Just use
		if (!SWITCHABLE_TARGET
		    || 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);
> +		    }
> +		}

and reindent.

> +	    }
> +	  this_fn_optabs = fn->optabs ? (struct target_optabs *) fn->optabs
> +	    : this_target_optabs;

I'd prefer : here be below ? on the line above it.

> +      /* ?? An existing optabs indicates multiple ((optimize))
> +	 attributes for the same function.  Is this even valid?  For
> +	 now, just clobber the existing entry with the new optabs.  */
> +      if (TREE_OPTIMIZATION_OPTABS (optnode))
> +	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));

The comment is wrong,
void foo (void) __attribute__((optimize (2)));
void foo (void) __attribute__((optimize ("fast-math")));
void foo (void) __attribute__((optimize ("unroll-loops")));

void
foo (void)
{
}
is just fine, and for foo results in -O2 -ffast-math -funroll-loops
options being in effect.
Plus XDELETE on GC allocated memory is wrong.  Just remove the comment
and if and XDELETE lines.

	Jakub

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]


> Looks good, just a few nits.  But please wait for Richard's feedback on it.

Will do.

>> +	  this_fn_optabs = fn->optabs ? (struct target_optabs *) fn->optabs
>> +	    : this_target_optabs;
>
> I'd prefer : here be below ? on the line above it.

Blame emacs.  I use whatever indentation it decides to use.

>
>> +      /* ?? An existing optabs indicates multiple ((optimize))
>> +	 attributes for the same function.  Is this even valid?  For
>> +	 now, just clobber the existing entry with the new optabs.  */
>> +      if (TREE_OPTIMIZATION_OPTABS (optnode))
>> +	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));
>
> The comment is wrong,
> void foo (void) __attribute__((optimize (2)));
> void foo (void) __attribute__((optimize ("fast-math")));
> void foo (void) __attribute__((optimize ("unroll-loops")));

Oh, that's just sick.  Fair enough, I removed the comment altogether.

> Plus XDELETE on GC allocated memory is wrong.  Just remove the comment
> and if and XDELETE lines.

And this is just embarrassing...fixed.  Thanks.

Retesting the attached patch.

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 9725 bytes --]

+2013-02-15  Aldy Hernandez  <aldyh@redhat.com>
+	    Jakub Jelinek  <jakub@redhat.com>
+
+	PR target/52555
+	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
+	(swap_optab_enable): Same.
+	(init_all_optabs): Use argument instead of global.
+	* tree.h (struct tree_optimization_option): New field
+	target_optabs.
+	* expr.h (init_all_optabs): Add argument to prototype.
+	(TREE_OPTIMIZATION_OPTABS): New.
+	(save_optabs_if_changed): Protoize.
+	* optabs.h: Declare this_fn_optabs.
+	* optabs.c (save_optabs_if_changed): New.
+	Declare this_fn_optabs.
+	(init_optabs): Add argument to init_all_optabs() call.
+	* function.c (invoke_set_current_function_hook): Handle per
+	function optabs.
+	* function.h (struct function): New field optabs.
+	* config/mips/mips.c (mips_set_mips16_mode): Handle when
+	optimization_current_node has changed.
c/family
+	PR target/52555
+	* c-common.c (handle_optimize_attribute): Call
+	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..a1d47a6 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (*node);
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index b203cdd..5e98485 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16313,7 +16313,26 @@ mips_set_mips16_mode (int mips16_p)
   if (mips16_p)
     {
       if (!mips16_globals)
-	mips16_globals = save_target_globals ();
+	{
+	  if (optimization_current_node != optimization_default_node)
+	    {
+	      tree opts = optimization_current_node;
+	      /* Temporarily switch to the default optimization node,
+		 so that *this_target_optabs is set to the default,
+		 not reflecting whatever the first mips16 function
+		 uses for the optimize attribute.  */
+	      optimization_current_node = optimization_default_node;
+	      cl_optimization_restore
+		(&global_options,
+		 TREE_OPTIMIZATION (optimization_default_node));
+	      mips16_globals = save_target_globals ();
+	      optimization_current_node = opts;
+	      cl_optimization_restore (&global_options,
+				       TREE_OPTIMIZATION (opts));
+	    }
+	  else
+	    mips16_globals = save_target_globals ();	  
+	}
       else
 	restore_target_globals (mips16_globals);
     }
diff --git a/gcc/expr.h b/gcc/expr.h
index f5063b4..15fcb47 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -718,7 +718,7 @@ extern bool split_comparison (enum rtx_code, enum machine_mode,
 /* Call this once to initialize the contents of the optabs
    appropriately for the current target machine.  */
 extern void init_optabs (void);
-extern void init_all_optabs (void);
+extern void init_all_optabs (struct target_optabs *);
 
 /* Call this to initialize an optab function entry.  */
 extern rtx init_one_libfunc (const char *);
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..c87f62d 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4400,6 +4400,27 @@ invoke_set_current_function_hook (tree fndecl)
 	}
 
       targetm.set_current_function (fndecl);
+
+      if (opts == optimization_default_node)
+	this_fn_optabs = this_target_optabs;
+      else
+	{
+	  struct function *fn = DECL_STRUCT_FUNCTION (fndecl);
+	  if (fn->optabs == NULL)
+	    {
+	      if (!SWITCHABLE_TARGET
+		  || 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;
+	}
     }
 }
 
diff --git a/gcc/function.h b/gcc/function.h
index 89d71e5..53e28b7 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -580,6 +580,9 @@ 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
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 1bb2f77..fb80717 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -422,8 +422,8 @@ main (int argc, char **argv)
     fprintf (s_file, "  { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
   fprintf (s_file, "};\n\n");
 
-  fprintf (s_file, "void\ninit_all_optabs (void)\n{\n");
-  fprintf (s_file, "  bool *ena = this_target_optabs->pat_enable;\n");
+  fprintf (s_file, "void\ninit_all_optabs (struct target_optabs *optabs)\n{\n");
+  fprintf (s_file, "  bool *ena = optabs->pat_enable;\n");
   for (i = 0; patterns.iterate (i, &p); ++i)
     fprintf (s_file, "  ena[%u] = HAVE_%s;\n", i, p->name);
   fprintf (s_file, "}\n\n");
@@ -456,7 +456,7 @@ main (int argc, char **argv)
 	   "raw_optab_handler (unsigned scode)\n"
 	   "{\n"
 	   "  int i = lookup_handler (scode);\n"
-	   "  return (i >= 0 && this_target_optabs->pat_enable[i]\n"
+	   "  return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
 	   "          ? pats[i].icode : CODE_FOR_nothing);\n"
 	   "}\n\n");
 
@@ -468,8 +468,8 @@ main (int argc, char **argv)
 	   "  int i = lookup_handler (scode);\n"
 	   "  if (i >= 0)\n"
 	   "    {\n"
-	   "      bool ret = this_target_optabs->pat_enable[i];\n"
-	   "      this_target_optabs->pat_enable[i] = set;\n"
+	   "      bool ret = this_fn_optabs->pat_enable[i];\n"
+	   "      this_fn_optabs->pat_enable[i] = set;\n"
 	   "      return ret;\n"
 	   "    }\n"
 	   "  else\n"
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..c5778d1 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
 #if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
@@ -6150,7 +6151,7 @@ init_optabs (void)
     libfunc_hash = htab_create_ggc (10, hash_libfunc, eq_libfunc, NULL);
 
   /* Fill in the optabs with the insns we support.  */
-  init_all_optabs ();
+  init_all_optabs (this_fn_optabs);
 
   /* The ffs function operates on `int'.  Fall back on it if we do not
      have a libgcc2 function for that width.  */
@@ -6207,6 +6208,38 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs and save them if they have changed.  */
+
+void
+save_optabs_if_changed (tree fndecl)
+{
+  /* ?? 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);
+
+  struct target_optabs *tmp_optabs = (struct target_optabs *)
+    ggc_alloc_atomic (sizeof (struct target_optabs));
+  tree optnode = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl);
+
+  /* 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;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      ggc_free (tmp_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..4de4409 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,6 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
+extern struct target_optabs *this_fn_optabs;
 #if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
 #else
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..740d438 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  unsigned char *GTY ((atomic)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford]

Aldy Hernandez <aldyh@redhat.com> writes:
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index b203cdd..5e98485 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -16313,7 +16313,26 @@ mips_set_mips16_mode (int mips16_p)
>    if (mips16_p)
>      {
>        if (!mips16_globals)
> -	mips16_globals = save_target_globals ();
> +	{
> +	  if (optimization_current_node != optimization_default_node)
> +	    {
> +	      tree opts = optimization_current_node;
> +	      /* Temporarily switch to the default optimization node,
> +		 so that *this_target_optabs is set to the default,
> +		 not reflecting whatever the first mips16 function
> +		 uses for the optimize attribute.  */
> +	      optimization_current_node = optimization_default_node;
> +	      cl_optimization_restore
> +		(&global_options,
> +		 TREE_OPTIMIZATION (optimization_default_node));
> +	      mips16_globals = save_target_globals ();
> +	      optimization_current_node = opts;
> +	      cl_optimization_restore (&global_options,
> +				       TREE_OPTIMIZATION (opts));
> +	    }
> +	  else
> +	    mips16_globals = save_target_globals ();	  
> +	}

I think this should go in target-globals.c, maybe as
save_target_globals_default_opts.

> diff --git a/gcc/function.c b/gcc/function.c
> index 4ce2259..c5eea2e 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -4400,6 +4400,31 @@ invoke_set_current_function_hook (tree fndecl)
>  	}
>  
>        targetm.set_current_function (fndecl);
> +
> +      if (opts == optimization_default_node)
> +	this_fn_optabs = this_target_optabs;
> +      else
> +	{
> +	  struct function *fn = DECL_STRUCT_FUNCTION (fndecl);
> +	  if (fn->optabs == NULL)
> +	    {
> +	      if (!SWITCHABLE_TARGET)
> +		fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
> +	      else
> +		{
> +		  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);
> +		    }
> +		}

Following on from Jakub's:

		if (!SWITCHABLE_TARGET
		    || this_target_optabs == &default_target_optabs)
		  fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

I'd prefer just:

		if (this_target_optabs == &default_target_optabs)
		  fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

Looks good to me otherwise, thanks.

Sorry that SWITCHABLE_TARGETS has been so much hassle.  TBH, like Jakub
says in the PR, I was hoping things like the optimize attribute could
use the target_globals stuff too.  Today we just care about optabs,
but e.g. arm.c has:

  if (TARGET_THUMB1 && optimize_size)
    {
      /* When optimizing for size on Thumb-1, it's better not
        to use the HI regs, because of the overhead of
        stacking them.  */
      for (regno = FIRST_HI_REGNUM;
	   regno <= LAST_HI_REGNUM; ++regno)
	fixed_regs[regno] = call_used_regs[regno] = 1;
    }

which affects other cached global state like IRA tables.

The rtx_costs also often depend on optimize_size, and are cached in
various places like expmed.c.  E.g. for:

int
foo (int x, int y)
{
  return x * 10;
}

the -O2 version on MIPS32 is:

        sll     $2,$4,1
        sll     $4,$4,3
        j       $31
        addu    $2,$2,$4

and the -Os version is:

        li      $2,10
        j       $31
        mul     $2,$4,$2

But even if an optimize attribute is added:

int __attribute__((optimize(2)))
foo (int x, int y)
{
  return x * 10;
}

what you get depends on whether -Os or -O2 was passed on the command line.
The attribute doesn't affect things either way.  The same thing happens
on x86_64.

So in the end I think we'll end up trying solve the same problem that
the SWITCHABLE_TARGETS stuff was trying to solve, but this time for
__attribute__((optimize)).

Richard

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On 02/16/13 05:19, Richard Sandiford wrote:

> Looks good to me otherwise, thanks.

Implemented all your suggestions.

>
> Sorry that SWITCHABLE_TARGETS has been so much hassle.  TBH, like Jakub
> says in the PR, I was hoping things like the optimize attribute could
> use the target_globals stuff too.  Today we just care about optabs,
> but e.g. arm.c has:
>
>    if (TARGET_THUMB1 && optimize_size)
>      {
>        /* When optimizing for size on Thumb-1, it's better not
>          to use the HI regs, because of the overhead of
>          stacking them.  */
>        for (regno = FIRST_HI_REGNUM;
> 	   regno <= LAST_HI_REGNUM; ++regno)
> 	fixed_regs[regno] = call_used_regs[regno] = 1;
>      }
>
> which affects other cached global state like IRA tables.

...
...

> So in the end I think we'll end up trying solve the same problem that
> the SWITCHABLE_TARGETS stuff was trying to solve, but this time for
> __attribute__((optimize)).

Ughhh... perhaps this can be something for 4.9, reimplementing the 
optimize attribute with the target_globals infrastructure?

Regstrapping attached patch.

OK pending tests?

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 10649 bytes --]

	PR target/52555
	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
	(swap_optab_enable): Same.
	(init_all_optabs): Use argument instead of global.
	* tree.h (struct tree_optimization_option): New field
	target_optabs.
	* expr.h (init_all_optabs): Add argument to prototype.
	(TREE_OPTIMIZATION_OPTABS): New.
	(save_optabs_if_changed): Protoize.
	* optabs.h: Declare this_fn_optabs.
	* optabs.c (save_optabs_if_changed): New.
	Declare this_fn_optabs.
	(init_optabs): Add argument to init_all_optabs() call.
	* function.c (invoke_set_current_function_hook): Handle per
	function optabs.
	* function.h (struct function): New field optabs.
	* config/mips/mips.c (mips_set_mips16_mode): Handle when
	optimization_current_node has changed.
	* target-globals.h (save_target_globals_default_opts): Protoize.
	* target-globals.c (save_target_globals_default_opts): New.
c/family
	PR target/52555
	* c-common.c (handle_optimize_attribute): Call
	save_optabs_if_changed.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1e6afaa..a1d47a6 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args,
       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
 	= build_optimization_node ();
 
+      save_optabs_if_changed (*node);
+
       /* Restore current options.  */
       cl_optimization_restore (&global_options, &cur_opts);
     }
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index b203cdd..252e828 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -16313,7 +16313,7 @@ mips_set_mips16_mode (int mips16_p)
   if (mips16_p)
     {
       if (!mips16_globals)
-	mips16_globals = save_target_globals ();
+	mips16_globals = save_target_globals_default_opts ();
       else
 	restore_target_globals (mips16_globals);
     }
diff --git a/gcc/expr.h b/gcc/expr.h
index f5063b4..15fcb47 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -718,7 +718,7 @@ extern bool split_comparison (enum rtx_code, enum machine_mode,
 /* Call this once to initialize the contents of the optabs
    appropriately for the current target machine.  */
 extern void init_optabs (void);
-extern void init_all_optabs (void);
+extern void init_all_optabs (struct target_optabs *);
 
 /* Call this to initialize an optab function entry.  */
 extern rtx init_one_libfunc (const char *);
diff --git a/gcc/function.c b/gcc/function.c
index 4ce2259..1b41cf2 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4400,6 +4400,26 @@ invoke_set_current_function_hook (tree fndecl)
 	}
 
       targetm.set_current_function (fndecl);
+
+      if (opts == optimization_default_node)
+	this_fn_optabs = this_target_optabs;
+      else
+	{
+	  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;
+	}
     }
 }
 
diff --git a/gcc/function.h b/gcc/function.h
index 89d71e5..53e28b7 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -580,6 +580,9 @@ 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
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 1bb2f77..fb80717 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -422,8 +422,8 @@ main (int argc, char **argv)
     fprintf (s_file, "  { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
   fprintf (s_file, "};\n\n");
 
-  fprintf (s_file, "void\ninit_all_optabs (void)\n{\n");
-  fprintf (s_file, "  bool *ena = this_target_optabs->pat_enable;\n");
+  fprintf (s_file, "void\ninit_all_optabs (struct target_optabs *optabs)\n{\n");
+  fprintf (s_file, "  bool *ena = optabs->pat_enable;\n");
   for (i = 0; patterns.iterate (i, &p); ++i)
     fprintf (s_file, "  ena[%u] = HAVE_%s;\n", i, p->name);
   fprintf (s_file, "}\n\n");
@@ -456,7 +456,7 @@ main (int argc, char **argv)
 	   "raw_optab_handler (unsigned scode)\n"
 	   "{\n"
 	   "  int i = lookup_handler (scode);\n"
-	   "  return (i >= 0 && this_target_optabs->pat_enable[i]\n"
+	   "  return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
 	   "          ? pats[i].icode : CODE_FOR_nothing);\n"
 	   "}\n\n");
 
@@ -468,8 +468,8 @@ main (int argc, char **argv)
 	   "  int i = lookup_handler (scode);\n"
 	   "  if (i >= 0)\n"
 	   "    {\n"
-	   "      bool ret = this_target_optabs->pat_enable[i];\n"
-	   "      this_target_optabs->pat_enable[i] = set;\n"
+	   "      bool ret = this_fn_optabs->pat_enable[i];\n"
+	   "      this_fn_optabs->pat_enable[i] = set;\n"
 	   "      return ret;\n"
 	   "    }\n"
 	   "  else\n"
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c1dacf4..c5778d1 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 
 struct target_optabs default_target_optabs;
 struct target_libfuncs default_target_libfuncs;
+struct target_optabs *this_fn_optabs = &default_target_optabs;
 #if SWITCHABLE_TARGET
 struct target_optabs *this_target_optabs = &default_target_optabs;
 struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs;
@@ -6150,7 +6151,7 @@ init_optabs (void)
     libfunc_hash = htab_create_ggc (10, hash_libfunc, eq_libfunc, NULL);
 
   /* Fill in the optabs with the insns we support.  */
-  init_all_optabs ();
+  init_all_optabs (this_fn_optabs);
 
   /* The ffs function operates on `int'.  Fall back on it if we do not
      have a libgcc2 function for that width.  */
@@ -6207,6 +6208,38 @@ init_optabs (void)
   targetm.init_libfuncs ();
 }
 
+/* Recompute the optabs and save them if they have changed.  */
+
+void
+save_optabs_if_changed (tree fndecl)
+{
+  /* ?? 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);
+
+  struct target_optabs *tmp_optabs = (struct target_optabs *)
+    ggc_alloc_atomic (sizeof (struct target_optabs));
+  tree optnode = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl);
+
+  /* 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;
+    }
+  else
+    {
+      TREE_OPTIMIZATION_OPTABS (optnode) = NULL;
+      ggc_free (tmp_optabs);
+    }
+}
+
 /* A helper function for init_sync_libfuncs.  Using the basename BASE,
    install libfuncs into TAB for BASE_N for 1 <= N <= MAX.  */
 
diff --git a/gcc/optabs.h b/gcc/optabs.h
index c08adcf..4de4409 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -76,6 +76,7 @@ struct target_optabs {
 };
 
 extern struct target_optabs default_target_optabs;
+extern struct target_optabs *this_fn_optabs;
 #if SWITCHABLE_TARGET
 extern struct target_optabs *this_target_optabs;
 #else
diff --git a/gcc/target-globals.c b/gcc/target-globals.c
index 533a8e5..d72495d 100644
--- a/gcc/target-globals.c
+++ b/gcc/target-globals.c
@@ -91,4 +91,33 @@ save_target_globals (void)
   return g;
 }
 
+/* Like save_target_globals() above, but set *this_target_optabs
+   correctly when a previous function has changed
+   *this_target_optabs.  */
+
+struct target_globals *
+save_target_globals_default_opts ()
+{
+  struct target_globals *globals;
+
+  if (optimization_current_node != optimization_default_node)
+    {
+      tree opts = optimization_current_node;
+      /* Temporarily switch to the default optimization node, so that
+	 *this_target_optabs is set to the default, not reflecting
+	 whatever a previous function used for the optimize
+	 attribute.  */
+      optimization_current_node = optimization_default_node;
+      cl_optimization_restore
+	(&global_options,
+	 TREE_OPTIMIZATION (optimization_default_node));
+      globals = save_target_globals ();
+      optimization_current_node = opts;
+      cl_optimization_restore (&global_options,
+			       TREE_OPTIMIZATION (opts));
+      return globals;
+    }
+  return save_target_globals ();
+}
+
 #endif
diff --git a/gcc/target-globals.h b/gcc/target-globals.h
index 110f879..04eba53 100644
--- a/gcc/target-globals.h
+++ b/gcc/target-globals.h
@@ -60,6 +60,7 @@ struct GTY(()) target_globals {
 extern struct target_globals default_target_globals;
 
 extern struct target_globals *save_target_globals (void);
+extern struct target_globals *save_target_globals_default_opts (void);
 
 static inline void
 restore_target_globals (struct target_globals *g)
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
new file mode 100644
index 0000000..7016834
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c
@@ -0,0 +1,10 @@
+/* { dg-options "-ffast-math" } */
+
+float farg;
+unsigned val;
+
+void __attribute__((optimize("O")))
+test()
+{
+  val = __builtin_ceilf(farg);
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index c3c814c..740d438 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option {
 
   /* The optimization options used by the user.  */
   struct cl_optimization opts;
+
+  /* Target optabs for this set of optimization options.  This is of
+     type `struct target_optabs *'.  */
+  unsigned char *GTY ((atomic)) target_optabs;
 };
 
 #define TREE_OPTIMIZATION(NODE) \
   (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts)
 
+#define TREE_OPTIMIZATION_OPTABS(NODE) \
+  (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_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);
+
 /* Target options used by a function.  */
 
 struct GTY(()) tree_target_option {

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Mon, Feb 18, 2013 at 12:50:59PM -0600, Aldy Hernandez wrote:
> OK pending tests?

> 	PR target/52555
> 	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
> 	(swap_optab_enable): Same.
> 	(init_all_optabs): Use argument instead of global.
> 	* tree.h (struct tree_optimization_option): New field
> 	target_optabs.
> 	* expr.h (init_all_optabs): Add argument to prototype.
> 	(TREE_OPTIMIZATION_OPTABS): New.
> 	(save_optabs_if_changed): Protoize.
> 	* optabs.h: Declare this_fn_optabs.
> 	* optabs.c (save_optabs_if_changed): New.
> 	Declare this_fn_optabs.
> 	(init_optabs): Add argument to init_all_optabs() call.
> 	* function.c (invoke_set_current_function_hook): Handle per
> 	function optabs.
> 	* function.h (struct function): New field optabs.
> 	* config/mips/mips.c (mips_set_mips16_mode): Handle when
> 	optimization_current_node has changed.
> 	* target-globals.h (save_target_globals_default_opts): Protoize.
> 	* target-globals.c (save_target_globals_default_opts): New.
> c/family
> 	PR target/52555
> 	* c-common.c (handle_optimize_attribute): Call
> 	save_optabs_if_changed.

Yes, thanks.

	Jakub

RE: PR target/52555: attribute optimize is overriding command line options

[Steve Ellcey]

On Mon, Feb 18, 2013 at 12:50:59PM -0600, Aldy Hernandez wrote:
> OK pending tests?

>       PR target/52555
>       * genopinit.c (raw_optab_handler): Use this_fn_optabs.
>       (swap_optab_enable): Same.
>       (init_all_optabs): Use argument instead of global.
>       * tree.h (struct tree_optimization_option): New field
>       target_optabs.
>       * expr.h (init_all_optabs): Add argument to prototype.
>       (TREE_OPTIMIZATION_OPTABS): New.
>       (save_optabs_if_changed): Protoize.
>       * optabs.h: Declare this_fn_optabs.
>       * optabs.c (save_optabs_if_changed): New.
>       Declare this_fn_optabs.
>       (init_optabs): Add argument to init_all_optabs() call.
>       * function.c (invoke_set_current_function_hook): Handle per
>       function optabs.
>       * function.h (struct function): New field optabs.
>       * config/mips/mips.c (mips_set_mips16_mode): Handle when
>       optimization_current_node has changed.
>       * target-globals.h (save_target_globals_default_opts): Protoize.
>       * target-globals.c (save_target_globals_default_opts): New.
> c/family
>       PR target/52555
>       * c-common.c (handle_optimize_attribute): Call
>       save_optabs_if_changed.

Aldy,

Have you gotten any reports of problems with this patch?  It seems to be sending cc1 into an infinite
loop during the GCC testsuite for me.  I am testing the mips-mti-linux-gnu target and tests like
gcc.target/mips/call-saved-1.c are causing cc1 to suck up all my memory and swap space before the
test times out.

I will keep digging and see if I can figure out what is going on but I wanted to see if anyone else has
reported this problem.

Steve Ellcey
sellcey@imgtec.com

Re: PR target/52555: attribute optimize is overriding command line options

[Aldy Hernandez]

> Have you gotten any reports of problems with this patch?  It seems to be sending cc1 into an infinite
> loop during the GCC testsuite for me.  I am testing the mips-mti-linux-gnu target and tests like
> gcc.target/mips/call-saved-1.c are causing cc1 to suck up all my memory and swap space before the
> test times out.

No, I have not.
>
> I will keep digging and see if I can figure out what is going on but I wanted to see if anyone else has
> reported this problem.
>

Feel free to open a PR and CC me on it.

Thanks.

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Thu, Feb 21, 2013 at 11:02:56PM +0000, Steve Ellcey wrote:
> Have you gotten any reports of problems with this patch?  It seems to be sending cc1 into an infinite
> loop during the GCC testsuite for me.  I am testing the mips-mti-linux-gnu target and tests like
> gcc.target/mips/call-saved-1.c are causing cc1 to suck up all my memory and swap space before the
> test times out.
> 
> I will keep digging and see if I can figure out what is going on but I wanted to see if anyone else has
> reported this problem.

I think this should fix this (but totally untested except for
call-saved-1.c, and it doesn't make any sense to test on non-mips).

The problem I believe is that Aldy has changed init_optabs and insn-opinit.c
to use this_fn_optabs instead of this_target_optabs, but it is only set in
invoke_set_current_function_hook.  During save_target_globals we want to
init this_target_optabs, so we need to temporarily switch this_fn_optabs to
make that happen.

2013-02-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/52555
	* target-globals.c (save_target_globals): For init_reg_sets and
	target_reinit remporarily set this_fn_optabs to this_target_optabs.

--- gcc/target-globals.c.jj	2013-02-19 07:40:03.000000000 +0100
+++ gcc/target-globals.c	2013-02-22 10:55:36.725435859 +0100
@@ -67,6 +67,7 @@ struct target_globals *
 save_target_globals (void)
 {
   struct target_globals *g;
+  struct target_optabs *saved_this_fn_optabs = this_fn_optabs;
 
   g = ggc_alloc_target_globals ();
   g->flag_state = XCNEW (struct target_flag_state);
@@ -86,8 +87,10 @@ save_target_globals (void)
   g->bb_reorder = XCNEW (struct target_bb_reorder);
   g->lower_subreg = XCNEW (struct target_lower_subreg);
   restore_target_globals (g);
+  this_fn_optabs = this_target_optabs;
   init_reg_sets ();
   target_reinit ();
+  this_fn_optabs = saved_this_fn_optabs;
   return g;
 }
 

	Jakub

Re: PR target/52555: attribute optimize is overriding command line options

[Steve Ellcey]

On Fri, 2013-02-22 at 11:03 +0100, Jakub Jelinek wrote:

> The problem I believe is that Aldy has changed init_optabs and insn-opinit.c
> to use this_fn_optabs instead of this_target_optabs, but it is only set in
> invoke_set_current_function_hook.  During save_target_globals we want to
> init this_target_optabs, so we need to temporarily switch this_fn_optabs to
> make that happen.
> 
> 2013-02-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/52555
> 	* target-globals.c (save_target_globals): For init_reg_sets and
> 	target_reinit remporarily set this_fn_optabs to this_target_optabs.
> 

Jakub,

I built with this patch and ran the GCC testsuite (using the target
mips-mti-linux-gnu), it fixed the problem and caused no regressions for
me.

Steve Ellcey
sellcey@imgtec.com

Re: PR target/52555: attribute optimize is overriding command line options

[Jakub Jelinek]

On Fri, Feb 22, 2013 at 09:31:54AM -0800, Steve Ellcey wrote:
> On Fri, 2013-02-22 at 11:03 +0100, Jakub Jelinek wrote:
> 
> > The problem I believe is that Aldy has changed init_optabs and insn-opinit.c
> > to use this_fn_optabs instead of this_target_optabs, but it is only set in
> > invoke_set_current_function_hook.  During save_target_globals we want to
> > init this_target_optabs, so we need to temporarily switch this_fn_optabs to
> > make that happen.
> > 
> > 2013-02-22  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR target/52555
> > 	* target-globals.c (save_target_globals): For init_reg_sets and
> > 	target_reinit remporarily set this_fn_optabs to this_target_optabs.
> > 
> 
> I built with this patch and ran the GCC testsuite (using the target
> mips-mti-linux-gnu), it fixed the problem and caused no regressions for
> me.

Richard, is that ok for trunk then?

	Jakub

Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford]

Jakub Jelinek <jakub@redhat.com> writes:
> On Fri, Feb 22, 2013 at 09:31:54AM -0800, Steve Ellcey wrote:
>> On Fri, 2013-02-22 at 11:03 +0100, Jakub Jelinek wrote:
>> 
>> > The problem I believe is that Aldy has changed init_optabs and insn-opinit.c
>> > to use this_fn_optabs instead of this_target_optabs, but it is only set in
>> > invoke_set_current_function_hook.  During save_target_globals we want to
>> > init this_target_optabs, so we need to temporarily switch this_fn_optabs to
>> > make that happen.
>> > 
>> > 2013-02-22  Jakub Jelinek  <jakub@redhat.com>
>> > 
>> > 	PR target/52555
>> > 	* target-globals.c (save_target_globals): For init_reg_sets and
>> > 	target_reinit remporarily set this_fn_optabs to this_target_optabs.
>> > 
>> 
>> I built with this patch and ran the GCC testsuite (using the target
>> mips-mti-linux-gnu), it fixed the problem and caused no regressions for
>> me.
>
> Richard, is that ok for trunk then?

Looks good to me.  Thanks for fixing it.

Richard

RE: PR target/52555: attribute optimize is overriding command line options

[Steve Ellcey]

Jakub and Aldy,

It looks like I am having another problem with this patch.  Jakubs earlier patch fixed things
for me when building my mips-mti-elf target but I just started building glibc in mips16 mode
with the latest compiler and I am running into this assert:

mktime.c:147:1: internal compiler error: in save_optabs_if_changed, at optabs.c:6221
 {
 ^
0x8198e5 save_optabs_if_changed(tree_node*)
	/local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/optabs.c:6221
0x4b090b decl_attributes(tree_node**, tree_node*, int)
	/local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/attribs.c:545
0x4cf728 start_function(c_declspecs*, c_declarator*, tree_node*)
	/local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/c/c-decl.c:7727
0x557114 c_common_parse_file()
	/local/home/sellcey/gcc/linux_mips16/src/gcc/gcc/c-family/c-opts.c:1046

I looked at this_target_optabs and it appears to be a valid pointer but it is not pointing at
default_target_optabs addr and so I get the assert.  I am still trying to dig out more 
information and create a good test case.

Steve Ellcey
sellcey@imgtec.com