On Wed, 2 Dec 2020, Bernd Edlinger wrote:
> On 12/2/20 8:50 AM, Richard Biener wrote:
> > On Tue, 1 Dec 2020, Bernd Edlinger wrote:
> >
> >> Hi!
> >>
> >>
> >> This removes gimple_debug stmts without block info after a
> >> NULL INLINE_ENTRY.
> >>
> >> The line numbers from these stmts are from the inline function,
> >> but since the inline function is completely optimized away,
> >> there will be no DW_TAG_inlined_subroutine so the debugger has
> >> no callstack available at this point, and therefore those
> >> line table entries are not helpful to the user.
> >>
> >> 2020-11-20 Bernd Edlinger <bernd.edlin...@hotmail.de>
> >>
> >> * cfgexpand.c (expand_gimple_basic_block): Remove debug_begin_stmts
> >> following a removed debug_inline_entry.
> >>
> >>
> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> >> Is it OK for trunk?
> >
> > So are those visited by clear_unused_block_pointer? If so wouldn't
> > it be more appropriate to remove those there, when we elide the
> > inlined block scope?
> >
>
> That's what I thought initially, but that is only true for 99% of the
> inline statements. However 1% of the inline_entries without block info,
> and debug_begin_stmts without block info, that have line numbers from
> an inline header, do actually originate here:
>
> copy_debug_stmt (gdebug *stmt, copy_body_data *id)
> {
> tree t, *n;
> struct walk_stmt_info wi;
>
> if (tree block = gimple_block (stmt))
> {
> n = id->decl_map->get (block);
> gimple_set_block (stmt, n ? *n : id->block);
> }
>
> because id->block is NULL, and decl_map does not have
> an entry.
>
> So I tracked it down why that happens.
>
> I think remap_gimple_stmt should just drop those nonbind markers
> on the floor when the call statement has no block information.
>
> Once that is fixed, the special handling of inline entries without
> block info can as well be moved from remap_gimple_stmt to
> clear_unused_block_pointer.
>
> What do you think of this (not yet fully tested) patch?
>
> Is it OK when bootstrap and reg-testing passes?
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index d9814bd..e87c653 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -1819,7 +1819,8 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
/* If the inlined function has too many debug markers,
don't copy them. */
if (id->src_cfun->debug_marker_count
- > param_max_debug_marker_count)
+ > param_max_debug_marker_count
+ || !id->block)
return stmts;
Isn't this overly pessimistic in throwing away all debug markers
of an inline rather than just those which are associated with
the outermost scope (that's mapped to NULL if !id->block)? Can
we instead remap the block here (move it from copy_debug_stmt)
and elide the copy only when it maps to NULL?
gdebug *copy = as_a <gdebug *> (gimple_copy (stmt));
diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
index 21a9ee4..ca119c6 100644
--- a/gcc/tree-ssa-live.c
+++ b/gcc/tree-ssa-live.c
@@ -623,13 +623,25 @@ clear_unused_block_pointer (void)
{
unsigned i;
tree b;
- gimple *stmt = gsi_stmt (gsi);
+ gimple *stmt;
+ next:
+ stmt = gsi_stmt (gsi);
if (!is_gimple_debug (stmt) && !gimple_clobber_p (stmt))
continue;
b = gimple_block (stmt);
if (b && !TREE_USED (b))
- gimple_set_block (stmt, NULL);
+ {
+ if (gimple_debug_nonbind_marker_p (stmt)
+ && BLOCK_ABSTRACT_ORIGIN (b))
why only for inlined BLOCKs? Did you want to restrict it further
to inlined_function_outer_scope_p?
I guess I don't understand the debug situation fully - I guess it is
about jumping to locations in inlines where the call stack does
not show we are in the actual inlined function? But IIRC at least
unused BLOCK removal never elides the actual
inlined_function_outer_scope_p which would leave the inlining case
you spotted. But there we should zap all markers that belong to
the inlined function but not those which belong to another inline
instance? So we want to walk BLOCK_SUPERCONTEXT until we hit
an inlined_function_outer_scope_p where we don't want to zap
or the inlined functions outermost scope where we do want to zap
(if the call stmt has a NULL block)?
+ {
+ gsi_remove (&gsi, true);
+ if (gsi_end_p (gsi))
+ break;
>
> Thanks
> Bernd.
>
> > Thanks,
> > Richard.
> >
> >>
> >> Thanks
> >> Bernd.
> >>
> >
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
