Re: [PATCH] Avoid static initialization in the strlen pass

[Richard Biener <rguenther at suse dot de>]

On Mon, 20 Nov 2017, Jakub Jelinek wrote:

> Hi!
> 
> All the hash_maps in tree-ssa-strlen.c except for the newly added one
> were pointers to hash maps, which were constructed either lazily or during
> the pass.  But strlen_to_stridx is now constructed at the compiler start,
> which is something I'd prefer to avoid, it affects even -O0 that way and
> empty/small file compilation, something e.g. the kernel folks care so much
> about.
> 
> Apparently the hash map is only needed when one of the two warnings
> is enabled, so this patch initializes it only in that case and otherwise
> doesn't fill it or query it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> 2017-11-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree-ssa-strlen.c (strlen_to_stridx): Change into a pointer to
> 	hash_map.
> 	(handle_builtin_strlen, strlen_optimize_stmt): Only access it
> 	if non-NULL, instead of . use ->.
> 	(handle_builtin_stxncpy): Return early if strlen_to_stridx
> 	is NULL.  Spelling fix.  Instead of . use ->.
> 	(pass_strlen::execute): Allocate strlen_to_stridx if
> 	warn_stringop_{truncation,overflow}.  Instead of calling empty on it
> 	delete it and clear it at the end of the pass.
> 
> --- gcc/tree-ssa-strlen.c.jj	2017-11-15 09:40:03.000000000 +0100
> +++ gcc/tree-ssa-strlen.c	2017-11-20 18:10:42.565458585 +0100
> @@ -153,7 +153,7 @@ struct decl_stridxlist_map
>  static hash_map<tree_decl_hash, stridxlist> *decl_to_stridxlist_htab;
>  
>  typedef std::pair<int, location_t> stridx_strlenloc;
> -static hash_map<tree, stridx_strlenloc> strlen_to_stridx;
> +static hash_map<tree, stridx_strlenloc> *strlen_to_stridx;
>  
>  /* Obstack for struct stridxlist and struct decl_stridxlist_map.  */
>  static struct obstack stridx_obstack;
> @@ -1207,8 +1207,11 @@ handle_builtin_strlen (gimple_stmt_itera
>  	      gcc_assert (si->full_string_p);
>  	    }
>  
> -	  location_t loc = gimple_location (stmt);
> -	  strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc));
> +	  if (strlen_to_stridx)
> +	    {
> +	      location_t loc = gimple_location (stmt);
> +	      strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
> +	    }
>  	  return;
>  	}
>      }
> @@ -1253,8 +1256,11 @@ handle_builtin_strlen (gimple_stmt_itera
>        set_strinfo (idx, si);
>        find_equal_ptrs (src, idx);
>  
> -      location_t loc = gimple_location (stmt);
> -      strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc));
> +      if (strlen_to_stridx)
> +	{
> +	  location_t loc = gimple_location (stmt);
> +	  strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
> +	}
>      }
>  }
>  
> @@ -1909,6 +1915,9 @@ maybe_diag_stxncpy_trunc (gimple_stmt_it
>  static void
>  handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi)
>  {
> +  if (strlen_to_stridx == NULL)
> +    return;
> +
>    gimple *stmt = gsi_stmt (*gsi);
>  
>    bool with_bounds = gimple_call_with_bounds_p (stmt);
> @@ -1917,9 +1926,9 @@ handle_builtin_stxncpy (built_in_functio
>    tree len = gimple_call_arg (stmt, with_bounds ? 3 : 2);
>  
>    /* If the length argument was computed from strlen(S) for some string
> -     S retrieve the strinfo index for the string (PSS->FIRST) alonng with
> +     S retrieve the strinfo index for the string (PSS->FIRST) along with
>       the location of the strlen() call (PSS->SECOND).  */
> -  stridx_strlenloc *pss = strlen_to_stridx.get (len);
> +  stridx_strlenloc *pss = strlen_to_stridx->get (len);
>    if (!pss || pss->first <= 0)
>      {
>        if (maybe_diag_stxncpy_trunc (*gsi, src, len))
> @@ -2966,9 +2975,12 @@ strlen_optimize_stmt (gimple_stmt_iterat
>  	  fold_strstr_to_strncmp (gimple_assign_rhs1 (stmt),
>  				  gimple_assign_rhs2 (stmt), stmt);
>  
> -	tree rhs1 = gimple_assign_rhs1 (stmt);
> -	if (stridx_strlenloc *ps = strlen_to_stridx.get (rhs1))
> -	  strlen_to_stridx.put (lhs, stridx_strlenloc (*ps));
> +	if (strlen_to_stridx)
> +	  {
> +	    tree rhs1 = gimple_assign_rhs1 (stmt);
> +	    if (stridx_strlenloc *ps = strlen_to_stridx->get (rhs1))
> +	      strlen_to_stridx->put (lhs, stridx_strlenloc (*ps));
> +	  }
>        }
>      else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
>  	{
> @@ -3202,6 +3214,9 @@ pass_strlen::execute (function *fun)
>    ssa_ver_to_stridx.safe_grow_cleared (num_ssa_names);
>    max_stridx = 1;
>  
> +  if (warn_stringop_truncation || warn_stringop_overflow)
> +    strlen_to_stridx = new hash_map<tree, stridx_strlenloc> (64);
> +
>    calculate_dominance_info (CDI_DOMINATORS);
>  
>    /* String length optimization is implemented as a walk of the dominator
> @@ -3220,7 +3235,11 @@ pass_strlen::execute (function *fun)
>    laststmt.len = NULL_TREE;
>    laststmt.stridx = 0;
>  
> -  strlen_to_stridx.empty ();
> +  if (strlen_to_stridx)
> +    {
> +      delete strlen_to_stridx;
> +      strlen_to_stridx = NULL;
> +    }
>  
>    return 0;
>  }
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)