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 <[email protected]>
>> Date: Tue, 12 Sep 2017 13:30:39 +0200
>> Subject: [PATCH] Reduce lookup_attribute memory footprint.
>>
>> gcc/ChangeLog:
>>
>> 2017-09-12 Martin Liska <[email protected]>
>>
>> * 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
>
