public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR target/52555: attribute optimize is overriding command line options
@ 2013-02-12  0:15 Aldy Hernandez
  2013-02-12 14:05 ` Jakub Jelinek
  0 siblings, 1 reply; 25+ messages in thread
From: Aldy Hernandez @ 2013-02-12  0:15 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches

[-- 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 {

^ permalink raw reply	[flat|nested] 25+ messages in thread
end of thread, other threads:[~2013-03-01 23:37 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-12  0:15 PR target/52555: attribute optimize is overriding command line options Aldy Hernandez
2013-02-12 14:05 ` Jakub Jelinek
2013-02-12 15:58   ` Aldy Hernandez
2013-02-12 16:16     ` Jakub Jelinek
2013-02-12 16:30   ` Richard Sandiford
2013-02-12 17:28     ` Aldy Hernandez
2013-02-12 17:48       ` Richard Sandiford
2013-02-12 17:46         ` Richard Sandiford
2013-02-13 17:39         ` Aldy Hernandez
2013-02-13 17:58           ` Richard Sandiford
2013-02-13 18:08             ` Aldy Hernandez
2013-02-13 19:54               ` Jakub Jelinek
2013-02-15 17:23                 ` Aldy Hernandez
2013-02-15 17:35                   ` Jakub Jelinek
2013-02-15 17:52                     ` Aldy Hernandez
2013-02-16 11:20                   ` Richard Sandiford
2013-02-18 18:51                     ` Aldy Hernandez
2013-02-18 23:05                       ` Jakub Jelinek
2013-02-21 23:03                         ` Steve Ellcey
2013-02-22  0:10                           ` Aldy Hernandez
2013-02-22 10:03                           ` Jakub Jelinek
2013-02-22 17:32                             ` Steve Ellcey
2013-02-22 18:17                               ` Jakub Jelinek
2013-02-22 19:49                                 ` Richard Sandiford
2013-03-01 23:37                             ` Steve Ellcey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).