On 09/12/2017 01:39 PM, Jakub Jelinek wrote: > 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 <mli...@suse.cz> >> Date: Tue, 12 Sep 2017 13:30:39 +0200 >> Subject: [PATCH] Reduce lookup_attribute memory footprint. >> >> gcc/ChangeLog: >> >> 2017-09-12 Martin Liska <mli...@suse.cz> >> >> * 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)?
Yes. > >> +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. You're completely right. > >> - 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). Doing it as it was is the right way ;) Installed as r252022. Martin > > Jakub >