This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC][PATCH] Do refactoring of attribute functions and move them to attribs.[hc].


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 <mliska@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  <mliska@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)?

> +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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]