public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: Martin Sebor <msebor@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	       Joseph Myers <joseph@codesourcery.com>,
	       Richard Biener <rguenther@suse.de>
Subject: Re: [C PATCH] Warn for optimize attribute on decl after definition (PR c/70255)
Date: Tue, 10 May 2016 10:03:00 -0000	[thread overview]
Message-ID: <20160510100304.GC20450@redhat.com> (raw)
In-Reply-To: <57315B09.1060606@gmail.com>

On Mon, May 09, 2016 at 09:52:41PM -0600, Martin Sebor wrote:
> On 05/09/2016 08:45 AM, Marek Polacek wrote:
> > In this PR, Richi pointed out that we don't warn for the case when a
> > declaration with attribute optimize follows the definition which is lacking
> > that attribute.  This patch adds such a warning.  Though the question is
> > whether this shouldn't apply to more attributes than just "optimize".  And,
> > as can be seen in the testcase, we'll warn for even for the case when the
> > definition has
> >    optimize ("no-associative-math,O2")
> > and the declaration
> >    optimize ("O2,no-associative-math")
> > Not sure if we have something better than attribute_value_equal, though.
> 
> There is attribute_list_equal which seems more appropriate given
> that there could be more than one attribute optimize associated
> with a function, and the order of the attributes shouldn't matter.

Ah, yea, I knew there was something else...  That's better so here is
a revised patch...

> attribute_value_equal only returns true if all attributes are
> the same and in the same order.  I would not expect GCC to warn
> on the following, for example:
> 
>   int attribute__ ((optimize ("no-reciprocal-math"),
>                     optimize ("no-associative-math")))
>   f () { return 0; }
> 
>   int __attribute__ ((optimize ("no-associative-math"),
>                       optimize ("no-reciprocal-math")))
>   f ();

... with this test included.  Thanks.

Bootstrapped/regtested on x86_64-linux, applying to trunk.

2016-05-10  Marek Polacek  <polacek@redhat.com>

	PR c/70255
	* c-decl.c (diagnose_mismatched_decls): Warn for optimize attribute
	on a declaration following the definition.

	* gcc.dg/attr-opt-1.c: New test.

diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index 7094efc..9c09536 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2228,6 +2228,18 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
 
   if (TREE_CODE (newdecl) == FUNCTION_DECL)
     {
+      tree a1 = lookup_attribute ("optimize", DECL_ATTRIBUTES (olddecl));
+      tree a2 = lookup_attribute ("optimize", DECL_ATTRIBUTES (newdecl));
+      /* An optimization attribute applied on a declaration after the
+	 definition is likely not what the user wanted.  */
+      if (a2 != NULL_TREE
+	  && DECL_SAVED_TREE (olddecl) != NULL_TREE
+	  && (a1 == NULL_TREE || !attribute_list_equal (a1, a2)))
+	warned |= warning (OPT_Wattributes,
+			   "optimization attribute on %qD follows "
+			   "definition but the attribute doesn%'t match",
+			   newdecl);
+
       /* Diagnose inline __attribute__ ((noinline)) which is silly.  */
       if (DECL_DECLARED_INLINE_P (newdecl)
 	  && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl)))
diff --git gcc/testsuite/gcc.dg/attr-opt-1.c gcc/testsuite/gcc.dg/attr-opt-1.c
index e69de29..4140fda 100644
--- gcc/testsuite/gcc.dg/attr-opt-1.c
+++ gcc/testsuite/gcc.dg/attr-opt-1.c
@@ -0,0 +1,37 @@
+/* PR c/70255 */
+/* { dg-do compile } */
+
+double
+fn1 (double h, double l) /* { dg-message "previous definition" } */
+{
+  return h + l;
+}
+double fn1 (double, double) __attribute__ ((optimize ("no-associative-math"))); /* { dg-warning "optimization attribute on .fn1. follows definition" } */
+
+__attribute__ ((optimize ("no-associative-math"))) double
+fn2 (double h, double l)
+{
+  return h + l;
+}
+double fn2 (double, double) __attribute__ ((optimize ("no-associative-math")));
+
+__attribute__ ((optimize ("no-associative-math"))) double
+fn3 (double h, double l) /* { dg-message "previous definition" } */
+{
+  return h + l;
+}
+double fn3 (double, double) __attribute__ ((optimize ("O2,no-associative-math"))); /* { dg-warning "optimization attribute on .fn3. follows definition" } */
+
+__attribute__ ((optimize ("no-associative-math,O2"))) double
+fn4 (double h, double l) /* { dg-message "previous definition" } */
+{
+  return h + l;
+}
+double fn4 (double, double) __attribute__ ((optimize ("O2,no-associative-math"))); /* { dg-warning "optimization attribute on .fn4. follows definition" } */
+
+__attribute__ ((optimize ("no-reciprocal-math"), optimize ("no-associative-math"))) int
+fn5 (void)
+{
+  return 0;
+}
+int __attribute__ ((optimize ("no-associative-math"), optimize ("no-reciprocal-math"))) fn5 (void);

	Marek

      reply	other threads:[~2016-05-10 10:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-09 14:45 Marek Polacek
2016-05-09 22:15 ` Joseph Myers
2016-05-10  3:53 ` Martin Sebor
2016-05-10 10:03   ` Marek Polacek [this message]

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=20160510100304.GC20450@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=joseph@codesourcery.com \
    --cc=msebor@gmail.com \
    --cc=rguenther@suse.de \
    /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).