This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Cleanup tree merging
- From: Richard Biener <rguenther at suse dot de>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 3 Sep 2018 09:32:42 +0200 (CEST)
- Subject: Re: Cleanup tree merging
- References: <20180902231442.GK68683@kam.mff.cuni.cz>
On Mon, 3 Sep 2018, Jan Hubicka wrote:
> Hi,
> while dropping streaming of now unnecesary fields I forgot to update lto.c
> side. This patch removes few checks form tree merging that will hopefully
> make it bit faster. While doing that I also turned DECL_ASSEMBLER_NAME
> check to use DECL_ASSEMBLER_NAME_RAW.
>
> lto-bootstraped/regtested x86_64-linux.
> OK?
>
> Honza
> * lto.c (mentions_vars_p_function): Do not check DECL_ARGUMENTS and
> VINDEX.
> (mentions_vars_p_field_decl): Do not check DECL_FCONTEXT.
> (mentions_vars_p_binfo): Do not check BASE_ACCESSES, VIRTUALS and
> VPTR_FIELD.
> (compare_tree_sccs_1): Do no check BLOCKs; do not check
> BINFO_BASE_ACCESSES; function arguments and results, ORIGINAL_TYPE;
> use DECL_ASSEMBLER_NAME_RAW when comparing assembler names;
> do not check DECL_FCONTEXT; do not check DECL_VINDEX; do not check
> TYPE_STUB_DECL.
> (compare_tree_sccs_1): Do not check BINFO_BASE_ACCESS;
> BINFO_VPTR_FIELD, DECL_ARGUMENTS, DECL_VINDEX, DECL_FCONTEXT.
>
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c (revision 263989)
> +++ lto/lto.c (working copy)
> @@ -614,8 +614,8 @@ mentions_vars_p_function (tree t)
> {
> if (mentions_vars_p_decl_non_common (t))
> return true;
> - CHECK_NO_VAR (DECL_ARGUMENTS (t));
> - CHECK_NO_VAR (DECL_VINDEX (t));
> + gcc_checking_assert (!DECL_ARGUMENTS (t));
> + gcc_checking_assert (!DECL_VINDEX (t));
> CHECK_VAR (DECL_FUNCTION_PERSONALITY (t));
> return false;
> }
> @@ -631,7 +630,7 @@ mentions_vars_p_field_decl (tree t)
> CHECK_NO_VAR (DECL_BIT_FIELD_TYPE (t));
> CHECK_NO_VAR (DECL_QUALIFIER (t));
> CHECK_NO_VAR (DECL_FIELD_BIT_OFFSET (t));
> - CHECK_NO_VAR (DECL_FCONTEXT (t));
> + gcc_checking_assert (!DECL_FCONTEXT (t));
> return false;
> }
>
> @@ -672,11 +671,8 @@ mentions_vars_p_binfo (tree t)
> return true;
> CHECK_VAR (BINFO_VTABLE (t));
> CHECK_NO_VAR (BINFO_OFFSET (t));
> - CHECK_NO_VAR (BINFO_VIRTUALS (t));
> - CHECK_NO_VAR (BINFO_VPTR_FIELD (t));
> - n = vec_safe_length (BINFO_BASE_ACCESSES (t));
> - for (i = 0; i < n; i++)
> - CHECK_NO_VAR (BINFO_BASE_ACCESS (t, i));
> + gcc_checking_assert (!BINFO_VIRTUALS (t));
> + gcc_checking_assert (!BINFO_VPTR_FIELD (t));
Can you add
gcc_checking_assert (vec_safe_lengthh (BINFO_BASE_ACCESSES (t)) == );
for completeness
> /* Do not walk BINFO_INHERITANCE_CHAIN, BINFO_SUBVTT_INDEX
> and BINFO_VPTR_INDEX; these are used by C++ FE only. */
> n = BINFO_N_BASE_BINFOS (t);
> @@ -1210,7 +1206,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> /* BLOCKs are function local and we don't merge anything there, so
> simply refuse to merge. */
> if (CODE_CONTAINS_STRUCT (code, TS_BLOCK))
> - return false;
> + gcc_unreachable ();
>
> if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
> if (strcmp (TRANSLATION_UNIT_LANGUAGE (t1),
> @@ -1227,9 +1223,8 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> return false;
>
> if (CODE_CONTAINS_STRUCT (code, TS_BINFO))
> - if (vec_safe_length (BINFO_BASE_ACCESSES (t1))
> - != vec_safe_length (BINFO_BASE_ACCESSES (t2)))
> - return false;
> + gcc_assert (!vec_safe_length (BINFO_BASE_ACCESSES (t1))
> + && !vec_safe_length (BINFO_BASE_ACCESSES (t2)));
>
> if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
> compare_values (CONSTRUCTOR_NELTS);
> @@ -1362,23 +1357,17 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> {
> if (code == FUNCTION_DECL)
> {
> - tree a1, a2;
> - for (a1 = DECL_ARGUMENTS (t1), a2 = DECL_ARGUMENTS (t2);
> - a1 || a2;
> - a1 = TREE_CHAIN (a1), a2 = TREE_CHAIN (a2))
> - compare_tree_edges (a1, a2);
> - compare_tree_edges (DECL_RESULT (t1), DECL_RESULT (t2));
> + gcc_checking_assert (!DECL_ARGUMENTS (t1));
> + gcc_checking_assert (!DECL_RESULT (t1));
Does this really work!? I see we do not stream DECL_ARGUMENTS
but populate that by function body streaming. Maybe add a comment
here. I wonder what makes us avoid merging too many FUNCTION_DECLs
then? Consider two functions with the same TYPE_ARG_TYPES but
differing DECL_BY_REFERENCE on the corresponding PARM_DECL?
[unfortunately type and decl specs are not sufficient on their own
semantically...]
With asserts in compare_tree_sccs_1 please always assert
for both t1 and t2.
Otherwise OK.
Richard.
> }
> else if (code == TYPE_DECL)
> - compare_tree_edges (DECL_ORIGINAL_TYPE (t1), DECL_ORIGINAL_TYPE (t2));
> + gcc_checking_assert (!DECL_ORIGINAL_TYPE (t1));
> }
>
> if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
> {
> - /* Make sure we don't inadvertently set the assembler name. */
> - if (DECL_ASSEMBLER_NAME_SET_P (t1))
> - compare_tree_edges (DECL_ASSEMBLER_NAME (t1),
> - DECL_ASSEMBLER_NAME (t2));
> + compare_tree_edges (DECL_ASSEMBLER_NAME_RAW (t1),
> + DECL_ASSEMBLER_NAME_RAW (t2));
> }
>
> if (CODE_CONTAINS_STRUCT (code, TS_FIELD_DECL))
> @@ -1389,14 +1378,14 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> DECL_BIT_FIELD_REPRESENTATIVE (t2));
> compare_tree_edges (DECL_FIELD_BIT_OFFSET (t1),
> DECL_FIELD_BIT_OFFSET (t2));
> - compare_tree_edges (DECL_FCONTEXT (t1), DECL_FCONTEXT (t2));
> + gcc_checking_assert (!DECL_FCONTEXT (t1));
> }
>
> if (CODE_CONTAINS_STRUCT (code, TS_FUNCTION_DECL))
> {
> compare_tree_edges (DECL_FUNCTION_PERSONALITY (t1),
> DECL_FUNCTION_PERSONALITY (t2));
> - compare_tree_edges (DECL_VINDEX (t1), DECL_VINDEX (t2));
> + gcc_checking_assert (!DECL_VINDEX (t1));
> compare_tree_edges (DECL_FUNCTION_SPECIFIC_TARGET (t1),
> DECL_FUNCTION_SPECIFIC_TARGET (t2));
> compare_tree_edges (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (t1),
> @@ -1423,7 +1412,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> compare_tree_edges (TYPE_CONTEXT (t1), TYPE_CONTEXT (t2));
> /* TYPE_CANONICAL is re-computed during type merging, so do not
> compare it here. */
> - compare_tree_edges (TYPE_STUB_DECL (t1), TYPE_STUB_DECL (t2));
> + gcc_checking_assert (!TYPE_STUB_DECL (t1));
> }
>
> if (CODE_CONTAINS_STRUCT (code, TS_TYPE_NON_COMMON))
> @@ -1468,7 +1457,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t
>
> /* BLOCKs are function local and we don't merge anything there. */
> if (TREE_BLOCK (t1) || TREE_BLOCK (t2))
> - return false;
> + gcc_unreachable ();
> }
>
> if (CODE_CONTAINS_STRUCT (code, TS_BINFO))
> @@ -1478,11 +1467,9 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> /* Lengths have already been compared above. */
> FOR_EACH_VEC_ELT (*BINFO_BASE_BINFOS (t1), i, t)
> compare_tree_edges (t, BINFO_BASE_BINFO (t2, i));
> - FOR_EACH_VEC_SAFE_ELT (BINFO_BASE_ACCESSES (t1), i, t)
> - compare_tree_edges (t, BINFO_BASE_ACCESS (t2, i));
> compare_tree_edges (BINFO_OFFSET (t1), BINFO_OFFSET (t2));
> compare_tree_edges (BINFO_VTABLE (t1), BINFO_VTABLE (t2));
> - compare_tree_edges (BINFO_VPTR_FIELD (t1), BINFO_VPTR_FIELD (t2));
> + gcc_checking_assert (!BINFO_VPTR_FIELD (t1));
> /* Do not walk BINFO_INHERITANCE_CHAIN, BINFO_SUBVTT_INDEX
> and BINFO_VPTR_INDEX; these are used by C++ FE only. */
> }
> @@ -2621,9 +2608,9 @@ lto_fixup_prevailing_decls (tree t)
> }
> if (CODE_CONTAINS_STRUCT (code, TS_FUNCTION_DECL))
> {
> - LTO_NO_PREVAIL (DECL_ARGUMENTS (t));
> + gcc_checking_assert (!DECL_ARGUMENTS (t));
> LTO_SET_PREVAIL (DECL_FUNCTION_PERSONALITY (t));
> - LTO_NO_PREVAIL (DECL_VINDEX (t));
> + gcc_checking_assert (!DECL_VINDEX (t));
> }
> if (CODE_CONTAINS_STRUCT (code, TS_FIELD_DECL))
> {
> @@ -2631,7 +2618,7 @@ lto_fixup_prevailing_decls (tree t)
> LTO_NO_PREVAIL (DECL_BIT_FIELD_TYPE (t));
> LTO_NO_PREVAIL (DECL_QUALIFIER (t));
> LTO_NO_PREVAIL (DECL_FIELD_BIT_OFFSET (t));
> - LTO_NO_PREVAIL (DECL_FCONTEXT (t));
> + gcc_checking_assert (!DECL_FCONTEXT (t));
> }
> }
> else if (TYPE_P (t))
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)