On Feb 26, 2015, Alexandre Oliva <aol...@redhat.com> wrote: > So far, all the differences I looked at were caused by padding at the > end of BBs, and by jump stmts without line numbers at the end of BBs, > both right after the debug reset stmts the proposed patch introduces.
Further investigation showed there were other sources of spurious differences: - copies arising from the expansion of PHI nodes; source code information associated with these copies points at the source of the copy, which is hardly useful and sensible. - "optimization" of single-range location lists to a location expression covering the entire function, which extends, often incorrectly, the range of the expression, and thus the coverage of the function. By patching GCC so as to eliminate these differences (patches attached for reference), I managed to reduce the coverage differences in libgcc_s.so to a manageable size (about a dozen variables in 3 functions in 2 object files), so I could manually investigate each one of them. All remaining differences amounted to single insns, originally before the reset debug stmt introduced by the patch, computing subexpressions of expressions expanded after the return point of the inlined function. Because of TER, this often causes stmts to be moved past corresponding debug stmts. This is a long-standing and long-known issue to me, thus nothing particularly new brought about or worsened by the proposed patch, and that would be fixed with stmt frontiers. Ultimately, the issue is not so much that the insn gets emitted at a different spot, but rather the possibility that no insn will remain at the point where it was expected, which might make it impossible (with current compiler and debugger) to inspect the results of that computation. The worst the reset debug stmts introduced by this patch could do is to make those effects not visible at other points of the computation, where they are not guaranteed or expected to be visible to begin with. That said, it might be nice if we emitted partial of full expansions of subexpressions at their original location, relative to debug stmts expanded to debug insns, rather than all at the point of the full expression. I recall having started something along these lines years ago, but that project never got to a working state. The differences in libstdc++.so, even after the patches intended to minimize differences, are too many to investigate in depth, but from what I can tell from a quick glance at the diff in dwlocstat per-variable per-range coverage dumps, nearly all of the differences amount to one insn in the final range, which likely means TER-introduced differences.
End scopes before location-less insns From: Alexandre Oliva <aol...@redhat.com> --- gcc/final.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/gcc/final.c b/gcc/final.c index 1fa93d9..6fab871 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -1634,13 +1634,19 @@ choose_inner_scope (tree s1, tree s2) /* Emit lexical block notes needed to change scope from S1 to S2. */ static void -change_scope (rtx_insn *orig_insn, tree s1, tree s2) +change_scope (rtx_insn *prev_insn, rtx_insn *orig_insn, tree s1, tree s2) { rtx_insn *insn = orig_insn; + rtx_insn *einsn; tree com = NULL_TREE; tree ts1 = s1, ts2 = s2; tree s; + if (!prev_insn) + einsn = insn; + else + einsn = NEXT_INSN (prev_insn); + while (ts1 != ts2) { gcc_assert (ts1 && ts2); @@ -1660,7 +1666,7 @@ change_scope (rtx_insn *orig_insn, tree s1, tree s2) s = s1; while (s != com) { - rtx_note *note = emit_note_before (NOTE_INSN_BLOCK_END, insn); + rtx_note *note = emit_note_before (NOTE_INSN_BLOCK_END, einsn); NOTE_BLOCK (note) = s; s = BLOCK_SUPERCONTEXT (s); } @@ -1684,6 +1690,7 @@ reemit_insn_block_notes (void) tree cur_block = DECL_INITIAL (cfun->decl); rtx_insn *insn; rtx_note *note; + rtx_insn *prev_insn = NULL; insn = get_insns (); for (; insn; insn = NEXT_INSN (insn)) @@ -1693,10 +1700,16 @@ reemit_insn_block_notes (void) /* Prevent lexical blocks from straddling section boundaries. */ if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS) { + rtx_insn *einsn; + if (prev_insn) + einsn = NEXT_INSN (prev_insn); + else + einsn = insn; + prev_insn = NULL; for (tree s = cur_block; s != DECL_INITIAL (cfun->decl); s = BLOCK_SUPERCONTEXT (s)) { - rtx_note *note = emit_note_before (NOTE_INSN_BLOCK_END, insn); + rtx_note *note = emit_note_before (NOTE_INSN_BLOCK_END, einsn); NOTE_BLOCK (note) = s; note = emit_note_after (NOTE_INSN_BLOCK_BEG, insn); NOTE_BLOCK (note) = s; @@ -1732,14 +1745,16 @@ reemit_insn_block_notes (void) if (this_block != cur_block) { - change_scope (insn, cur_block, this_block); + change_scope (prev_insn, insn, cur_block, this_block); cur_block = this_block; } + + prev_insn = insn; } /* change_scope emits before the insn, not after. */ note = emit_note (NOTE_INSN_DELETED); - change_scope (note, cur_block, DECL_INITIAL (cfun->decl)); + change_scope (NULL, note, cur_block, DECL_INITIAL (cfun->decl)); delete_insn (note); reorder_blocks ();
Don't assign locations to out-of-ssa copies. From: Alexandre Oliva <aol...@redhat.com> --- gcc/tree-outof-ssa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gcc/tree-outof-ssa.c b/gcc/tree-outof-ssa.c index e6310cd..7d15a9a 100644 --- a/gcc/tree-outof-ssa.c +++ b/gcc/tree-outof-ssa.c @@ -289,6 +289,7 @@ insert_partition_copy_on_edge (edge e, int dest, int src, source_location locus) /* If a locus is provided, override the default. */ if (locus) set_curr_insn_location (locus); + set_curr_insn_location (0); var = partition_to_var (SA.map, src); seq = emit_partition_copy (copy_rtx (SA.partition_to_pseudo[dest]), @@ -327,6 +328,7 @@ insert_value_copy_on_edge (edge e, int dest, tree src, source_location locus) /* If a locus is provided, override the default. */ if (locus) set_curr_insn_location (locus); + set_curr_insn_location (0); start_sequence (); @@ -382,6 +384,7 @@ insert_rtx_to_part_on_edge (edge e, int dest, rtx src, int unsignedsrcp, /* If a locus is provided, override the default. */ if (locus) set_curr_insn_location (locus); + set_curr_insn_location (0); /* We give the destination as sizeexp in case src/dest are BLKmode mems. Usually we give the source. As we result from SSA names @@ -418,6 +421,7 @@ insert_part_to_rtx_on_edge (edge e, rtx dest, int src, source_location locus) /* If a locus is provided, override the default. */ if (locus) set_curr_insn_location (locus); + set_curr_insn_location (0); var = partition_to_var (SA.map, src); seq = emit_partition_copy (dest,
do not optimize single-loc list From: Alexandre Oliva <aol...@redhat.com> --- gcc/dwarf2out.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index ebf41c8..308c070 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -14209,7 +14209,7 @@ dw_loc_list (var_loc_list *loc_list, tree decl, int want_address) representable, we don't want to pretend a single entry that was applies to the entire scope in which the variable is available. */ - if (list && loc_list->first->next) + if (list && (1 || loc_list->first->next)) gen_llsym (list); return list; @@ -16049,7 +16049,7 @@ add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p, a constant value. That way we are better to use add_const_value_attribute rather than expanding constant value equivalent. */ loc_list = lookup_decl_loc (decl); - if (loc_list + if (0 && loc_list && loc_list->first && loc_list->first->next == NULL && NOTE_P (loc_list->first->loc)
-- 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