On 12/7/20 11:50 AM, Richard Biener wrote:
> The ipa-param-manipulation.c hunk is OK, please commit separately.
>
done.
> The tree-inline.c and cfgexpand.c changes are OK as well, for the
> tree-ssa-live.c change see below
>
>
> From 85b0e37d0c0d3ecac4908ebbfd67edc612ef22b2 Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlin...@hotmail.de>
> Date: Wed, 2 Dec 2020 12:32:02 +0100
> Subject: [PATCH] Remove misleading debug line entries
>
> This removes gimple_debug_begin_stmts without block info which remain
> after a gimple block originating from an inline function is unused.
>
> 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-12-02 Bernd Edlinger <bernd.edlin...@hotmail.de>
>
> * cfgexpand.c (expand_gimple_basic_block): Remove special handling
> of debug_inline_entries without block info.
> * ipa-param-manipulation.c
> (ipa_param_body_adjustments::modify_call_stmt): Set location info.
> * tree-inline.c (remap_gimple_stmt): Drop debug_nonbind_markers when
> the call statement has no block info.
> (copy_debug_stmt): Remove debug_nonbind_markers when inlining
> and the block info is mapped to NULL.
> * tree-ssa-live.c (clear_unused_block_pointer): Remove
> debug_nonbind_markers originating from inlined functions.
> ---
> gcc/cfgexpand.c | 9 +--------
> gcc/ipa-param-manipulation.c | 2 ++
> gcc/tree-inline.c | 14 ++++++++++----
> gcc/tree-ssa-live.c | 22 ++++++++++++++++++++--
> 4 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 7e0bdd58e85..df7b62080b6 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -5953,14 +5953,7 @@ expand_gimple_basic_block (basic_block bb, bool
> disable_tail_calls)
> else if (gimple_debug_begin_stmt_p (stmt))
> val = GEN_RTX_DEBUG_MARKER_BEGIN_STMT_PAT ();
> else if (gimple_debug_inline_entry_p (stmt))
> - {
> - tree block = gimple_block (stmt);
> -
> - if (block)
> - val = GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT ();
> - else
> - goto delink_debug_stmt;
> - }
> + val = GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT ();
> else
> gcc_unreachable ();
>
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index 2bbea21be2e..9ab4a10096d 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -1681,6 +1681,8 @@ ipa_param_body_adjustments::modify_call_stmt (gcall
> **stmt_p)
> }
> }
> gcall *new_stmt = gimple_build_call_vec (gimple_call_fn (stmt), vargs);
> + if (gimple_has_location (stmt))
> + gimple_set_location (new_stmt, gimple_location (stmt));
> gimple_call_set_chain (new_stmt, gimple_call_chain (stmt));
> gimple_call_copy_flags (new_stmt, stmt);
> if (tree lhs = gimple_call_lhs (stmt))
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index d9814bd10d3..360b85f14dc 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -1819,12 +1819,11 @@ 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->reset_location)
> return stmts;
>
> gdebug *copy = as_a <gdebug *> (gimple_copy (stmt));
> - if (id->reset_location)
> - gimple_set_location (copy, input_location);
> id->debug_stmts.safe_push (copy);
> gimple_seq_add_stmt (&stmts, copy);
> return stmts;
> @@ -3169,7 +3168,14 @@ copy_debug_stmt (gdebug *stmt, copy_body_data *id)
> }
>
> if (gimple_debug_nonbind_marker_p (stmt))
> - return;
> + {
> + if (id->call_stmt && !gimple_block (stmt))
> + {
> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> + gsi_remove (&gsi, true);
> + }
> + return;
> + }
>
> /* Remap all the operands in COPY. */
> memset (&wi, 0, sizeof (wi));
> diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
> index 21a9ee43e6b..acba4a58626 100644
> --- a/gcc/tree-ssa-live.c
> +++ b/gcc/tree-ssa-live.c
> @@ -623,13 +623,31 @@ 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))
>
> so we have a non-bind marker that was inlined and its BLOCK is
> elided.
>
> + {
> + while (TREE_CODE (b) == BLOCK
> + && !inlined_function_outer_scope_p (b))
> + b = BLOCK_SUPERCONTEXT (b);
>
> we're looking for the inlined_function_outer_scope_p containing it
> and if we found one we remove the stmt instead of setting its
> block to zero.
>
> I'm still confused a bit here - a comment explaining what we do
> here would be much appreciated. Note my earlier comment about
I added a comment now, but I am not sure if it states just
the obvious, or if it needs further improvement.
... and while I was there, I realized that
I handle the debug_begin_stmt_p could be more conservative.
That is, if only a lexical block in an inlined subroutine is
elided - which is possible - there is no need to zap the
line entirely, just assign the inlined function's outer scope,
which is similar to what happens for debug_begin_stmt_p outside
of inline functions, where NULL is assigned as block.
> this hunk derailed into comments about how to handle the
> tree-inline.c case which might have confused you (but I think
> you found a nice solution there).
>
> That said, originally I wondered why not simply do the following,
> that is - what is special about not inlined debug markers with
> a NULL block?
>
actually the gimple_block of a gimple_debug_begin_stmt_p has no
significance at all, it's just is a note, to which function the line number
originally belongs to. BUT once we let it pass to the final stage of
compilation, the line numbers from debug-begin-stmt and ordinary
gimple_locations etc. from executable code line number do not have
any block structure anymore. The block structure is an entirely
different data structure, that consists of sets of byte-ranges
where code for a certain lexical block or inline block is located.
The line numbers from debug-begin-stmt are presented to the
user when the program stops at the given byte address. But
the when the block's debug_inline_entry_p is removed, for debug_inline_entry_p
it is the same, if it gets the block_info set to NULL or if it is actually
removed. But for debug_begin_stmt_p setting the block-info to NULL makes them
ordinary line numbers from the global scope.
If the debug_inline_entry_p is removed, not even an empty block range is left
in the resulting dwarf debug info, and in fact the complete
DW_TAG_inlined_subroutine
debug entry is removed, and now the line entry from the removed subroutine block
looks like it is from the outer scope now. Totally confusing when you step
into this line.
Attached is the new version of the patch, with the mentioned
improvement and a short comment at the clear_unused_block_pointer
function.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
> Richard.
>
> diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
> index 21a9ee43e6b..e9633da26ba 100644
> --- a/gcc/tree-ssa-live.c
> +++ b/gcc/tree-ssa-live.c
> @@ -623,13 +623,24 @@ 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))
> + {
> + gsi_remove (&gsi, true);
> + if (gsi_end_p (gsi))
> + break;
> + goto next;
> + }
> + gimple_set_block (stmt, NULL);
> + }
> for (i = 0; i < gimple_num_ops (stmt); i++)
> walk_tree (gimple_op_ptr (stmt, i),
> clear_unused_block_pointer_1,
> NULL, NULL);
>
From 8df88947587dfab04d715f5bb78e1a53119e51ad Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlin...@hotmail.de>
Date: Mon, 7 Dec 2020 12:00:00 +0100
Subject: [PATCH] Remove misleading debug line entries
This removes gimple_debug_begin_stmts without block info which remain
after a gimple block originating from an inline function is unused.
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-12-07 Bernd Edlinger <bernd.edlin...@hotmail.de>
* cfgexpand.c (expand_gimple_basic_block): Remove special handling
of debug_inline_entries without block info.
* tree-inline.c (remap_gimple_stmt): Drop debug_nonbind_markers when
the call statement has no block info.
(copy_debug_stmt): Remove debug_nonbind_markers when inlining
and the block info is mapped to NULL.
* tree-ssa-live.c (clear_unused_block_pointer): Remove
debug_nonbind_markers originating from inlined functions.
---
gcc/cfgexpand.c | 9 +--------
gcc/tree-inline.c | 14 ++++++++++----
gcc/tree-ssa-live.c | 29 +++++++++++++++++++++++++++--
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index f7b4091..2cbcbc8 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -5956,14 +5956,7 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
else if (gimple_debug_begin_stmt_p (stmt))
val = GEN_RTX_DEBUG_MARKER_BEGIN_STMT_PAT ();
else if (gimple_debug_inline_entry_p (stmt))
- {
- tree block = gimple_block (stmt);
-
- if (block)
- val = GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT ();
- else
- goto delink_debug_stmt;
- }
+ val = GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT ();
else
gcc_unreachable ();
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index d9814bd..360b85f 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -1819,12 +1819,11 @@ 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->reset_location)
return stmts;
gdebug *copy = as_a <gdebug *> (gimple_copy (stmt));
- if (id->reset_location)
- gimple_set_location (copy, input_location);
id->debug_stmts.safe_push (copy);
gimple_seq_add_stmt (&stmts, copy);
return stmts;
@@ -3169,7 +3168,14 @@ copy_debug_stmt (gdebug *stmt, copy_body_data *id)
}
if (gimple_debug_nonbind_marker_p (stmt))
- return;
+ {
+ if (id->call_stmt && !gimple_block (stmt))
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gsi_remove (&gsi, true);
+ }
+ return;
+ }
/* Remap all the operands in COPY. */
memset (&wi, 0, sizeof (wi));
diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
index 21a9ee4..9ea24a1 100644
--- a/gcc/tree-ssa-live.c
+++ b/gcc/tree-ssa-live.c
@@ -623,13 +623,38 @@ 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);
+ {
+ /* Remove a nonbind marker when the outer scope of the
+ inline function is completely removed. */
+ if (gimple_debug_nonbind_marker_p (stmt)
+ && BLOCK_ABSTRACT_ORIGIN (b))
+ {
+ while (TREE_CODE (b) == BLOCK
+ && !inlined_function_outer_scope_p (b))
+ b = BLOCK_SUPERCONTEXT (b);
+ if (TREE_CODE (b) == BLOCK)
+ {
+ if (TREE_USED (b))
+ {
+ gimple_set_block (stmt, b);
+ continue;
+ }
+ gsi_remove (&gsi, true);
+ if (gsi_end_p (gsi))
+ break;
+ goto next;
+ }
+ }
+ gimple_set_block (stmt, NULL);
+ }
for (i = 0; i < gimple_num_ops (stmt); i++)
walk_tree (gimple_op_ptr (stmt, i), clear_unused_block_pointer_1,
NULL, NULL);
--
1.9.1
