[PATCH] keep scope blocks for all inlined functions (PR 98664)

[Richard Biener]

On Thu, Jan 14, 2021 at 8:13 PM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> One aspect of PR 98465 - Bogus warning stringop-overread for std::string
> is the inconsistency between -g and -g0 which turns out to be due to
> GCC eliminating apparently unused scope blocks from inlined functions
> that aren't explicitly declared inline and artificial.  PR 98664 tracks
> just this part of PR 98465.
>
> To resolve just the PR 98664 subset the attached change has
> the tree-ssa-live.c pass preserve these blocks for all inlined
> functions, not just artificial ones.  Besides avoiding the interaction
> between -g and warnings it also seems to improve the inlining context
> by including more inlined call sites.  This can be seen in the adjusted
> tests.  (Its effect on PR 98465 is that the false positive is issued
> consistently, regardless of -g.  Avoiding the false positive is my
> next step.)
>
> Jakub, you raised a concern yesterday in PR 98465 c#13 about the memory
> footprint of this change.  Can you please comment on whether it's in
> line with what you were suggesting?

     {
       tree ao = BLOCK_ABSTRACT_ORIGIN (block);
-      if (TREE_CODE (ao) == FUNCTION_DECL)
-       loc = BLOCK_SOURCE_LOCATION (block);
-      else if (TREE_CODE (ao) != BLOCK)
-       break;
+       if (TREE_CODE (ao) == FUNCTION_DECL)
+        loc = BLOCK_SOURCE_LOCATION (block);
+       else if (TREE_CODE (ao) != BLOCK)
+        break;

you are replacing tabs with spaces?

@@ -558,16 +558,13 @@ remove_unused_scope_block_p (tree scope, bool
in_ctor_dtor_block)
    else if (!flag_auto_profile && debug_info_level == DINFO_LEVEL_NONE
            && !optinfo_wants_inlining_info_p ())
      {
-       /* Even for -g0 don't prune outer scopes from artificial
-         functions, otherwise diagnostics using tree_nonartificial_location
-         will not be emitted properly.  */
+       /* Even for -g0 don't prune outer scopes from inlined functions,
+         otherwise late diagnostics from such functions will not be
+         emitted or suppressed properly.  */
        if (inlined_function_outer_scope_p (scope))
         {
           tree ao = BLOCK_ORIGIN (scope);
-          if (ao
-              && TREE_CODE (ao) == FUNCTION_DECL
-              && DECL_DECLARED_INLINE_P (ao)
-              && lookup_attribute ("artificial", DECL_ATTRIBUTES (ao)))
+          if (ao && TREE_CODE (ao) == FUNCTION_DECL)
             unused = false;
         }
      }

so which inlined_function_outer_scope_p are you _not_ marking now?
BLOCK_ORIGIN is never NULL and all inlined scopes should have
an abstract origin - I believe always a FUNCTIN_DECL.  Which means
you could have simplified it further?

And yes, the main reason for the code above is memory use for
C++ with lots of inlining.  I suggest to try the patch on tramp3d
for example (there's about 10 inline instances per emitted
assembly op).

Richard.

> Martin