Re: [SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Feb 21, 2018 at 11:32 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jan 24, 2018, Jakub Jelinek <jakub@redhat.com> wrote:
>
>>> --- a/gcc/tree-ssa-live.c
>>> +++ b/gcc/tree-ssa-live.c
>>> @@ -520,6 +520,11 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block)
>>> else if (!BLOCK_SUPERCONTEXT (scope)
>>> || TREE_CODE (BLOCK_SUPERCONTEXT (scope)) == FUNCTION_DECL)
>>> unused = false;
>>> +   /* Preserve the block, it is referenced by at least the inline
>>> +      entry point marker.  */
>>> +   else if (debug_nonbind_markers_p
>>> +        && inlined_function_outer_scope_p (scope))
>>> +     unused = false;
>>> /* Innermost blocks with no live variables nor statements can be always
>>> eliminated.  */
>>> else if (!nsubblocks)
>>> @@ -548,11 +553,13 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block)
>>> }
>>> else if (BLOCK_VARS (scope) || BLOCK_NUM_NONLOCALIZED_VARS (scope))
>>> unused = false;
>>> -   /* See if this block is important for representation of inlined function.
>>> -      Inlined functions are always represented by block with
>>> -      block_ultimate_origin being set to FUNCTION_DECL and DECL_SOURCE_LOCATION
>>> -      set...  */
>>> -   else if (inlined_function_outer_scope_p (scope))
>>> +   /* See if this block is important for representation of inlined
>>> +      function.  Inlined functions are always represented by block
>>> +      with block_ultimate_origin being set to FUNCTION_DECL and
>>> +      DECL_SOURCE_LOCATION set, unless they expand to nothing...  But
>>> +      see above for the case of statement frontiers.  */
>>> +   else if (!debug_nonbind_markers_p
>>> +        && inlined_function_outer_scope_p (scope))
>>> unused = false;
>
>> Wonder what the above hunks will do for LTO memory consumption.  We'll see.
>
>
> [IEPM] don't preserve lexical blocks just for debug inline markers
>
> This patch stops preserving scope blocks just because they are inlined
> function scopes, when cleaning up unused scope blocks.  This change
> was introduced along with IEPM, but it preserved lots of blocks, and
> output debug information for them, although no code from the inlined
> function remained after optimization.
>
> The additional preserved blocks took up compile-time memory, and
> significant disk space and link time, in some cases more than 25%.
> This is deemed excessive, compared with the reasonably small benefit
> of allowing one to single-step into an inlined function using a
> view-capable debugger.
>
> There was another way of marking inlined function scopes as unused,
> based on the markers referencing them during stmt scanning, but that
> still preserved too much.
>
> So, this patch restores the pre-IEPM logic of preservation of scopes.
> Should a scope block referenced by an inline entry marker be found to
> be unused in remove_unused_scope_block_p, the marker will be cleaned
> up right after that, in clear_unused_block_pointer, so we won't keep
> a dangling reference to a dropped block.
>
> Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?

Ok.

Thanks,
Richard.

> for  gcc/ChangeLog
>
>         * tree-ssa-live.c (remove_unused_scope_block_p): Do not
>         preserve inline entry blocks for the sake of debug inline
>         entry point markers alone.
>         (remove_unused_locals): Suggest in comments a better place to
>         force the preservation of inline entry blocks that are
>         otherwise unused, but do not preserve them.
> ---
>  gcc/tree-ssa-live.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
> index 62bb3c5de659..62316bac7929 100644
> --- a/gcc/tree-ssa-live.c
> +++ b/gcc/tree-ssa-live.c
> @@ -520,11 +520,6 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block)
>     else if (!BLOCK_SUPERCONTEXT (scope)
>              || TREE_CODE (BLOCK_SUPERCONTEXT (scope)) == FUNCTION_DECL)
>       unused = false;
> -   /* Preserve the block, it is referenced by at least the inline
> -      entry point marker.  */
> -   else if (debug_inline_points
> -           && inlined_function_outer_scope_p (scope))
> -     unused = false;
>     /* Innermost blocks with no live variables nor statements can be always
>        eliminated.  */
>     else if (!nsubblocks)
> @@ -556,10 +551,8 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block)
>     /* See if this block is important for representation of inlined
>        function.  Inlined functions are always represented by block
>        with block_ultimate_origin being set to FUNCTION_DECL and
> -      DECL_SOURCE_LOCATION set, unless they expand to nothing...  But
> -      see above for the case of statement frontiers.  */
> -   else if (!debug_inline_points
> -           && inlined_function_outer_scope_p (scope))
> +      DECL_SOURCE_LOCATION set, unless they expand to nothing...  */
> +   else if (inlined_function_outer_scope_p (scope))
>       unused = false;
>     else
>     /* Verfify that only blocks with source location set
> @@ -741,6 +734,10 @@ remove_unused_locals (void)
>           gimple *stmt = gsi_stmt (gsi);
>           tree b = gimple_block (stmt);
>
> +         /* If we wanted to mark the block referenced by the inline
> +            entry point marker as used, this would be a good spot to
> +            do it.  If the block is not otherwise used, the stmt will
> +            be cleaned up in clean_unused_block_pointer.  */
>           if (is_gimple_debug (stmt))
>             continue;
>
>
>
> Here's a patch I'm sharing just FTR, that introduces (on top of the
> above) options to select other approaches to preserve scopes related
> with inlined functions.  It's not meant for inclusion; the options'
> spelling are such that the three possible approaches, as well as
> -gno-inline-points, can be tested so as to compare file sizes to measure
> debug info size changes, without noise out of different length of the
> command-line options.
>
> The differences in libgcc are tiny, but those in cc1 and cc1plus are
> quite significant:
>
>                no         df         mr         al
> cc1            260726424  280455288  312591944  312600224
> cc1plus        289090200  309719888  347144256  347152840
> libgcc_s.so       864016     867840     868128     868128
> libstdc++.so    12846528   14758400   17619736   17619480
>
>
> ---
>  gcc/common.opt      |   12 ++++++++++++
>  gcc/tree-ssa-live.c |    5 ++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index e0bc4d1bb18d..594ea0213cba 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2924,6 +2924,18 @@ ginline-points
>  Common Driver Var(debug_inline_points) Init(2)
>  Generate extended entry point information for inlined functions
>
> +ginline-points=df
> +Common Driver Var(debug_inline_points, 1) Init(2)
> +Don't preserve blocks just for inlined functions
> +
> +ginline-points=al
> +Common Driver Var(debug_inline_points, 3) Init(2)
> +Preserve all blocks regarded as inlined function scopes
> +
> +ginline-points=mr
> +Common Driver Var(debug_inline_points, 4) Init(2)
> +Preserve blocks referenced by inline entry point markers only
> +
>  ginternal-reset-location-views
>  Common Driver Var(debug_internal_reset_location_views) Init(2)
>  Compute locview reset points based on insn length estimates
> diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
> index 62316bac7929..f75a6ea2126a 100644
> --- a/gcc/tree-ssa-live.c
> +++ b/gcc/tree-ssa-live.c
> @@ -520,6 +520,8 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block)
>     else if (!BLOCK_SUPERCONTEXT (scope)
>              || TREE_CODE (BLOCK_SUPERCONTEXT (scope)) == FUNCTION_DECL)
>       unused = false;
> +   else if (debug_inline_points == 3 && inlined_function_outer_scope_p (scope))
> +     unused = false;
>     /* Innermost blocks with no live variables nor statements can be always
>        eliminated.  */
>     else if (!nsubblocks)
> @@ -738,7 +740,8 @@ remove_unused_locals (void)
>              entry point marker as used, this would be a good spot to
>              do it.  If the block is not otherwise used, the stmt will
>              be cleaned up in clean_unused_block_pointer.  */
> -         if (is_gimple_debug (stmt))
> +         if (is_gimple_debug (stmt)
> +             && (debug_inline_points != 4 || !gimple_debug_inline_entry_p (stmt)))
>             continue;
>
>           if (gimple_clobber_p (stmt))
>
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer