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 <pola...@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
