public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Richard Sandiford <rdsandiford@googlemail.com>,
	       gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: PR target/52555: attribute optimize is overriding command line options
Date: Tue, 12 Feb 2013 15:58:00 -0000	[thread overview]
Message-ID: <511A66AE.4070903@redhat.com> (raw)
In-Reply-To: <20130212140503.GD4385@tucnak.redhat.com>

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

  reply	other threads:[~2013-02-12 15:58 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-12  0:15 Aldy Hernandez
2013-02-12 14:05 ` Jakub Jelinek
2013-02-12 15:58   ` Aldy Hernandez [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=511A66AE.4070903@redhat.com \
    --to=aldyh@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=rdsandiford@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).