This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Avoid static initialization in the strlen pass
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jeff Law <law at redhat dot com>, Martin Sebor <msebor at gmail dot com>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 21 Nov 2017 09:16:10 +0100 (CET)
- Subject: Re: [PATCH] Avoid static initialization in the strlen pass
- Authentication-results: sourceware.org; auth=none
- References: <20171120211440.GR14653@tucnak>
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)