From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61890 invoked by alias); 12 Sep 2017 11:40:02 -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 61861 invoked by uid 89); 12 Sep 2017 11:40:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,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 ESMTP; Tue, 12 Sep 2017 11:40:00 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9B2EBC04B328; Tue, 12 Sep 2017 11:39:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9B2EBC04B328 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jakub@redhat.com Received: from tucnak.zalov.cz (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 108D877D65; Tue, 12 Sep 2017 11:39:57 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v8CBdtFx015098; Tue, 12 Sep 2017 13:39:55 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v8CBdp4I015097; Tue, 12 Sep 2017 13:39:51 +0200 Date: Tue, 12 Sep 2017 11:40:00 -0000 From: Jakub Jelinek To: Martin =?utf-8?B?TGnFoWth?= Cc: Jason Merrill , Joseph Myers , gcc-patches List , Richard Biener Subject: Re: [RFC][PATCH] Do refactoring of attribute functions and move them to attribs.[hc]. Message-ID: <20170912113951.GG1701@tucnak> Reply-To: Jakub Jelinek References: <4cf00ee9-4cd7-1806-481f-966611768a15@suse.cz> <8a0b2ff0-50f9-771d-fb63-01d17cb06b46@suse.cz> <669c960e-f4a8-eae3-efd2-f8c29477a817@suse.cz> <7e541554-10a6-9cae-fca0-042900271768@suse.cz> <20170912075453.GD1701@tucnak> <1f89ae73-7e84-66db-fec3-1e86e6c816d8@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1f89ae73-7e84-66db-fec3-1e86e6c816d8@suse.cz> User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00671.txt.bz2 On Tue, Sep 12, 2017 at 01:31:47PM +0200, Martin Liška wrote: > >From a40c06fc06afcb7bb886d7a3106e6da631a48430 Mon Sep 17 00:00:00 2001 > From: marxin > Date: Tue, 12 Sep 2017 13:30:39 +0200 > Subject: [PATCH] Reduce lookup_attribute memory footprint. > > gcc/ChangeLog: > > 2017-09-12 Martin Liska > > * attribs.c (private_lookup_attribute): New function. > * attribs.h (private_lookup_attribute): Declared here. > (lookup_attribute): Called from this place. > --- > gcc/attribs.c | 17 +++++++++++++++++ > gcc/attribs.h | 17 ++++++----------- > 2 files changed, 23 insertions(+), 11 deletions(-) > > diff --git a/gcc/attribs.c b/gcc/attribs.c > index b8f58a74596..9064434e5f2 100644 > --- a/gcc/attribs.c > +++ b/gcc/attribs.c > @@ -1584,3 +1584,20 @@ attribute_list_contained (const_tree l1, const_tree l2) > > return 1; > } > + > + Can you please add a function comment (and in addition to arguments explain there why lookup_attribute is split into the two parts)? > +tree > +private_lookup_attribute (const char *attr_name, size_t attr_len, tree list) > +{ > + while (list) > + { > @@ -151,17 +156,7 @@ lookup_attribute (const char *attr_name, tree list) > /* Do the strlen() before calling the out-of-line implementation. > In most cases attr_name is a string constant, and the compiler > will optimize the strlen() away. */ Part of the comment is of course here and that comment didn't make any sense when everything was inlined. > - while (list) > - { > - tree attr = get_attribute_name (list); > - size_t ident_len = IDENTIFIER_LENGTH (attr); > - if (cmp_attribs (attr_name, attr_len, IDENTIFIER_POINTER (attr), > - ident_len)) > - break; > - list = TREE_CHAIN (list); > - } > - > - return list; > + return private_lookup_attribute (attr_name, attr_len, list); > } > } LGTM with the comment added. In theory fnsplit could handle that too, but 1) it would emit out of line stuff in every TU separately 2) the compiler doesn't know that NULL DECL_ATTRIBUTES is so common (it could with profiledbootstrap). And of course LTO can decide to inline it from attribs.c back anyway if there are reasons why it would be beneficial somewhere (but I doubt it is beneficial at least in most spots). Jakub