From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 924 invoked by alias); 10 May 2016 10:03:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 903 invoked by uid 89); 10 May 2016 10:03:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 10 May 2016 10:03:10 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3FFB785542; Tue, 10 May 2016 10:03:09 +0000 (UTC) Received: from redhat.com (ovpn-204-17.brq.redhat.com [10.40.204.17]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4AA344D004402 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 10 May 2016 06:03:06 -0400 Date: Tue, 10 May 2016 10:03:00 -0000 From: Marek Polacek To: Martin Sebor Cc: GCC Patches , Joseph Myers , Richard Biener Subject: Re: [C PATCH] Warn for optimize attribute on decl after definition (PR c/70255) Message-ID: <20160510100304.GC20450@redhat.com> References: <20160509144510.GA20450@redhat.com> <57315B09.1060606@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57315B09.1060606@gmail.com> User-Agent: Mutt/1.6.0 (2016-04-01) X-SW-Source: 2016-05/txt/msg00681.txt.bz2 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 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