| Message ID | 20171110023448.28164-9-aoliva@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SFN+LVU+IEPM,v4,1/9,SFN] adjust RTL insn-walking API | expand |
On Nov 10, 2017, Alexandre Oliva <aoliva@redhat.com> wrote:
> Output DW_AT_entry_pc based on markers.
Here's an updated version of the patch.
[IEPM] Introduce inline entry point markers
Output DW_AT_entry_pc based on markers.
Introduce DW_AT_GNU_entry_view as a DWARF extension.
If views are enabled are we're not in strict compliance mode, output
DW_AT_GNU_entry_view if it might be nonzero.
This patch depends on SFN and LVU patchsets, and on the IEPM patch that
introduces the inline_entry debug hook.
for include/ChangeLog
* dwarf2.def (DW_AT_GNU_entry_view): New.
for gcc/ChangeLog
* cfgexpand.c (expand_gimple_basic_block): Handle inline entry
markers.
* dwarf2out.c (dwarf2_debug_hooks): Enable inline_entry hook.
(BLOCK_INLINE_ENTRY_LABEL): New.
(dwarf2out_var_location): Disregard inline entry markers.
(inline_entry_data): New struct.
(inline_entry_data_hasher): New hashtable type.
(inline_entry_data_hasher::hash): New.
(inline_entry_data_hasher::equal): New.
(inline_entry_data_table): New variable.
(add_high_low_attributes): Add DW_AT_entry_pc and
DW_AT_GNU_entry_view attributes if a pending entry is found
in inline_entry_data_table. Add old entry_pc attribute only
if debug nonbinding markers are disabled.
(gen_inlined_subroutine_die): Set BLOCK_DIE if nonbinding
markers are enabled.
(block_within_block_p, dwarf2out_inline_entry): New.
(dwarf2out_finish): Check that no entries remained in
inline_entry_data_table.
* final.c (reemit_insn_block_notes): Handle inline entry notes.
(final_scan_insn, notice_source_line): Likewise.
(rest_of_clean_state): Skip inline entry markers.
* gimple-pretty-print.c (dump_gimple_debug): Handle inline entry
markers.
* gimple.c (gimple_build_debug_inline_entry): New.
* gimple.h (enum gimple_debug_subcode): Add
GIMPLE_DEBUG_INLINE_ENTRY.
(gimple_build_debug_inline_entry): Declare.
(gimple_debug_inline_entry_p): New.
(gimple_debug_nonbind_marker_p): Adjust.
* insn-notes.def (INLINE_ENTRY): New.
* print-rtl.c (rtx_writer::print_rtx_operand_code_0): Handle
inline entry marker notes.
(print_insn): Likewise.
* rtl.h (NOTE_MARKER_P): Add INLINE_ENTRY support.
(INSN_DEBUG_MARKER_KIND): Likewise.
(GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT): New.
* tree-inline.c (expand_call_inline): Build and insert
debug_inline_entry stmt.
* tree-ssa-live.c (remove_unused_scope_block_p): Preserve
inline entry blocks early, if nonbind markers are enabled.
(dump_scope_block): Dump fragment info.
* var-tracking.c (reemit_marker_as_note): Handle inline entry note.
* doc/gimple.texi (gimple_debug_inline_entry_p): New.
(gimple_build_debug_inline_entry): New.
* doc/invoke.texi (gstatement-frontiers, gno-statement-frontiers):
Enable/disable inline entry points too.
* doc/rtl.texi (NOTE_INSN_INLINE_ENTRY): New.
(DEBUG_INSN): Describe inline entry markers.
---
gcc/cfgexpand.c | 9 ++
gcc/doc/gimple.texi | 18 ++++
gcc/doc/rtl.texi | 24 ++++-
gcc/dwarf2out.c | 199 ++++++++++++++++++++++++++++++++++++++++++++-
gcc/final.c | 26 ++++++
gcc/gimple-pretty-print.c | 13 +++
gcc/gimple.c | 21 +++++
gcc/gimple.h | 18 ++++
gcc/insn-notes.def | 4 +
gcc/print-rtl.c | 5 +
gcc/rtl.h | 7 +-
gcc/tree-inline.c | 7 ++
gcc/tree-ssa-live.c | 27 +++++-
gcc/var-tracking.c | 1
include/dwarf2.def | 1
15 files changed, 364 insertions(+), 16 deletions(-)
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index ce98264214ae..23a061fe1c40 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -5713,6 +5713,15 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls)
goto delink_debug_stmt;
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;
+ }
else
gcc_unreachable ();
diff --git a/gcc/doc/gimple.texi b/gcc/doc/gimple.texi
index 492678705c6f..3825378080fa 100644
--- a/gcc/doc/gimple.texi
+++ b/gcc/doc/gimple.texi
@@ -836,6 +836,11 @@ Return true if g is a @code{GIMPLE_DEBUG} that marks the beginning of
a source statement.
@end deftypefn
+@deftypefn {GIMPLE function} gimple_debug_inline_entry_p (gimple g)
+Return true if g is a @code{GIMPLE_DEBUG} that marks the entry
+point of an inlined function.
+@end deftypefn
+
@deftypefn {GIMPLE function} gimple_debug_nonbind_marker_p (gimple g)
Return true if g is a @code{GIMPLE_DEBUG} that marks a program location,
without any variable binding.
@@ -1541,6 +1546,7 @@ Set the conditional @code{COND_STMT} to be of the form 'if (1 == 1)'.
@cindex @code{GIMPLE_DEBUG}
@cindex @code{GIMPLE_DEBUG_BIND}
@cindex @code{GIMPLE_DEBUG_BEGIN_STMT}
+@cindex @code{GIMPLE_DEBUG_INLINE_ENTRY}
@deftypefn {GIMPLE function} gdebug *gimple_build_debug_bind (tree var, @
tree value, gimple stmt)
@@ -1626,6 +1632,18 @@ observable, and that none of the side effects of subsequent user
statements are.
@end deftypefn
+@deftypefn {GIMPLE function} gimple gimple_build_debug_inline_entry (tree block, location_t location)
+Build a @code{GIMPLE_DEBUG} statement with
+@code{GIMPLE_DEBUG_INLINE_ENTRY} @code{subcode}. The effect of this
+statement is to tell debug information generation machinery that a
+function call at @code{location} underwent inline substitution, that
+@code{block} is the enclosing lexical block created for the
+substitution, and that at the point of the program in which the stmt is
+inserted, all parameters for the inlined function are bound to the
+respective arguments, and none of the side effects of its stmts are
+observable.
+@end deftypefn
+
@node @code{GIMPLE_EH_FILTER}
@subsection @code{GIMPLE_EH_FILTER}
@cindex @code{GIMPLE_EH_FILTER}
diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index dd3c0d3cfc1e..e219d4bdeacd 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -3465,7 +3465,10 @@ Refers to a parameter that was completely optimized out.
@item (debug_marker:@var{mode})
Marks a program location. With @code{VOIDmode}, it stands for the
beginning of a statement, a recommended inspection point logically after
-all prior side effects, and before any subsequent side effects.
+all prior side effects, and before any subsequent side effects. With
+@code{BLKmode}, it indicates an inline entry point: the lexical block
+encoded in the @code{INSN_LOCATION} is the enclosing block that encloses
+the inlined function.
@end table
@@ -3749,6 +3752,13 @@ This note is used to generate @code{is_stmt} markers in line number
debuggign information. It indicates the beginning of a user
statement.
+@findex NOTE_INSN_INLINE_ENTRY
+@item NOTE_INSN_INLINE_ENTRY
+This note is used to generate @code{entry_pc} for inlined subroutines in
+debugging information. It indicates an inspection point at which all
+arguments for the inlined function have been bound, and before its first
+statement.
+
@end table
These codes are printed symbolically when they appear in debugging dumps.
@@ -3766,8 +3776,12 @@ binds a user variable tree to an RTL representation of the
it stands for the value bound to the corresponding
@code{DEBUG_EXPR_DECL}.
-@code{GIMPLE_DEBUG_BEGIN_STMT} is expanded to RTL as a @code{DEBUG_INSN}
-with a @code{VOIDmode} @code{DEBUG_MARKER} @code{PATTERN}. These
+@code{GIMPLE_DEBUG_BEGIN_STMT} and @code{GIMPLE_DEBUG_INLINE_ENTRY} are
+expanded to RTL as a @code{DEBUG_INSN} with a @code{DEBUG_MARKER}
+@code{PATTERN}; the difference is the RTL mode: the former's
+@code{DEBUG_MARKER} is @code{VOIDmode}, whereas the latter is
+@code{BLKmode}; information about the inlined function can be taken from
+the lexical block encoded in the @code{INSN_LOCATION}. These
@code{DEBUG_INSN}s, that do not carry @code{VAR_LOCATION} information,
just @code{DEBUG_MARKER}s, can be detected by testing
@code{DEBUG_MARKER_INSN_P}, whereas those that do can be recognized as
@@ -3778,8 +3792,8 @@ with respect to each other, particularly during scheduling. Binding
information is kept in pseudo-instruction form, so that, unlike notes,
it gets the same treatment and adjustments that regular instructions
would. It is the variable tracking pass that turns these
-pseudo-instructions into @code{NOTE_INSN_VAR_LOCATION} and
-@code{NOTE_INSN_BEGIN_STMT} notes,
+pseudo-instructions into @code{NOTE_INSN_VAR_LOCATION},
+@code{NOTE_INSN_BEGIN_STMT} and @code{NOTE_INSN_INLINE_ENTRY} notes,
analyzing control flow, value equivalences and changes to registers and
memory referenced in value expressions, propagating the values of debug
temporaries and determining expressions that can be used to compute the
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index a88b4cdf6e25..df2f6d92c6fc 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2731,6 +2731,7 @@ static void dwarf2out_imported_module_or_decl_1 (tree, tree, tree,
dw_die_ref);
static void dwarf2out_abstract_function (tree);
static void dwarf2out_var_location (rtx_insn *);
+static void dwarf2out_inline_entry (tree);
static void dwarf2out_size_function (tree);
static void dwarf2out_begin_function (tree);
static void dwarf2out_end_function (unsigned int);
@@ -2784,7 +2785,7 @@ const struct gcc_debug_hooks dwarf2_debug_hooks =
debug_nothing_rtx_code_label, /* label */
debug_nothing_int, /* handle_pch */
dwarf2out_var_location,
- debug_nothing_tree, /* inline_entry */
+ dwarf2out_inline_entry, /* inline_entry */
dwarf2out_size_function, /* size_function */
dwarf2out_switch_text_section,
dwarf2out_set_name,
@@ -4025,6 +4026,9 @@ static char ranges_base_label[2 * MAX_ARTIFICIAL_LABEL_BYTES];
#ifndef BLOCK_BEGIN_LABEL
#define BLOCK_BEGIN_LABEL "LBB"
#endif
+#ifndef BLOCK_INLINE_ENTRY_LABEL
+#define BLOCK_INLINE_ENTRY_LABEL "LBI"
+#endif
#ifndef BLOCK_END_LABEL
#define BLOCK_END_LABEL "LBE"
#endif
@@ -23079,6 +23083,48 @@ block_die_hasher::equal (die_struct *x, die_struct *y)
return x->decl_id == y->decl_id && x->die_parent == y->die_parent;
}
+/* Hold information about markers for inlined entry points. */
+struct GTY ((for_user)) inline_entry_data
+{
+ /* The block that's the inlined_function_outer_scope for an inlined
+ function. */
+ tree block;
+
+ /* The label at the inlined entry point. */
+ const char *label_pfx;
+ unsigned int label_num;
+
+ /* The view number to be used as the inlined entry point. */
+ var_loc_view view;
+};
+
+struct inline_entry_data_hasher : ggc_ptr_hash <inline_entry_data>
+{
+ typedef tree compare_type;
+ static inline hashval_t hash (const inline_entry_data *);
+ static inline bool equal (const inline_entry_data *, const_tree);
+};
+
+/* Hash table routines for inline_entry_data. */
+
+inline hashval_t
+inline_entry_data_hasher::hash (const inline_entry_data *data)
+{
+ return htab_hash_pointer (data->block);
+}
+
+inline bool
+inline_entry_data_hasher::equal (const inline_entry_data *data,
+ const_tree block)
+{
+ return data->block == block;
+}
+
+/* Inlined entry points pending DIE creation in this compilation unit. */
+
+static GTY(()) hash_table<inline_entry_data_hasher> *inline_entry_data_table;
+
+
/* Return TRUE if DECL, which may have been previously generated as
OLD_DIE, is a candidate for a DW_AT_specification. DECLARATION is
true if decl (or its origin) is either an extern declaration or a
@@ -23533,6 +23579,42 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
{
char label[MAX_ARTIFICIAL_LABEL_BYTES];
+ if (inline_entry_data **iedp
+ = !inline_entry_data_table ? NULL
+ : inline_entry_data_table->find_slot_with_hash (stmt,
+ htab_hash_pointer (stmt),
+ NO_INSERT))
+ {
+ inline_entry_data *ied = *iedp;
+ gcc_assert (MAY_HAVE_DEBUG_MARKER_INSNS);
+ gcc_assert (inlined_function_outer_scope_p (stmt));
+ ASM_GENERATE_INTERNAL_LABEL (label, ied->label_pfx, ied->label_num);
+ add_AT_lbl_id (die, DW_AT_entry_pc, label);
+
+ if (debug_variable_location_views && !ZERO_VIEW_P (ied->view))
+ {
+ if (!output_asm_line_debug_info ())
+ add_AT_unsigned (die, DW_AT_GNU_entry_view, ied->view);
+ else
+ {
+ ASM_GENERATE_INTERNAL_LABEL (label, "LVU", ied->view);
+ /* FIXME: this will resolve to a small number. Could we
+ possibly emit smaller data? Ideally we'd emit a
+ uleb128, but that would make the size of DIEs
+ impossible for the compiler to compute, since it's
+ the assembler that computes the value of the view
+ label in this case. Ideally, we'd have a single form
+ encompassing both the address and the view, and
+ indirecting them through a table might make things
+ easier, but even that would be more wasteful,
+ space-wise, than what we have now. */
+ add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);
+ }
+ }
+
+ inline_entry_data_table->clear_slot (iedp);
+ }
+
if (BLOCK_FRAGMENT_CHAIN (stmt)
&& (dwarf_version >= 3 || !dwarf_strict))
{
@@ -23540,7 +23622,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
dw_die_ref pdie;
dw_attr_node *attr = NULL;
- if (inlined_function_outer_scope_p (stmt))
+ if (!MAY_HAVE_DEBUG_MARKER_INSNS && inlined_function_outer_scope_p (stmt))
{
ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_BEGIN_LABEL,
BLOCK_NUMBER (stmt));
@@ -23705,7 +23787,7 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die)
dw_die_ref subr_die
= new_die (DW_TAG_inlined_subroutine, context_die, stmt);
- if (call_arg_locations)
+ if (call_arg_locations || MAY_HAVE_DEBUG_MARKER_INSNS)
BLOCK_DIE (stmt) = subr_die;
add_abstract_origin_attribute (subr_die, decl);
if (TREE_ASM_WRITTEN (stmt))
@@ -26735,6 +26817,7 @@ dwarf2out_var_location (rtx_insn *loc_note)
|| ! NOTE_P (next_note)
|| (NOTE_KIND (next_note) != NOTE_INSN_VAR_LOCATION
&& NOTE_KIND (next_note) != NOTE_INSN_BEGIN_STMT
+ && NOTE_KIND (next_note) != NOTE_INSN_INLINE_ENTRY
&& NOTE_KIND (next_note) != NOTE_INSN_CALL_ARG_LOCATION))
next_note = NULL;
@@ -26923,6 +27006,113 @@ create_label:
last_in_cold_section_p = in_cold_section_p;
}
+/* Check whether BLOCK, a lexical block, is nested within OUTER, or is
+ OUTER itself. If BOTHWAYS, check not only that BLOCK can reach
+ OUTER through BLOCK_SUPERCONTEXT links, but also that there is a
+ path from OUTER to BLOCK through BLOCK_SUBBLOCKs and
+ BLOCK_FRAGMENT_ORIGIN links. */
+static bool
+block_within_block_p (tree block, tree outer, bool bothways)
+{
+ if (block == outer)
+ return true;
+
+ /* Quickly check that OUTER is up BLOCK's supercontext chain. */
+ for (tree context = BLOCK_SUPERCONTEXT (block);
+ context != outer;
+ context = BLOCK_SUPERCONTEXT (context))
+ if (!context || TREE_CODE (context) != BLOCK)
+ return false;
+
+ if (!bothways)
+ return true;
+
+ /* Now check that each block is actually referenced by its
+ parent. */
+ for (tree context = BLOCK_SUPERCONTEXT (block); ;
+ context = BLOCK_SUPERCONTEXT (context))
+ {
+ if (BLOCK_FRAGMENT_ORIGIN (context))
+ {
+ gcc_assert (!BLOCK_SUBBLOCKS (context));
+ context = BLOCK_FRAGMENT_ORIGIN (context);
+ }
+ for (tree sub = BLOCK_SUBBLOCKS (context);
+ sub != block;
+ sub = BLOCK_CHAIN (sub))
+ if (!sub)
+ return false;
+ if (context == outer)
+ return true;
+ else
+ block = context;
+ }
+}
+
+/* Called during final while assembling the marker of the entry point
+ for an inlined function. */
+
+static void
+dwarf2out_inline_entry (tree block)
+{
+ gcc_assert (DECL_P (block_ultimate_origin (block)));
+
+ /* Sanity check the block tree. This would catch a case in which
+ BLOCK got removed from the tree reachable from the outermost
+ lexical block, but got retained in markers. It would still link
+ back to its parents, but some ancestor would be missing a link
+ down the path to the sub BLOCK. If the block got removed, its
+ BLOCK_NUMBER will not be a usable value. */
+ gcc_checking_assert (block_within_block_p (block,
+ DECL_INITIAL
+ (current_function_decl),
+ true));
+
+ gcc_assert (inlined_function_outer_scope_p (block));
+ gcc_assert (!BLOCK_DIE (block));
+
+ /* If we can't represent it, don't bother. */
+ if (!(dwarf_version >= 3 || !dwarf_strict))
+ return;
+
+ if (BLOCK_FRAGMENT_ORIGIN (block))
+ block = BLOCK_FRAGMENT_ORIGIN (block);
+ /* Can the entry point ever not be at the beginning of an
+ unfragmented lexical block? */
+ else if (!(BLOCK_FRAGMENT_CHAIN (block)
+ || (cur_line_info_table
+ && !ZERO_VIEW_P (cur_line_info_table->view))))
+ return;
+
+ if (!inline_entry_data_table)
+ inline_entry_data_table
+ = hash_table<inline_entry_data_hasher>::create_ggc (10);
+
+
+ inline_entry_data **iedp
+ = inline_entry_data_table->find_slot_with_hash (block,
+ htab_hash_pointer (block),
+ INSERT);
+ if (*iedp)
+ /* ??? Ideally, we'd record all entry points for the same inlined
+ function (some may have been duplicated by e.g. unrolling), but
+ we have no way to represent that ATM. */
+ return;
+
+ inline_entry_data *ied = *iedp = ggc_cleared_alloc<inline_entry_data> ();
+ ied->block = block;
+ ied->label_pfx = BLOCK_INLINE_ENTRY_LABEL;
+ ied->label_num = BLOCK_NUMBER (block);
+ if (cur_line_info_table)
+ ied->view = cur_line_info_table->view;
+
+ char label[MAX_ARTIFICIAL_LABEL_BYTES];
+
+ ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_INLINE_ENTRY_LABEL,
+ BLOCK_NUMBER (block));
+ ASM_OUTPUT_LABEL (asm_out_file, label);
+}
+
/* Called from finalize_size_functions for size functions so that their body
can be encoded in the debug info to describe the layout of variable-length
structures. */
@@ -30401,6 +30591,9 @@ dwarf2out_finish (const char *)
/* Flush out any latecomers to the limbo party. */
flush_limbo_die_list ();
+ if (inline_entry_data_table)
+ gcc_assert (inline_entry_data_table->elements () == 0);
+
if (flag_checking)
{
verify_die (comp_unit_die ());
diff --git a/gcc/final.c b/gcc/final.c
index 72719c218e0c..6a8693d6b1f9 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1677,6 +1677,7 @@ reemit_insn_block_notes (void)
break;
case NOTE_INSN_BEGIN_STMT:
+ case NOTE_INSN_INLINE_ENTRY:
this_block = LOCATION_BLOCK (NOTE_MARKER_LOCATION (insn));
goto set_cur_block_to_this_block;
@@ -2543,6 +2544,7 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
if (!DECL_IGNORED_P (current_function_decl)
&& notice_source_line (insn, NULL))
{
+ output_source_line:
(*debug_hooks->source_line) (last_linenum, last_columnnum,
last_filename, last_discriminator,
true);
@@ -2550,6 +2552,18 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
}
break;
+ case NOTE_INSN_INLINE_ENTRY:
+ gcc_checking_assert (cfun->debug_nonbind_markers);
+ if (!DECL_IGNORED_P (current_function_decl))
+ {
+ if (!notice_source_line (insn, NULL))
+ break;
+ (*debug_hooks->inline_entry) (LOCATION_BLOCK
+ (NOTE_MARKER_LOCATION (insn)));
+ goto output_source_line;
+ }
+ break;
+
default:
gcc_unreachable ();
break;
@@ -3253,6 +3267,17 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
if (NOTE_MARKER_P (insn))
{
location_t loc = NOTE_MARKER_LOCATION (insn);
+ /* The inline entry markers (gimple, insn, note) carry the
+ location of the call, because that's what we want to carry
+ during compilation, but the location we want to output in
+ debug information for the inline entry point is the location
+ of the function itself. */
+ if (NOTE_KIND (insn) == NOTE_INSN_INLINE_ENTRY)
+ {
+ tree block = LOCATION_BLOCK (loc);
+ tree fn = block_ultimate_origin (block);
+ loc = DECL_SOURCE_LOCATION (fn);
+ }
expanded_location xloc = expand_location (loc);
if (xloc.line == 0)
{
@@ -4859,6 +4884,7 @@ rest_of_clean_state (void)
&& (!NOTE_P (insn) ||
(NOTE_KIND (insn) != NOTE_INSN_VAR_LOCATION
&& NOTE_KIND (insn) != NOTE_INSN_BEGIN_STMT
+ && NOTE_KIND (insn) != NOTE_INSN_INLINE_ENTRY
&& NOTE_KIND (insn) != NOTE_INSN_CALL_ARG_LOCATION
&& NOTE_KIND (insn) != NOTE_INSN_BLOCK_BEG
&& NOTE_KIND (insn) != NOTE_INSN_BLOCK_END
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index aee2ad8f1ea1..1a0551033c99 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -1370,6 +1370,19 @@ dump_gimple_debug (pretty_printer *buffer, gdebug *gs, int spc,
dump_gimple_fmt (buffer, spc, flags, "# DEBUG BEGIN_STMT");
break;
+ case GIMPLE_DEBUG_INLINE_ENTRY:
+ if (flags & TDF_RAW)
+ dump_gimple_fmt (buffer, spc, flags, "%G INLINE_ENTRY %T", gs,
+ gimple_block (gs)
+ ? block_ultimate_origin (gimple_block (gs))
+ : NULL_TREE);
+ else
+ dump_gimple_fmt (buffer, spc, flags, "# DEBUG INLINE_ENTRY %T",
+ gimple_block (gs)
+ ? block_ultimate_origin (gimple_block (gs))
+ : NULL_TREE);
+ break;
+
default:
gcc_unreachable ();
}
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 5a118e9095ca..be422467b11e 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -874,6 +874,27 @@ gimple_build_debug_begin_stmt (tree block, location_t location
}
+/* Build a new GIMPLE_DEBUG_INLINE_ENTRY statement in BLOCK at
+ LOCATION. The BLOCK links to the inlined function. */
+
+gdebug *
+gimple_build_debug_inline_entry (tree block, location_t location
+ MEM_STAT_DECL)
+{
+ gdebug *p
+ = as_a <gdebug *> (
+ gimple_build_with_ops_stat (GIMPLE_DEBUG,
+ (unsigned)GIMPLE_DEBUG_INLINE_ENTRY, 0
+ PASS_MEM_STAT));
+
+ gimple_set_location (p, location);
+ gimple_set_block (p, block);
+ cfun->debug_marker_count++;
+
+ return p;
+}
+
+
/* Build a GIMPLE_OMP_CRITICAL statement.
BODY is the sequence of statements for which only one thread can execute.
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 7c36679d092f..0406aedcf0e4 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -202,7 +202,8 @@ enum gf_mask {
enum gimple_debug_subcode {
GIMPLE_DEBUG_BIND = 0,
GIMPLE_DEBUG_SOURCE_BIND = 1,
- GIMPLE_DEBUG_BEGIN_STMT = 2
+ GIMPLE_DEBUG_BEGIN_STMT = 2,
+ GIMPLE_DEBUG_INLINE_ENTRY = 3
};
/* Masks for selecting a pass local flag (PLF) to work on. These
@@ -1454,6 +1455,7 @@ geh_dispatch *gimple_build_eh_dispatch (int);
gdebug *gimple_build_debug_bind (tree, tree, gimple * CXX_MEM_STAT_INFO);
gdebug *gimple_build_debug_source_bind (tree, tree, gimple * CXX_MEM_STAT_INFO);
gdebug *gimple_build_debug_begin_stmt (tree, location_t CXX_MEM_STAT_INFO);
+gdebug *gimple_build_debug_inline_entry (tree, location_t CXX_MEM_STAT_INFO);
gomp_critical *gimple_build_omp_critical (gimple_seq, tree, tree);
gomp_for *gimple_build_omp_for (gimple_seq, int, tree, size_t, gimple_seq);
gomp_parallel *gimple_build_omp_parallel (gimple_seq, tree, tree, tree);
@@ -4784,13 +4786,25 @@ gimple_debug_begin_stmt_p (const gimple *s)
return false;
}
+/* Return true if S is a GIMPLE_DEBUG INLINE_ENTRY statement. */
+
+static inline bool
+gimple_debug_inline_entry_p (const gimple *s)
+{
+ if (is_gimple_debug (s))
+ return s->subcode == GIMPLE_DEBUG_INLINE_ENTRY;
+
+ return false;
+}
+
/* Return true if S is a GIMPLE_DEBUG non-binding marker statement. */
static inline bool
gimple_debug_nonbind_marker_p (const gimple *s)
{
if (is_gimple_debug (s))
- return s->subcode == GIMPLE_DEBUG_BEGIN_STMT;
+ return s->subcode == GIMPLE_DEBUG_BEGIN_STMT
+ || s->subcode == GIMPLE_DEBUG_INLINE_ENTRY;
return false;
}
diff --git a/gcc/insn-notes.def b/gcc/insn-notes.def
index 960487b31db1..252e95722447 100644
--- a/gcc/insn-notes.def
+++ b/gcc/insn-notes.def
@@ -71,6 +71,10 @@ INSN_NOTE (CALL_ARG_LOCATION)
/* The beginning of a statement. */
INSN_NOTE (BEGIN_STMT)
+/* The entry point for an inlined function. Its NOTE_BLOCK references
+ the lexical block whose abstract origin is the inlined function. */
+INSN_NOTE (INLINE_ENTRY)
+
/* Record the struct for the following basic block. Uses
NOTE_BASIC_BLOCK. FIXME: Redundant with the basic block pointer
now included in every insn. NOTE: If there's no CFG anymore, in other words,
diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 0af739da0be6..af9448de6e43 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -259,6 +259,7 @@ rtx_writer::print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED,
break;
case NOTE_INSN_BEGIN_STMT:
+ case NOTE_INSN_INLINE_ENTRY:
#ifndef GENERATOR_FILE
{
expanded_location xloc
@@ -1826,6 +1827,10 @@ print_insn (pretty_printer *pp, const rtx_insn *x, int verbose)
pp_string (pp, "debug begin stmt marker");
break;
+ case NOTE_INSN_INLINE_ENTRY:
+ pp_string (pp, "debug inline entry marker");
+ break;
+
default:
gcc_unreachable ();
}
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 3ef687e5a371..63df02e0376a 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1625,7 +1625,8 @@ extern const char * const reg_note_name[];
for which NOTE_MARKER_LOCATION can be used. */
#define NOTE_MARKER_P(INSN) \
(NOTE_P (INSN) && \
- (NOTE_KIND (INSN) == NOTE_INSN_BEGIN_STMT))
+ (NOTE_KIND (INSN) == NOTE_INSN_BEGIN_STMT \
+ || NOTE_KIND (INSN) == NOTE_INSN_INLINE_ENTRY))
/* Variable declaration and the location of a variable. */
#define PAT_VAR_LOCATION_DECL(PAT) (XCTREE ((PAT), 0, VAR_LOCATION))
@@ -1663,6 +1664,8 @@ extern const char * const reg_note_name[];
(GET_CODE (PATTERN (INSN)) == DEBUG_MARKER \
? (GET_MODE (PATTERN (INSN)) == VOIDmode \
? NOTE_INSN_BEGIN_STMT \
+ : GET_MODE (PATTERN (INSN)) == BLKmode \
+ ? NOTE_INSN_INLINE_ENTRY \
: (enum insn_note)-1) \
: (enum insn_note)-1)
/* Create patterns for debug markers. These and the above abstract
@@ -1672,6 +1675,8 @@ extern const char * const reg_note_name[];
wouldn't be a problem. */
#define GEN_RTX_DEBUG_MARKER_BEGIN_STMT_PAT() \
gen_rtx_DEBUG_MARKER (VOIDmode)
+#define GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT() \
+ gen_rtx_DEBUG_MARKER (BLKmode)
/* The VAR_LOCATION rtx in a DEBUG_INSN. */
#define INSN_VAR_LOCATION(INSN) \
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 8604ba14a00a..f312b50ce57c 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4602,6 +4602,13 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id)
GSI_NEW_STMT);
}
initialize_inlined_parameters (id, stmt, fn, bb);
+ if (debug_nonbind_markers_p && id->block
+ && inlined_function_outer_scope_p (id->block))
+ {
+ gimple_stmt_iterator si = gsi_last_bb (bb);
+ gsi_insert_after (&si, gimple_build_debug_inline_entry
+ (id->block, input_location), GSI_NEW_STMT);
+ }
if (DECL_INITIAL (fn))
{
diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
index 8738fe21a6e1..734d15df2c51 100644
--- 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;
else
/* Verfify that only blocks with source location set
@@ -640,6 +647,16 @@ dump_scope_block (FILE *file, int indent, tree scope, dump_flags_t flags)
fprintf (file, "#%i", BLOCK_NUMBER (origin));
}
}
+ if (BLOCK_FRAGMENT_ORIGIN (scope))
+ fprintf (file, " Fragment of : #%i",
+ BLOCK_NUMBER (BLOCK_FRAGMENT_ORIGIN (scope)));
+ else if (BLOCK_FRAGMENT_CHAIN (scope))
+ {
+ fprintf (file, " Fragment chain :");
+ for (t = BLOCK_FRAGMENT_CHAIN (scope); t ;
+ t = BLOCK_FRAGMENT_CHAIN (t))
+ fprintf (file, " #%i", BLOCK_NUMBER (t));
+ }
fprintf (file, " \n");
for (var = BLOCK_VARS (scope); var; var = DECL_CHAIN (var))
{
diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 8e500b144712..4f8f9f182203 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -9938,6 +9938,7 @@ reemit_marker_as_note (rtx_insn *insn, basic_block *bb)
switch (kind)
{
case NOTE_INSN_BEGIN_STMT:
+ case NOTE_INSN_INLINE_ENTRY:
{
rtx_insn *note = NULL;
if (cfun->debug_nonbind_markers)
diff --git a/include/dwarf2.def b/include/dwarf2.def
index f3fa4009207b..8e731a3b87a6 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -444,6 +444,7 @@ DW_AT (DW_AT_GNU_pubtypes, 0x2135)
See http://gcc.gnu.org/wiki/Discriminator */
DW_AT (DW_AT_GNU_discriminator, 0x2136)
DW_AT (DW_AT_GNU_locviews, 0x2137)
+DW_AT (DW_AT_GNU_entry_view, 0x2138)
/* VMS extensions. */
DW_AT (DW_AT_VMS_rtnbeg_pd_address, 0x2201)
/* GNAT extensions. */
On 12/11/2017 07:54 PM, Alexandre Oliva wrote: > On Nov 10, 2017, Alexandre Oliva <aoliva@redhat.com> wrote: > >> Output DW_AT_entry_pc based on markers. > > Here's an updated version of the patch. > > [IEPM] Introduce inline entry point markers > > Output DW_AT_entry_pc based on markers. > > Introduce DW_AT_GNU_entry_view as a DWARF extension. > > If views are enabled are we're not in strict compliance mode, output > DW_AT_GNU_entry_view if it might be nonzero. > > This patch depends on SFN and LVU patchsets, and on the IEPM patch that > introduces the inline_entry debug hook. > > for include/ChangeLog > > * dwarf2.def (DW_AT_GNU_entry_view): New. > > for gcc/ChangeLog > > * cfgexpand.c (expand_gimple_basic_block): Handle inline entry > markers. > * dwarf2out.c (dwarf2_debug_hooks): Enable inline_entry hook. > (BLOCK_INLINE_ENTRY_LABEL): New. > (dwarf2out_var_location): Disregard inline entry markers. > (inline_entry_data): New struct. > (inline_entry_data_hasher): New hashtable type. > (inline_entry_data_hasher::hash): New. > (inline_entry_data_hasher::equal): New. > (inline_entry_data_table): New variable. > (add_high_low_attributes): Add DW_AT_entry_pc and > DW_AT_GNU_entry_view attributes if a pending entry is found > in inline_entry_data_table. Add old entry_pc attribute only > if debug nonbinding markers are disabled. > (gen_inlined_subroutine_die): Set BLOCK_DIE if nonbinding > markers are enabled. > (block_within_block_p, dwarf2out_inline_entry): New. > (dwarf2out_finish): Check that no entries remained in > inline_entry_data_table. > * final.c (reemit_insn_block_notes): Handle inline entry notes. > (final_scan_insn, notice_source_line): Likewise. > (rest_of_clean_state): Skip inline entry markers. > * gimple-pretty-print.c (dump_gimple_debug): Handle inline entry > markers. > * gimple.c (gimple_build_debug_inline_entry): New. > * gimple.h (enum gimple_debug_subcode): Add > GIMPLE_DEBUG_INLINE_ENTRY. > (gimple_build_debug_inline_entry): Declare. > (gimple_debug_inline_entry_p): New. > (gimple_debug_nonbind_marker_p): Adjust. > * insn-notes.def (INLINE_ENTRY): New. > * print-rtl.c (rtx_writer::print_rtx_operand_code_0): Handle > inline entry marker notes. > (print_insn): Likewise. > * rtl.h (NOTE_MARKER_P): Add INLINE_ENTRY support. > (INSN_DEBUG_MARKER_KIND): Likewise. > (GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT): New. > * tree-inline.c (expand_call_inline): Build and insert > debug_inline_entry stmt. > * tree-ssa-live.c (remove_unused_scope_block_p): Preserve > inline entry blocks early, if nonbind markers are enabled. > (dump_scope_block): Dump fragment info. > * var-tracking.c (reemit_marker_as_note): Handle inline entry note. > * doc/gimple.texi (gimple_debug_inline_entry_p): New. > (gimple_build_debug_inline_entry): New. > * doc/invoke.texi (gstatement-frontiers, gno-statement-frontiers): > Enable/disable inline entry points too. > * doc/rtl.texi (NOTE_INSN_INLINE_ENTRY): New. > (DEBUG_INSN): Describe inline entry markers. > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index a88b4cdf6e25..df2f6d92c6fc 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -23533,6 +23579,42 @@ add_high_low_attributes (tree stmt, dw_die_ref die) > { > char label[MAX_ARTIFICIAL_LABEL_BYTES]; > > + if (inline_entry_data **iedp > + = !inline_entry_data_table ? NULL > + : inline_entry_data_table->find_slot_with_hash (stmt, > + htab_hash_pointer (stmt), > + NO_INSERT)) > + { > + inline_entry_data *ied = *iedp; > + gcc_assert (MAY_HAVE_DEBUG_MARKER_INSNS); > + gcc_assert (inlined_function_outer_scope_p (stmt)); > + ASM_GENERATE_INTERNAL_LABEL (label, ied->label_pfx, ied->label_num); > + add_AT_lbl_id (die, DW_AT_entry_pc, label); > + > + if (debug_variable_location_views && !ZERO_VIEW_P (ied->view)) > + { > + if (!output_asm_line_debug_info ()) > + add_AT_unsigned (die, DW_AT_GNU_entry_view, ied->view); > + else > + { > + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", ied->view); > + /* FIXME: this will resolve to a small number. Could we > + possibly emit smaller data? Ideally we'd emit a > + uleb128, but that would make the size of DIEs > + impossible for the compiler to compute, since it's > + the assembler that computes the value of the view > + label in this case. Ideally, we'd have a single form > + encompassing both the address and the view, and > + indirecting them through a table might make things > + easier, but even that would be more wasteful, > + space-wise, than what we have now. */ > + add_AT_lbl_id (die, DW_AT_GNU_entry_view, label); Do you have a sense of whether or not this really matters in practice? how much bigger do we get due when we enable LVU? In general I'm more concerned with getting the data we need into the dwarf2 records than I am with optimizing its size. I'm not saying we should ignore the size, but > + > +/* Called during final while assembling the marker of the entry point > + for an inlined function. */ > + > +static void > +dwarf2out_inline_entry (tree block) > +{ > + gcc_assert (DECL_P (block_ultimate_origin (block))); > + > + /* Sanity check the block tree. This would catch a case in which > + BLOCK got removed from the tree reachable from the outermost > + lexical block, but got retained in markers. It would still link > + back to its parents, but some ancestor would be missing a link > + down the path to the sub BLOCK. If the block got removed, its > + BLOCK_NUMBER will not be a usable value. */ > + gcc_checking_assert (block_within_block_p (block, > + DECL_INITIAL > + (current_function_decl), > + true)); > + > + gcc_assert (inlined_function_outer_scope_p (block)); > + gcc_assert (!BLOCK_DIE (block)); > + > + /* If we can't represent it, don't bother. */ > + if (!(dwarf_version >= 3 || !dwarf_strict)) > + return; Consider moving this check earlier. I don't think it's a hard requirement, so if you put it after the asserts for a reason, then leave it has is. Generally I think this is fine (it's much simpler than the dwarf2 bits of the LVU patch. Jeff
On Dec 21, 2017, Jeff Law <law@redhat.com> wrote: > On 12/11/2017 07:54 PM, Alexandre Oliva wrote: >> + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", ied->view); >> + /* FIXME: this will resolve to a small number. Could we >> + possibly emit smaller data? Ideally we'd emit a >> + uleb128, but that would make the size of DIEs >> + impossible for the compiler to compute, since it's >> + the assembler that computes the value of the view >> + label in this case. Ideally, we'd have a single form >> + encompassing both the address and the view, and >> + indirecting them through a table might make things >> + easier, but even that would be more wasteful, >> + space-wise, than what we have now. */ >> + add_AT_lbl_id (die, DW_AT_GNU_entry_view, label); > Do you have a sense of whether or not this really matters in practice? > how much bigger do we get due when we enable LVU? It's more of a matter of general design principle. DWARF strives to be compact, and outputting a very-likely small constant with most bits known to be zero is kind of against the rules. LVU proper doesn't run into this because the location view numbers are emitted as uleb128 constants in location list sections, never in the main debug section. How much it really grows depends on the representation: DWARF>5 loclists only get additional entries when at least one view number in a range pair is nonzero, and I have a sense that most view numbers in loclists are zero; for DWARF[2,5], we emit list of pairs corresponding to the ranges in each entry in the actual loclist, so we spend at least two bytes for both views, plus the pointer to the locviewlist in an additional attribute. Considering each range amounts to a pair of pointers plus at least two bytes for the location expression, and that each range gets a corresponding pair of views, and assuming all views will fit in a single uleb128 byte (128+ views at the same PC are very unlikely), the loclist section would grow by at most 20% with 32-bit pointers, and about half that much with 64-bit pointers. In reality, it ought to be a lot less than that, since location expressions's taking up a single byte (plus another byte representing the size) are common but hardly a majority of the cases. >> + /* Sanity check the block tree. This would catch a case in which >> + BLOCK got removed from the tree reachable from the outermost >> + lexical block, but got retained in markers. It would still link >> + back to its parents, but some ancestor would be missing a link >> + down the path to the sub BLOCK. If the block got removed, its >> + BLOCK_NUMBER will not be a usable value. */ >> + gcc_checking_assert (block_within_block_p (block, >> + DECL_INITIAL >> + (current_function_decl), >> + true)); >> + >> + gcc_assert (inlined_function_outer_scope_p (block)); >> + gcc_assert (!BLOCK_DIE (block)); >> + >> + /* If we can't represent it, don't bother. */ >> + if (!(dwarf_version >= 3 || !dwarf_strict)) >> + return; > Consider moving this check earlier. I don't think it's a hard > requirement, so if you put it after the asserts for a reason, then leave > it has is. The reason I put the check after the asserts was that I wanted to catch messed up block trees even if we wouldn't emit the entry point after all. Now, considering the block tree messing up was figured out, I guess moving the test earlier and saving the cycles if we're not emitting the annotation makes sense. Will do. > Generally I think this is fine (it's much simpler than the dwarf2 bits > of the LVU patch. Yeah. I wonder if there's anything I can do, now that I'm back from vacations, to help get the LVU patch reviewed. I suppose it might still be eligible for GCC 8, considering it was posted even before stage3, let alone stage4, and considering it only deals with debug info (mainly location lists), but then... unlike the SFN stuff, the additional information it outputs will only make a difference once debug info consumers learn to use it. So maybe we could afford to leave it out, or to bring it in but disabled. Thoughts? Does it make sense at this point for me to pester ^W entice some review to have a look at it? Thanks,
On Tue, Dec 12, 2017 at 12:54:13AM -0200, Alexandre Oliva wrote: > +/* Check whether BLOCK, a lexical block, is nested within OUTER, or is > + OUTER itself. If BOTHWAYS, check not only that BLOCK can reach > + OUTER through BLOCK_SUPERCONTEXT links, but also that there is a > + path from OUTER to BLOCK through BLOCK_SUBBLOCKs and > + BLOCK_FRAGMENT_ORIGIN links. */ > +static bool > +block_within_block_p (tree block, tree outer, bool bothways) > +{ > + if (block == outer) > + return true; > + > + /* Quickly check that OUTER is up BLOCK's supercontext chain. */ > + for (tree context = BLOCK_SUPERCONTEXT (block); > + context != outer; > + context = BLOCK_SUPERCONTEXT (context)) > + if (!context || TREE_CODE (context) != BLOCK) > + return false; > + > + if (!bothways) > + return true; > + > + /* Now check that each block is actually referenced by its > + parent. */ > + for (tree context = BLOCK_SUPERCONTEXT (block); ; > + context = BLOCK_SUPERCONTEXT (context)) > + { > + if (BLOCK_FRAGMENT_ORIGIN (context)) > + { > + gcc_assert (!BLOCK_SUBBLOCKS (context)); > + context = BLOCK_FRAGMENT_ORIGIN (context); > + } > + for (tree sub = BLOCK_SUBBLOCKS (context); > + sub != block; > + sub = BLOCK_CHAIN (sub)) > + if (!sub) > + return false; > + if (context == outer) > + return true; > + else > + block = context; > + } > +} > + > +/* Called during final while assembling the marker of the entry point > + for an inlined function. */ > + > +static void > +dwarf2out_inline_entry (tree block) > +{ > + gcc_assert (DECL_P (block_ultimate_origin (block))); > + > + /* Sanity check the block tree. This would catch a case in which > + BLOCK got removed from the tree reachable from the outermost > + lexical block, but got retained in markers. It would still link > + back to its parents, but some ancestor would be missing a link > + down the path to the sub BLOCK. If the block got removed, its > + BLOCK_NUMBER will not be a usable value. */ > + gcc_checking_assert (block_within_block_p (block, > + DECL_INITIAL > + (current_function_decl), > + true)); I think this asks for if (flag_checking) gcc_assert (block_within_block_p (block, DECL_INITIAL (current_function_decl), true)); > --- 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. Otherwise the patch looks reasonable to me, but I think it depends on the 7/9. Jakub
On Jan 24, 2018, Jakub Jelinek <jakub@redhat.com> wrote: > I think this asks for > if (flag_checking) > gcc_assert (block_within_block_p (block, > DECL_INITIAL (current_function_decl), > true)); 'k, changed. > Otherwise the patch looks reasonable to me, but I think it depends on the > 7/9. Thanks, yeah, it very much does. It *might* be possible to split out the dependency, but... it would just take most of the LVU patch with it ;-)
On Jan 25, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > On Jan 24, 2018, Jakub Jelinek <jakub@redhat.com> wrote: >> I think this asks for >> if (flag_checking) >> gcc_assert (block_within_block_p (block, >> DECL_INITIAL (current_function_decl), >> true)); > 'k, changed. >> Otherwise the patch looks reasonable to me, but I think it depends on the >> 7/9. > Thanks, yeah, it very much does. It *might* be possible to split out > the dependency, but... it would just take most of the LVU patch with it > ;-) Here's what I checked in, right after the LVU patch. [IEPM] Introduce inline entry point markers Output DW_AT_entry_pc based on markers. Introduce DW_AT_GNU_entry_view as a DWARF extension. If views are enabled are we're not in strict compliance mode, output DW_AT_GNU_entry_view if it might be nonzero. This patch depends on SFN and LVU patchsets, and on the IEPM patch that introduces the inline_entry debug hook. for include/ChangeLog * dwarf2.def (DW_AT_GNU_entry_view): New. for gcc/ChangeLog * cfgexpand.c (expand_gimple_basic_block): Handle inline entry markers. * dwarf2out.c (dwarf2_debug_hooks): Enable inline_entry hook. (BLOCK_INLINE_ENTRY_LABEL): New. (dwarf2out_var_location): Disregard inline entry markers. (inline_entry_data): New struct. (inline_entry_data_hasher): New hashtable type. (inline_entry_data_hasher::hash): New. (inline_entry_data_hasher::equal): New. (inline_entry_data_table): New variable. (add_high_low_attributes): Add DW_AT_entry_pc and DW_AT_GNU_entry_view attributes if a pending entry is found in inline_entry_data_table. Add old entry_pc attribute only if debug nonbinding markers are disabled. (gen_inlined_subroutine_die): Set BLOCK_DIE if nonbinding markers are enabled. (block_within_block_p, dwarf2out_inline_entry): New. (dwarf2out_finish): Check that no entries remained in inline_entry_data_table. * final.c (reemit_insn_block_notes): Handle inline entry notes. (final_scan_insn, notice_source_line): Likewise. (rest_of_clean_state): Skip inline entry markers. * gimple-pretty-print.c (dump_gimple_debug): Handle inline entry markers. * gimple.c (gimple_build_debug_inline_entry): New. * gimple.h (enum gimple_debug_subcode): Add GIMPLE_DEBUG_INLINE_ENTRY. (gimple_build_debug_inline_entry): Declare. (gimple_debug_inline_entry_p): New. (gimple_debug_nonbind_marker_p): Adjust. * insn-notes.def (INLINE_ENTRY): New. * print-rtl.c (rtx_writer::print_rtx_operand_code_0): Handle inline entry marker notes. (print_insn): Likewise. * rtl.h (NOTE_MARKER_P): Add INLINE_ENTRY support. (INSN_DEBUG_MARKER_KIND): Likewise. (GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT): New. * tree-inline.c (expand_call_inline): Build and insert debug_inline_entry stmt. * tree-ssa-live.c (remove_unused_scope_block_p): Preserve inline entry blocks early, if nonbind markers are enabled. (dump_scope_block): Dump fragment info. * var-tracking.c (reemit_marker_as_note): Handle inline entry note. * doc/gimple.texi (gimple_debug_inline_entry_p): New. (gimple_build_debug_inline_entry): New. * doc/invoke.texi (gstatement-frontiers, gno-statement-frontiers): Enable/disable inline entry points too. * doc/rtl.texi (NOTE_INSN_INLINE_ENTRY): New. (DEBUG_INSN): Describe inline entry markers. --- gcc/cfgexpand.c | 9 ++ gcc/doc/gimple.texi | 18 ++++ gcc/doc/rtl.texi | 24 ++++- gcc/dwarf2out.c | 199 ++++++++++++++++++++++++++++++++++++++++++++- gcc/final.c | 26 ++++++ gcc/gimple-pretty-print.c | 13 +++ gcc/gimple.c | 21 +++++ gcc/gimple.h | 18 ++++ gcc/insn-notes.def | 4 + gcc/print-rtl.c | 5 + gcc/rtl.h | 7 +- gcc/tree-inline.c | 7 ++ gcc/tree-ssa-live.c | 27 +++++- gcc/var-tracking.c | 1 include/dwarf2.def | 1 15 files changed, 364 insertions(+), 16 deletions(-) diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 2ee6fbac2e30..deab9296001a 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5731,6 +5731,15 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) goto delink_debug_stmt; 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; + } else gcc_unreachable (); diff --git a/gcc/doc/gimple.texi b/gcc/doc/gimple.texi index 6d920aeb9675..1f9449f133bd 100644 --- a/gcc/doc/gimple.texi +++ b/gcc/doc/gimple.texi @@ -836,6 +836,11 @@ Return true if g is a @code{GIMPLE_DEBUG} that marks the beginning of a source statement. @end deftypefn +@deftypefn {GIMPLE function} gimple_debug_inline_entry_p (gimple g) +Return true if g is a @code{GIMPLE_DEBUG} that marks the entry +point of an inlined function. +@end deftypefn + @deftypefn {GIMPLE function} gimple_debug_nonbind_marker_p (gimple g) Return true if g is a @code{GIMPLE_DEBUG} that marks a program location, without any variable binding. @@ -1541,6 +1546,7 @@ Set the conditional @code{COND_STMT} to be of the form 'if (1 == 1)'. @cindex @code{GIMPLE_DEBUG} @cindex @code{GIMPLE_DEBUG_BIND} @cindex @code{GIMPLE_DEBUG_BEGIN_STMT} +@cindex @code{GIMPLE_DEBUG_INLINE_ENTRY} @deftypefn {GIMPLE function} gdebug *gimple_build_debug_bind (tree var, @ tree value, gimple stmt) @@ -1626,6 +1632,18 @@ observable, and that none of the side effects of subsequent user statements are. @end deftypefn +@deftypefn {GIMPLE function} gimple gimple_build_debug_inline_entry (tree block, location_t location) +Build a @code{GIMPLE_DEBUG} statement with +@code{GIMPLE_DEBUG_INLINE_ENTRY} @code{subcode}. The effect of this +statement is to tell debug information generation machinery that a +function call at @code{location} underwent inline substitution, that +@code{block} is the enclosing lexical block created for the +substitution, and that at the point of the program in which the stmt is +inserted, all parameters for the inlined function are bound to the +respective arguments, and none of the side effects of its stmts are +observable. +@end deftypefn + @node @code{GIMPLE_EH_FILTER} @subsection @code{GIMPLE_EH_FILTER} @cindex @code{GIMPLE_EH_FILTER} diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi index 2523f63c6738..43d5405ddb47 100644 --- a/gcc/doc/rtl.texi +++ b/gcc/doc/rtl.texi @@ -3670,7 +3670,10 @@ Refers to a parameter that was completely optimized out. @item (debug_marker:@var{mode}) Marks a program location. With @code{VOIDmode}, it stands for the beginning of a statement, a recommended inspection point logically after -all prior side effects, and before any subsequent side effects. +all prior side effects, and before any subsequent side effects. With +@code{BLKmode}, it indicates an inline entry point: the lexical block +encoded in the @code{INSN_LOCATION} is the enclosing block that encloses +the inlined function. @end table @@ -3954,6 +3957,13 @@ This note is used to generate @code{is_stmt} markers in line number debuggign information. It indicates the beginning of a user statement. +@findex NOTE_INSN_INLINE_ENTRY +@item NOTE_INSN_INLINE_ENTRY +This note is used to generate @code{entry_pc} for inlined subroutines in +debugging information. It indicates an inspection point at which all +arguments for the inlined function have been bound, and before its first +statement. + @end table These codes are printed symbolically when they appear in debugging dumps. @@ -3971,8 +3981,12 @@ binds a user variable tree to an RTL representation of the it stands for the value bound to the corresponding @code{DEBUG_EXPR_DECL}. -@code{GIMPLE_DEBUG_BEGIN_STMT} is expanded to RTL as a @code{DEBUG_INSN} -with a @code{VOIDmode} @code{DEBUG_MARKER} @code{PATTERN}. These +@code{GIMPLE_DEBUG_BEGIN_STMT} and @code{GIMPLE_DEBUG_INLINE_ENTRY} are +expanded to RTL as a @code{DEBUG_INSN} with a @code{DEBUG_MARKER} +@code{PATTERN}; the difference is the RTL mode: the former's +@code{DEBUG_MARKER} is @code{VOIDmode}, whereas the latter is +@code{BLKmode}; information about the inlined function can be taken from +the lexical block encoded in the @code{INSN_LOCATION}. These @code{DEBUG_INSN}s, that do not carry @code{VAR_LOCATION} information, just @code{DEBUG_MARKER}s, can be detected by testing @code{DEBUG_MARKER_INSN_P}, whereas those that do can be recognized as @@ -3983,8 +3997,8 @@ with respect to each other, particularly during scheduling. Binding information is kept in pseudo-instruction form, so that, unlike notes, it gets the same treatment and adjustments that regular instructions would. It is the variable tracking pass that turns these -pseudo-instructions into @code{NOTE_INSN_VAR_LOCATION} and -@code{NOTE_INSN_BEGIN_STMT} notes, +pseudo-instructions into @code{NOTE_INSN_VAR_LOCATION}, +@code{NOTE_INSN_BEGIN_STMT} and @code{NOTE_INSN_INLINE_ENTRY} notes, analyzing control flow, value equivalences and changes to registers and memory referenced in value expressions, propagating the values of debug temporaries and determining expressions that can be used to compute the diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 56d3e14b81bf..749c7e3b9bbc 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2747,6 +2747,7 @@ static void dwarf2out_imported_module_or_decl_1 (tree, tree, tree, dw_die_ref); static void dwarf2out_abstract_function (tree); static void dwarf2out_var_location (rtx_insn *); +static void dwarf2out_inline_entry (tree); static void dwarf2out_size_function (tree); static void dwarf2out_begin_function (tree); static void dwarf2out_end_function (unsigned int); @@ -2800,7 +2801,7 @@ const struct gcc_debug_hooks dwarf2_debug_hooks = debug_nothing_rtx_code_label, /* label */ debug_nothing_int, /* handle_pch */ dwarf2out_var_location, - debug_nothing_tree, /* inline_entry */ + dwarf2out_inline_entry, /* inline_entry */ dwarf2out_size_function, /* size_function */ dwarf2out_switch_text_section, dwarf2out_set_name, @@ -4068,6 +4069,9 @@ static char ranges_base_label[2 * MAX_ARTIFICIAL_LABEL_BYTES]; #ifndef BLOCK_BEGIN_LABEL #define BLOCK_BEGIN_LABEL "LBB" #endif +#ifndef BLOCK_INLINE_ENTRY_LABEL +#define BLOCK_INLINE_ENTRY_LABEL "LBI" +#endif #ifndef BLOCK_END_LABEL #define BLOCK_END_LABEL "LBE" #endif @@ -23215,6 +23219,48 @@ block_die_hasher::equal (die_struct *x, die_struct *y) return x->decl_id == y->decl_id && x->die_parent == y->die_parent; } +/* Hold information about markers for inlined entry points. */ +struct GTY ((for_user)) inline_entry_data +{ + /* The block that's the inlined_function_outer_scope for an inlined + function. */ + tree block; + + /* The label at the inlined entry point. */ + const char *label_pfx; + unsigned int label_num; + + /* The view number to be used as the inlined entry point. */ + var_loc_view view; +}; + +struct inline_entry_data_hasher : ggc_ptr_hash <inline_entry_data> +{ + typedef tree compare_type; + static inline hashval_t hash (const inline_entry_data *); + static inline bool equal (const inline_entry_data *, const_tree); +}; + +/* Hash table routines for inline_entry_data. */ + +inline hashval_t +inline_entry_data_hasher::hash (const inline_entry_data *data) +{ + return htab_hash_pointer (data->block); +} + +inline bool +inline_entry_data_hasher::equal (const inline_entry_data *data, + const_tree block) +{ + return data->block == block; +} + +/* Inlined entry points pending DIE creation in this compilation unit. */ + +static GTY(()) hash_table<inline_entry_data_hasher> *inline_entry_data_table; + + /* Return TRUE if DECL, which may have been previously generated as OLD_DIE, is a candidate for a DW_AT_specification. DECLARATION is true if decl (or its origin) is either an extern declaration or a @@ -23667,6 +23713,42 @@ add_high_low_attributes (tree stmt, dw_die_ref die) { char label[MAX_ARTIFICIAL_LABEL_BYTES]; + if (inline_entry_data **iedp + = !inline_entry_data_table ? NULL + : inline_entry_data_table->find_slot_with_hash (stmt, + htab_hash_pointer (stmt), + NO_INSERT)) + { + inline_entry_data *ied = *iedp; + gcc_assert (MAY_HAVE_DEBUG_MARKER_INSNS); + gcc_assert (inlined_function_outer_scope_p (stmt)); + ASM_GENERATE_INTERNAL_LABEL (label, ied->label_pfx, ied->label_num); + add_AT_lbl_id (die, DW_AT_entry_pc, label); + + if (debug_variable_location_views && !ZERO_VIEW_P (ied->view)) + { + if (!output_asm_line_debug_info ()) + add_AT_unsigned (die, DW_AT_GNU_entry_view, ied->view); + else + { + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", ied->view); + /* FIXME: this will resolve to a small number. Could we + possibly emit smaller data? Ideally we'd emit a + uleb128, but that would make the size of DIEs + impossible for the compiler to compute, since it's + the assembler that computes the value of the view + label in this case. Ideally, we'd have a single form + encompassing both the address and the view, and + indirecting them through a table might make things + easier, but even that would be more wasteful, + space-wise, than what we have now. */ + add_AT_lbl_id (die, DW_AT_GNU_entry_view, label); + } + } + + inline_entry_data_table->clear_slot (iedp); + } + if (BLOCK_FRAGMENT_CHAIN (stmt) && (dwarf_version >= 3 || !dwarf_strict)) { @@ -23674,7 +23756,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die) dw_die_ref pdie; dw_attr_node *attr = NULL; - if (inlined_function_outer_scope_p (stmt)) + if (!MAY_HAVE_DEBUG_MARKER_INSNS && inlined_function_outer_scope_p (stmt)) { ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_BEGIN_LABEL, BLOCK_NUMBER (stmt)); @@ -23839,7 +23921,7 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die) dw_die_ref subr_die = new_die (DW_TAG_inlined_subroutine, context_die, stmt); - if (call_arg_locations) + if (call_arg_locations || MAY_HAVE_DEBUG_MARKER_INSNS) BLOCK_DIE (stmt) = subr_die; add_abstract_origin_attribute (subr_die, decl); if (TREE_ASM_WRITTEN (stmt)) @@ -26871,6 +26953,7 @@ dwarf2out_var_location (rtx_insn *loc_note) || ! NOTE_P (next_note) || (NOTE_KIND (next_note) != NOTE_INSN_VAR_LOCATION && NOTE_KIND (next_note) != NOTE_INSN_BEGIN_STMT + && NOTE_KIND (next_note) != NOTE_INSN_INLINE_ENTRY && NOTE_KIND (next_note) != NOTE_INSN_CALL_ARG_LOCATION)) next_note = NULL; @@ -27064,6 +27147,113 @@ create_label: last_in_cold_section_p = in_cold_section_p; } +/* Check whether BLOCK, a lexical block, is nested within OUTER, or is + OUTER itself. If BOTHWAYS, check not only that BLOCK can reach + OUTER through BLOCK_SUPERCONTEXT links, but also that there is a + path from OUTER to BLOCK through BLOCK_SUBBLOCKs and + BLOCK_FRAGMENT_ORIGIN links. */ +static bool +block_within_block_p (tree block, tree outer, bool bothways) +{ + if (block == outer) + return true; + + /* Quickly check that OUTER is up BLOCK's supercontext chain. */ + for (tree context = BLOCK_SUPERCONTEXT (block); + context != outer; + context = BLOCK_SUPERCONTEXT (context)) + if (!context || TREE_CODE (context) != BLOCK) + return false; + + if (!bothways) + return true; + + /* Now check that each block is actually referenced by its + parent. */ + for (tree context = BLOCK_SUPERCONTEXT (block); ; + context = BLOCK_SUPERCONTEXT (context)) + { + if (BLOCK_FRAGMENT_ORIGIN (context)) + { + gcc_assert (!BLOCK_SUBBLOCKS (context)); + context = BLOCK_FRAGMENT_ORIGIN (context); + } + for (tree sub = BLOCK_SUBBLOCKS (context); + sub != block; + sub = BLOCK_CHAIN (sub)) + if (!sub) + return false; + if (context == outer) + return true; + else + block = context; + } +} + +/* Called during final while assembling the marker of the entry point + for an inlined function. */ + +static void +dwarf2out_inline_entry (tree block) +{ + /* If we can't represent it, don't bother. */ + if (!(dwarf_version >= 3 || !dwarf_strict)) + return; + + gcc_assert (DECL_P (block_ultimate_origin (block))); + + /* Sanity check the block tree. This would catch a case in which + BLOCK got removed from the tree reachable from the outermost + lexical block, but got retained in markers. It would still link + back to its parents, but some ancestor would be missing a link + down the path to the sub BLOCK. If the block got removed, its + BLOCK_NUMBER will not be a usable value. */ + if (flag_checking) + gcc_assert (block_within_block_p (block, + DECL_INITIAL (current_function_decl), + true)); + + gcc_assert (inlined_function_outer_scope_p (block)); + gcc_assert (!BLOCK_DIE (block)); + + if (BLOCK_FRAGMENT_ORIGIN (block)) + block = BLOCK_FRAGMENT_ORIGIN (block); + /* Can the entry point ever not be at the beginning of an + unfragmented lexical block? */ + else if (!(BLOCK_FRAGMENT_CHAIN (block) + || (cur_line_info_table + && !ZERO_VIEW_P (cur_line_info_table->view)))) + return; + + if (!inline_entry_data_table) + inline_entry_data_table + = hash_table<inline_entry_data_hasher>::create_ggc (10); + + + inline_entry_data **iedp + = inline_entry_data_table->find_slot_with_hash (block, + htab_hash_pointer (block), + INSERT); + if (*iedp) + /* ??? Ideally, we'd record all entry points for the same inlined + function (some may have been duplicated by e.g. unrolling), but + we have no way to represent that ATM. */ + return; + + inline_entry_data *ied = *iedp = ggc_cleared_alloc<inline_entry_data> (); + ied->block = block; + ied->label_pfx = BLOCK_INLINE_ENTRY_LABEL; + ied->label_num = BLOCK_NUMBER (block); + if (cur_line_info_table) + ied->view = cur_line_info_table->view; + + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + + ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_INLINE_ENTRY_LABEL, + BLOCK_NUMBER (block)); + ASM_OUTPUT_LABEL (asm_out_file, label); +} + /* Called from finalize_size_functions for size functions so that their body can be encoded in the debug info to describe the layout of variable-length structures. */ @@ -30560,6 +30750,9 @@ dwarf2out_finish (const char *) /* Flush out any latecomers to the limbo party. */ flush_limbo_die_list (); + if (inline_entry_data_table) + gcc_assert (inline_entry_data_table->elements () == 0); + if (flag_checking) { verify_die (comp_unit_die ()); diff --git a/gcc/final.c b/gcc/final.c index 68397b3ded36..99a7cadd7c9f 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -1611,6 +1611,7 @@ reemit_insn_block_notes (void) break; case NOTE_INSN_BEGIN_STMT: + case NOTE_INSN_INLINE_ENTRY: this_block = LOCATION_BLOCK (NOTE_MARKER_LOCATION (insn)); goto set_cur_block_to_this_block; @@ -2479,6 +2480,7 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, if (!DECL_IGNORED_P (current_function_decl) && notice_source_line (insn, NULL)) { + output_source_line: (*debug_hooks->source_line) (last_linenum, last_columnnum, last_filename, last_discriminator, true); @@ -2486,6 +2488,18 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, } break; + case NOTE_INSN_INLINE_ENTRY: + gcc_checking_assert (cfun->debug_nonbind_markers); + if (!DECL_IGNORED_P (current_function_decl)) + { + if (!notice_source_line (insn, NULL)) + break; + (*debug_hooks->inline_entry) (LOCATION_BLOCK + (NOTE_MARKER_LOCATION (insn))); + goto output_source_line; + } + break; + default: gcc_unreachable (); break; @@ -3189,6 +3203,17 @@ notice_source_line (rtx_insn *insn, bool *is_stmt) if (NOTE_MARKER_P (insn)) { location_t loc = NOTE_MARKER_LOCATION (insn); + /* The inline entry markers (gimple, insn, note) carry the + location of the call, because that's what we want to carry + during compilation, but the location we want to output in + debug information for the inline entry point is the location + of the function itself. */ + if (NOTE_KIND (insn) == NOTE_INSN_INLINE_ENTRY) + { + tree block = LOCATION_BLOCK (loc); + tree fn = block_ultimate_origin (block); + loc = DECL_SOURCE_LOCATION (fn); + } expanded_location xloc = expand_location (loc); if (xloc.line == 0) { @@ -4795,6 +4820,7 @@ rest_of_clean_state (void) && (!NOTE_P (insn) || (NOTE_KIND (insn) != NOTE_INSN_VAR_LOCATION && NOTE_KIND (insn) != NOTE_INSN_BEGIN_STMT + && NOTE_KIND (insn) != NOTE_INSN_INLINE_ENTRY && NOTE_KIND (insn) != NOTE_INSN_CALL_ARG_LOCATION && NOTE_KIND (insn) != NOTE_INSN_BLOCK_BEG && NOTE_KIND (insn) != NOTE_INSN_BLOCK_END diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 374bd45b09b9..6695526f3702 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -1371,6 +1371,19 @@ dump_gimple_debug (pretty_printer *buffer, gdebug *gs, int spc, dump_gimple_fmt (buffer, spc, flags, "# DEBUG BEGIN_STMT"); break; + case GIMPLE_DEBUG_INLINE_ENTRY: + if (flags & TDF_RAW) + dump_gimple_fmt (buffer, spc, flags, "%G INLINE_ENTRY %T", gs, + gimple_block (gs) + ? block_ultimate_origin (gimple_block (gs)) + : NULL_TREE); + else + dump_gimple_fmt (buffer, spc, flags, "# DEBUG INLINE_ENTRY %T", + gimple_block (gs) + ? block_ultimate_origin (gimple_block (gs)) + : NULL_TREE); + break; + default: gcc_unreachable (); } diff --git a/gcc/gimple.c b/gcc/gimple.c index c1b7229a6b79..9dc4911a36e0 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -874,6 +874,27 @@ gimple_build_debug_begin_stmt (tree block, location_t location } +/* Build a new GIMPLE_DEBUG_INLINE_ENTRY statement in BLOCK at + LOCATION. The BLOCK links to the inlined function. */ + +gdebug * +gimple_build_debug_inline_entry (tree block, location_t location + MEM_STAT_DECL) +{ + gdebug *p + = as_a <gdebug *> ( + gimple_build_with_ops_stat (GIMPLE_DEBUG, + (unsigned)GIMPLE_DEBUG_INLINE_ENTRY, 0 + PASS_MEM_STAT)); + + gimple_set_location (p, location); + gimple_set_block (p, block); + cfun->debug_marker_count++; + + return p; +} + + /* Build a GIMPLE_OMP_CRITICAL statement. BODY is the sequence of statements for which only one thread can execute. diff --git a/gcc/gimple.h b/gcc/gimple.h index 74605864a71a..265e3e24398c 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -202,7 +202,8 @@ enum gf_mask { enum gimple_debug_subcode { GIMPLE_DEBUG_BIND = 0, GIMPLE_DEBUG_SOURCE_BIND = 1, - GIMPLE_DEBUG_BEGIN_STMT = 2 + GIMPLE_DEBUG_BEGIN_STMT = 2, + GIMPLE_DEBUG_INLINE_ENTRY = 3 }; /* Masks for selecting a pass local flag (PLF) to work on. These @@ -1454,6 +1455,7 @@ geh_dispatch *gimple_build_eh_dispatch (int); gdebug *gimple_build_debug_bind (tree, tree, gimple * CXX_MEM_STAT_INFO); gdebug *gimple_build_debug_source_bind (tree, tree, gimple * CXX_MEM_STAT_INFO); gdebug *gimple_build_debug_begin_stmt (tree, location_t CXX_MEM_STAT_INFO); +gdebug *gimple_build_debug_inline_entry (tree, location_t CXX_MEM_STAT_INFO); gomp_critical *gimple_build_omp_critical (gimple_seq, tree, tree); gomp_for *gimple_build_omp_for (gimple_seq, int, tree, size_t, gimple_seq); gomp_parallel *gimple_build_omp_parallel (gimple_seq, tree, tree, tree); @@ -4784,13 +4786,25 @@ gimple_debug_begin_stmt_p (const gimple *s) return false; } +/* Return true if S is a GIMPLE_DEBUG INLINE_ENTRY statement. */ + +static inline bool +gimple_debug_inline_entry_p (const gimple *s) +{ + if (is_gimple_debug (s)) + return s->subcode == GIMPLE_DEBUG_INLINE_ENTRY; + + return false; +} + /* Return true if S is a GIMPLE_DEBUG non-binding marker statement. */ static inline bool gimple_debug_nonbind_marker_p (const gimple *s) { if (is_gimple_debug (s)) - return s->subcode == GIMPLE_DEBUG_BEGIN_STMT; + return s->subcode == GIMPLE_DEBUG_BEGIN_STMT + || s->subcode == GIMPLE_DEBUG_INLINE_ENTRY; return false; } diff --git a/gcc/insn-notes.def b/gcc/insn-notes.def index 5186d5812b30..9cac5f1549a7 100644 --- a/gcc/insn-notes.def +++ b/gcc/insn-notes.def @@ -71,6 +71,10 @@ INSN_NOTE (CALL_ARG_LOCATION) /* The beginning of a statement. */ INSN_NOTE (BEGIN_STMT) +/* The entry point for an inlined function. Its NOTE_BLOCK references + the lexical block whose abstract origin is the inlined function. */ +INSN_NOTE (INLINE_ENTRY) + /* Record the struct for the following basic block. Uses NOTE_BASIC_BLOCK. FIXME: Redundant with the basic block pointer now included in every insn. NOTE: If there's no CFG anymore, in other words, diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c index c26e46c0b767..3ad11dc4cf9b 100644 --- a/gcc/print-rtl.c +++ b/gcc/print-rtl.c @@ -276,6 +276,7 @@ rtx_writer::print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED, break; case NOTE_INSN_BEGIN_STMT: + case NOTE_INSN_INLINE_ENTRY: #ifndef GENERATOR_FILE { expanded_location xloc @@ -1879,6 +1880,10 @@ print_insn (pretty_printer *pp, const rtx_insn *x, int verbose) pp_string (pp, "debug begin stmt marker"); break; + case NOTE_INSN_INLINE_ENTRY: + pp_string (pp, "debug inline entry marker"); + break; + default: gcc_unreachable (); } diff --git a/gcc/rtl.h b/gcc/rtl.h index fcad6a7e3f89..c9dd7dd959fc 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -1654,7 +1654,8 @@ extern const char * const reg_note_name[]; for which NOTE_MARKER_LOCATION can be used. */ #define NOTE_MARKER_P(INSN) \ (NOTE_P (INSN) && \ - (NOTE_KIND (INSN) == NOTE_INSN_BEGIN_STMT)) + (NOTE_KIND (INSN) == NOTE_INSN_BEGIN_STMT \ + || NOTE_KIND (INSN) == NOTE_INSN_INLINE_ENTRY)) /* Variable declaration and the location of a variable. */ #define PAT_VAR_LOCATION_DECL(PAT) (XCTREE ((PAT), 0, VAR_LOCATION)) @@ -1692,6 +1693,8 @@ extern const char * const reg_note_name[]; (GET_CODE (PATTERN (INSN)) == DEBUG_MARKER \ ? (GET_MODE (PATTERN (INSN)) == VOIDmode \ ? NOTE_INSN_BEGIN_STMT \ + : GET_MODE (PATTERN (INSN)) == BLKmode \ + ? NOTE_INSN_INLINE_ENTRY \ : (enum insn_note)-1) \ : (enum insn_note)-1) /* Create patterns for debug markers. These and the above abstract @@ -1701,6 +1704,8 @@ extern const char * const reg_note_name[]; wouldn't be a problem. */ #define GEN_RTX_DEBUG_MARKER_BEGIN_STMT_PAT() \ gen_rtx_DEBUG_MARKER (VOIDmode) +#define GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT() \ + gen_rtx_DEBUG_MARKER (BLKmode) /* The VAR_LOCATION rtx in a DEBUG_INSN. */ #define INSN_VAR_LOCATION(INSN) \ diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index b0d9beb530a0..7f9ec770e197 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -4605,6 +4605,13 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id) GSI_NEW_STMT); } initialize_inlined_parameters (id, stmt, fn, bb); + if (debug_nonbind_markers_p && id->block + && inlined_function_outer_scope_p (id->block)) + { + gimple_stmt_iterator si = gsi_last_bb (bb); + gsi_insert_after (&si, gimple_build_debug_inline_entry + (id->block, input_location), GSI_NEW_STMT); + } if (DECL_INITIAL (fn)) { diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c index 08ac678e74c1..26da31f74cb2 100644 --- 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; else /* Verfify that only blocks with source location set @@ -640,6 +647,16 @@ dump_scope_block (FILE *file, int indent, tree scope, dump_flags_t flags) fprintf (file, "#%i", BLOCK_NUMBER (origin)); } } + if (BLOCK_FRAGMENT_ORIGIN (scope)) + fprintf (file, " Fragment of : #%i", + BLOCK_NUMBER (BLOCK_FRAGMENT_ORIGIN (scope))); + else if (BLOCK_FRAGMENT_CHAIN (scope)) + { + fprintf (file, " Fragment chain :"); + for (t = BLOCK_FRAGMENT_CHAIN (scope); t ; + t = BLOCK_FRAGMENT_CHAIN (t)) + fprintf (file, " #%i", BLOCK_NUMBER (t)); + } fprintf (file, " \n"); for (var = BLOCK_VARS (scope); var; var = DECL_CHAIN (var)) { diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index fbb63dbc305f..c3d4dac59ef2 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -9959,6 +9959,7 @@ reemit_marker_as_note (rtx_insn *insn) switch (kind) { case NOTE_INSN_BEGIN_STMT: + case NOTE_INSN_INLINE_ENTRY: { rtx_insn *note = NULL; if (cfun->debug_nonbind_markers) diff --git a/include/dwarf2.def b/include/dwarf2.def index c454506d1eda..3becd7f8219c 100644 --- a/include/dwarf2.def +++ b/include/dwarf2.def @@ -444,6 +444,7 @@ DW_AT (DW_AT_GNU_pubtypes, 0x2135) See http://gcc.gnu.org/wiki/Discriminator */ DW_AT (DW_AT_GNU_discriminator, 0x2136) DW_AT (DW_AT_GNU_locviews, 0x2137) +DW_AT (DW_AT_GNU_entry_view, 0x2138) /* VMS extensions. */ DW_AT (DW_AT_VMS_rtnbeg_pd_address, 0x2201) /* GNAT extensions. */
On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: > Here's what I checked in, right after the LVU patch. > > [IEPM] Introduce inline entry point markers One of these two patches breaks ppc64le bootstrap with the assembler complaining "Error: view number mismatch" when compiling libdecnumber.
On 02/08/2018 08:53 PM, Alan Modra wrote: > On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: >> Here's what I checked in, right after the LVU patch. >> >> [IEPM] Introduce inline entry point markers > > One of these two patches breaks ppc64le bootstrap with the assembler > complaining "Error: view number mismatch" when compiling > libdecnumber. > I've just passed along a similar failure (.i, .s and command line options) to Alex for ppc64 (be) building glibc. Jeff
On Feb 9, 2018, Jeff Law <law@redhat.com> wrote: > On 02/08/2018 08:53 PM, Alan Modra wrote: >> On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: >>> Here's what I checked in, right after the LVU patch. >>> >>> [IEPM] Introduce inline entry point markers >> >> One of these two patches breaks ppc64le bootstrap with the assembler >> complaining "Error: view number mismatch" when compiling >> libdecnumber. >> > I've just passed along a similar failure (.i, .s and command line > options) to Alex for ppc64 (be) building glibc. This fixes at least the testcase Jeff provided me with. I'm going ahead and checking it in as obvious. I suppose we might need more of these, on this and other ports, if they have been sloppy about zero-length pseudo insns :-( Would you guys please let me know whether you still see a problem, if you get a chance to respin? I was just about to crash in bed when I saw your email. When I get back up, I'll build the latest binutils release on ppc64, ppc64el and aarch64, and then bootstrap gcc with it. I should have done that when I broadened my testing of the SFN+LVU+IEPM patchset to those platforms, but I didn't realize I was failing to test them with an assembler with view support, doh! Sorry about that. for gcc/ChangeLog * config/rs6000/rs6000.md (blockage): Set length to zero. diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 33f0d959f5d0..8aa4e0e7c71e 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -11063,7 +11063,8 @@ (define_insn "blockage" [(unspec_volatile [(const_int 0)] UNSPECV_BLOCK)] "" - "") + "" + [(set_attr "length" "0")]) (define_expand "probe_stack_address" [(use (match_operand 0 "address_operand"))]
On Fri, Feb 09, 2018 at 08:34:08AM -0200, Alexandre Oliva wrote:
> * config/rs6000/rs6000.md (blockage): Set length to zero.
Thanks! This fixed the ppc64le libdecnumber error for me.
On 02/09/2018 03:34 AM, Alexandre Oliva wrote: > On Feb 9, 2018, Jeff Law <law@redhat.com> wrote: > >> On 02/08/2018 08:53 PM, Alan Modra wrote: >>> On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: >>>> Here's what I checked in, right after the LVU patch. >>>> >>>> [IEPM] Introduce inline entry point markers >>> >>> One of these two patches breaks ppc64le bootstrap with the assembler >>> complaining "Error: view number mismatch" when compiling >>> libdecnumber. >>> >> I've just passed along a similar failure (.i, .s and command line >> options) to Alex for ppc64 (be) building glibc. > > This fixes at least the testcase Jeff provided me with. I'm going ahead > and checking it in as obvious. I suppose we might need more of these, > on this and other ports, if they have been sloppy about zero-length > pseudo insns :-( > > Would you guys please let me know whether you still see a problem, if > you get a chance to respin? I was just about to crash in bed when I saw > your email. > > When I get back up, I'll build the latest binutils release on ppc64, > ppc64el and aarch64, and then bootstrap gcc with it. I should have done > that when I broadened my testing of the SFN+LVU+IEPM patchset to those > platforms, but I didn't realize I was failing to test them with an > assembler with view support, doh! Sorry about that. No need for the binutils+gcc bootstrapping test when you get up. Mine's already run. ppc, ppc64, ppc64le, aarch64 all covered. My tester does have half-dozen or so other failures overnight, but I haven't looked at them yet. If any look similar I'll first check if we're dealing with a zero length pseudo insn (I wouldn't be surprised if blockage insns are consistently wrong on that). jeff
On Feb 9, 2018, Jeff Law <law@redhat.com> wrote: > On 02/08/2018 08:53 PM, Alan Modra wrote: >> On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: >>> Here's what I checked in, right after the LVU patch. >>> >>> [IEPM] Introduce inline entry point markers >> >> One of these two patches breaks ppc64le bootstrap with the assembler >> complaining "Error: view number mismatch" when compiling >> libdecnumber. >> > I've just passed along a similar failure (.i, .s and command line > options) to Alex for ppc64 (be) building glibc. Thanks. So, I'm told there are more such issues, that non-asm insn length attrs can't be relied on at this time to be nonzero only when the actual length is not zero. When we fail that and regard a zero-length insn as nonzero and that's all we have between two subsequent views, the assembler (GNU as 2.30) will catch and report the error. With other assemblers, the incorrect view reset will go unnoticed and result in incorrect debug info. So, as discussed on IRC, I'm trying to use a target hook to allow targets to indicate that their length attrs have been assessed for this purpose, and a param to make that overridable, but I'm having trouble initializing the param from the target hook. How does one do that? Meanwhile, here's the (WIP) patch that introduces the param defaulting to no locview optimizations (which can only be performed at points in which the compiler knows there are PC changes); I'd like it to default to targetm.attr_length_reliable_for_view_count, but I couldn't figure out how to do so. Help? By disabling it altogether, we won't get the assembler checks or incorrect view numbers in debug info, but we will get plenty of all-zero locview lists. Oh well... I guess at this point that's better than wrong debug info or assembler failures. disable locview optimizations for now --- gcc/doc/tm.texi | 13 +++++++++++++ gcc/doc/tm.texi.in | 2 ++ gcc/dwarf2out.c | 4 +++- gcc/opts.c | 6 ++++++ gcc/params.def | 5 +++++ gcc/target.def | 13 +++++++++++++ 6 files changed, 42 insertions(+), 1 deletion(-) diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index ddf48cb4b4d2..50fa0d387f32 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -9986,6 +9986,19 @@ following it should not be run. Usually true only for virtual assembler targets. @end deftypevr +@deftypevr {Target Hook} bool TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT +TRUE if get_min_attr_length returns +zero for any non-asm insn that might have length zero, during final. +It's ok if it's conservative and always returns zero. +If, in addition to the essential property, when an insn is known to have +nonzero length, get_min_attr_length returns a positive number, that will +enable loclist optimizations. There is little point in making this TRUE +otherwise. Enabling this when the essential property is not met while +using GNU as 2.30 or newer may cause the assembler to detect the errors +and fail to assemble; other assemblers will silently let the errors +through, with incorrect view numbers in debug information. +@end deftypevr + @defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2}) A C statement to issue assembly directives that create a difference @var{lab1} minus @var{lab2}, using an integer of the given @var{size}. diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 0aab45f4992c..26b32db77f74 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -6929,6 +6929,8 @@ tables, and hence is desirable if it works. @hook TARGET_NO_REGISTER_ALLOCATION +@hook TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT + @defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2}) A C statement to issue assembly directives that create a difference @var{lab1} minus @var{lab2}, using an integer of the given @var{size}. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 749c7e3b9bbc..6a6520a05e8b 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -96,6 +96,7 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "file-prefix-map.h" /* remap_debug_filename() */ +#include "params.h" static void dwarf2out_source_line (unsigned int, unsigned int, const char *, int, bool); @@ -26926,7 +26927,8 @@ dwarf2out_var_location (rtx_insn *loc_note) || GET_CODE (loc_note) == ASM_INPUT || asm_noperands (loc_note) >= 0) ; - else if (get_attr_min_length (loc_note) > 0) + else if (PARAM_VALUE (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING) + && get_attr_min_length (loc_note) > 0) RESET_NEXT_VIEW (cur_line_info_table->view); return; diff --git a/gcc/opts.c b/gcc/opts.c index f2795f98bf44..d4b3065caad6 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -1039,6 +1039,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, if ((opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS) && opts->x_flag_tm) sorry ("transactional memory is not supported with " "%<-fsanitize=kernel-address%>"); + +#if 0 + maybe_set_param_value (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING, + targetm.attr_length_reliable_for_view_count, + opts->x_param_values, opts_set->x_param_values); +#endif } #define LEFT_COLUMN 27 diff --git a/gcc/params.def b/gcc/params.def index 930b31820be9..184e435c786a 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -1331,6 +1331,11 @@ DEFPARAM(PARAM_AVOID_FMA_MAX_BITS, "Maximum number of bits for which we avoid creating FMAs.", 0, 0, 512) +DEFPARAM(PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING, + "use-attr-length-in-view-counting", + "Optimize locview lists based on length attributes.", + 0 /* targetm.attr_length_reliable_for_view_count */, 0, 1) + /* Local variables: diff --git a/gcc/target.def b/gcc/target.def index aeb41df19454..6966053157a3 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -6631,6 +6631,19 @@ following it should not be run. Usually true only for virtual assembler\n\ targets.", bool, false) +DEFHOOKPOD +(attr_length_reliable_for_view_count, "TRUE if get_min_attr_length returns\n\ +zero for any non-asm insn that might have length zero, during final.\n\ +It's ok if it's conservative and always returns zero.\n\ +If, in addition to the essential property, when an insn is known to have\n\ +nonzero length, get_min_attr_length returns a positive number, that will\n\ +enable loclist optimizations. There is little point in making this TRUE\n\ +otherwise. Enabling this when the essential property is not met while\n\ +using GNU as 2.30 or newer may cause the assembler to detect the errors\n\ +and fail to assemble; other assemblers will silently let the errors\n\ +through, with incorrect view numbers in debug information.", +bool, false) + /* Leave the boolean fields at the end. */ /* Functions related to mode switching. */
On Fri, Feb 09, 2018 at 07:01:25PM -0200, Alexandre Oliva wrote: > So, as discussed on IRC, I'm trying to use a target hook to allow > targets to indicate that their length attrs have been assessed for this > purpose, and a param to make that overridable, but I'm having trouble > initializing the param from the target hook. How does one do that? Better in the default version of the target hook check the param whether it should return true or false, and for analyzed targets just use an always true (or false, depending on what the hook is) as the hook. > By disabling it altogether, we won't get the assembler checks or > incorrect view numbers in debug info, but we will get plenty of all-zero > locview lists. Oh well... I guess at this point that's better than > wrong debug info or assembler failures. For the debugging of the target issues, you can also use a hack: in final.c for instructions that have minimum length longer than zero emit a label right before emitting the insn and after it: .Lhacke1: .if (.Lhacke1 - .Lhackb1) == 0 .error "zero length" .endif (or perhaps just: .if (. - .Lhack1) == 0 .error "zero length" .endif ), then make check various targets with that. Jakub
I'm seeing regressions from my glibc bot for all of arm, mips, s390 and sh. https://sourceware.org/ml/libc-testresults/2018-q1/msg00283.html arm and mips are "view number mismatch" building glibc and s390 is the same error but building libgcc, so presumably those are the present issue. sh is GCC segfaults building glibc, so it's less clearly associated with this patch (but GCC mainline r257539 is bad and r257480 is good, in any case). (It's possible more failures might appear on other architectures once the bot builds the glibc testsuite, that's just failures from building GCC and glibc.)
On Feb 9, 2018, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, Feb 09, 2018 at 07:01:25PM -0200, Alexandre Oliva wrote: >> So, as discussed on IRC, I'm trying to use a target hook to allow >> targets to indicate that their length attrs have been assessed for this >> purpose, and a param to make that overridable, but I'm having trouble >> initializing the param from the target hook. How does one do that? > Better in the default version of the target hook check the param > whether it should return true or false, and for analyzed targets > just use an always true (or false, depending on what the hook is) > as the hook. I want it to be overridable, so here's what I ended up with. Testing underway; ok to install if it succeeds? We need accurate length information to be able to optimize away locviews that would contain only zero-numbered views, and even to output correct view numbers. E.g., if we assume an insn has length nonzero, i.e., that it will advance PC, we conclude the view number after it is zero. If the assumption turns out to be false, the assembler view zero assert will catch the mistake, but only if we're using a view-capable assembler. Otherwise, view numbers will silently go out of sync between the compiler and the assembler, and thus the debug info consumer. Unfortunately, a number of targets seem to fail the essential property for this to work, namely, that get_min_attr_length() be zero for every insn that might have length zero. We thus introduce a target hook that indicates whether we can rely on the length attribute for this purpose, and a param that can be used to override it, for testing purposes or to work around errors. for gcc/ChangeLog * params.def (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING): New. * target.def (attr_length_reliable_for_view_count): New. * dwarf2out.c: Include params.h. (unreliable_attr_length_p): New. (dwarf2out_var_location): Use it. * doc/tm.texi.in (TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT): New. * doc/tm.texi: Rebuilt. --- gcc/doc/tm.texi | 13 +++++++++++++ gcc/doc/tm.texi.in | 2 ++ gcc/dwarf2out.c | 28 +++++++++++++++++++++++++++- gcc/params.def | 5 +++++ gcc/target.def | 14 ++++++++++++++ 5 files changed, 61 insertions(+), 1 deletion(-) diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index ddf48cb4b4d2..50fa0d387f32 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -9986,6 +9986,19 @@ following it should not be run. Usually true only for virtual assembler targets. @end deftypevr +@deftypevr {Target Hook} bool TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT +TRUE if get_min_attr_length returns +zero for any non-asm insn that might have length zero, during final. +It's ok if it's conservative and always returns zero. +If, in addition to the essential property, when an insn is known to have +nonzero length, get_min_attr_length returns a positive number, that will +enable loclist optimizations. There is little point in making this TRUE +otherwise. Enabling this when the essential property is not met while +using GNU as 2.30 or newer may cause the assembler to detect the errors +and fail to assemble; other assemblers will silently let the errors +through, with incorrect view numbers in debug information. +@end deftypevr + @defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2}) A C statement to issue assembly directives that create a difference @var{lab1} minus @var{lab2}, using an integer of the given @var{size}. diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 0aab45f4992c..26b32db77f74 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -6929,6 +6929,8 @@ tables, and hence is desirable if it works. @hook TARGET_NO_REGISTER_ALLOCATION +@hook TARGET_ATTR_LENGTH_RELIABLE_FOR_VIEW_COUNT + @defmac ASM_OUTPUT_DWARF_DELTA (@var{stream}, @var{size}, @var{label1}, @var{label2}) A C statement to issue assembly directives that create a difference @var{lab1} minus @var{lab2}, using an integer of the given @var{size}. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 749c7e3b9bbc..c48c117c8a16 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -96,6 +96,7 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "file-prefix-map.h" /* remap_debug_filename() */ +#include "params.h" static void dwarf2out_source_line (unsigned int, unsigned int, const char *, int, bool); @@ -26860,6 +26861,30 @@ dwarf2out_next_real_insn (rtx_insn *loc_note) return next_real; } +/* Return TRUE if we should NOT use ATTR_length to optimize locview + lists, and FALSE otherwise. + + If ATTR_length is always zero for insns that could have length + zero, we can use it, then we'll know when we're guaranteed to have + a PC change that implies a view reset. locview lists whose views + are all zero can be optimized out completely. + + However, if we enable these optimizations based on unreliable data, + we might get errors from the assembler (binutils 2.30+) when it + notices we have incorrectly assumed a view would be zero, or + silently get incorrect locview lists otherwise. */ + +static bool +unreliable_attr_length_p (void) +{ + int parm = PARAM_VALUE (PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING); + + if (parm != -1) + return !parm; + else + return !targetm.attr_length_reliable_for_view_count; +} + /* Called by the final INSN scan whenever we see a var location. We use it to drop labels in the right places, and throw the location in our lookup table. */ @@ -26921,7 +26946,8 @@ dwarf2out_var_location (rtx_insn *loc_note) gcc_unreachable (); else if (JUMP_TABLE_DATA_P (loc_note)) RESET_NEXT_VIEW (cur_line_info_table->view); - else if (GET_CODE (loc_note) == USE + else if (unreliable_attr_length_p () + || GET_CODE (loc_note) == USE || GET_CODE (loc_note) == CLOBBER || GET_CODE (loc_note) == ASM_INPUT || asm_noperands (loc_note) >= 0) diff --git a/gcc/params.def b/gcc/params.def index 930b31820be9..12e217ed9da4 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -1331,6 +1331,11 @@ DEFPARAM(PARAM_AVOID_FMA_MAX_BITS, "Maximum number of bits for which we avoid creating FMAs.", 0, 0, 512) +DEFPARAM(PARAM_USE_ATTR_LENGTH_IN_VIEW_COUNTING, + "use-attr-length-in-view-counting", + "Optimize locview lists based on length attributes.", + -1 /* targetm.attr_length_reliable_for_view_count */, 0, 1) + /* Local variables: diff --git a/gcc/target.def b/gcc/target.def index aeb41df19454..a7fedb72fef2 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -6631,6 +6631,20 @@ following it should not be run. Usually true only for virtual assembler\n\ targets.", bool, false) +DEFHOOKPOD +(attr_length_reliable_for_view_count, +"True if, during final, get_min_attr_length returns zero\n\ +for all non-asm insns that might have length zero.\n\ +It's ok if it's conservative and always returns zero.\n\ +If, in addition to the essential property, when an insn is known to have\n\ +nonzero length, get_min_attr_length returns a positive number, that will\n\ +enable loclist optimizations. There is little point in making this true\n\ +otherwise. Enabling this when the essential property is not met while\n\ +using GNU as 2.30 or newer may cause the assembler to detect the errors\n\ +and fail to assemble; other assemblers will silently let the errors\n\ +through, with incorrect view numbers in debug information.", +bool, false) + /* Leave the boolean fields at the end. */ /* Functions related to mode switching. */
On Fri, 9 Feb 2018, Joseph Myers wrote: > I'm seeing regressions from my glibc bot for all of arm, mips, s390 and > sh. > > https://sourceware.org/ml/libc-testresults/2018-q1/msg00283.html > > arm and mips are "view number mismatch" building glibc and s390 is the > same error but building libgcc, so presumably those are the present issue. > sh is GCC segfaults building glibc, so it's less clearly associated with > this patch (but GCC mainline r257539 is bad and r257480 is good, in any > case). sh4 is: during RTL pass: final In file included from strtof_l.c:45: strtod_l.c: In function '____strtof_l_internal': strtod_l.c:1769:1: internal compiler error: Segmentation fault } ^ 0xb98e3f crash_signal /scratch/jmyers/glibc-bot/src/gcc/gcc/toplev.c:325 0x856b18 maybe_output_next_view /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:1726 0x856b18 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2714 0xef36bc print_slot /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:2557 0xef6520 output_far_jump(rtx_insn*, rtx_def*) /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:2617 0x857098 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:3101 0x856c26 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2806 0x858469 final_1 /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2088 0x8591d8 rest_of_handle_final /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4637 0x8591d8 execute /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4711 Since it's in maybe_output_next_view, I think it probably comes from these patches. > (It's possible more failures might appear on other architectures once the > bot builds the glibc testsuite, that's just failures from building GCC and > glibc.) hppa has a similar ICE building glibc tests, also in maybe_output_next_view, so probably also from these patches: during RTL pass: final test-tgmath2.c: In function 'test_fma_3': test-tgmath2.c:421:1: internal compiler error: Segmentation fault } ^ 0xb7525f crash_signal /scratch/jmyers/glibc-bot/src/gcc/gcc/toplev.c:325 0x835f14 maybe_output_next_view /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:1726 0x835f14 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2714 0xed7761 pa_output_call(rtx_insn*, rtx_def*, int) /scratch/jmyers/glibc-bot/src/gcc/gcc/config/pa/pa.c:8019 0x83642b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:3101 0x836026 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2806 0x837629 final_1 /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2088 0x8389d6 rest_of_handle_final /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4637 0x8389d6 execute /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4711
On Feb 9, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > On Feb 9, 2018, Jeff Law <law@redhat.com> wrote: >> On 02/08/2018 08:53 PM, Alan Modra wrote: >>> On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: >>>> Here's what I checked in, right after the LVU patch. >>>> >>>> [IEPM] Introduce inline entry point markers >>> >>> One of these two patches breaks ppc64le bootstrap with the assembler >>> complaining "Error: view number mismatch" when compiling >>> libdecnumber. >>> >> I've just passed along a similar failure (.i, .s and command line >> options) to Alex for ppc64 (be) building glibc. > Thanks. So, I'm told there are more such issues, that non-asm insn > length attrs can't be relied on at this time to be nonzero only when the > actual length is not zero. I wonder... In the previously-posted patch, we still regard call insns are advancing PC. I think that's a safe assumption, but... are there any other kinds of patterns we could recognize that would certainly generate an actual PC-changing insn? How about jump insns? How about insns that are SETs, or PARALLELs containing at least one SET? Does anyone see any risk in recognizing those when the length attr is, conservatively or not, deemed unreliable? Any other paterns we could recognize to that end?
On 02/09/2018 09:39 PM, Alexandre Oliva wrote: > On Feb 9, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > >> On Feb 9, 2018, Jeff Law <law@redhat.com> wrote: >>> On 02/08/2018 08:53 PM, Alan Modra wrote: >>>> On Fri, Feb 09, 2018 at 01:21:27AM -0200, Alexandre Oliva wrote: >>>>> Here's what I checked in, right after the LVU patch. >>>>> >>>>> [IEPM] Introduce inline entry point markers >>>> >>>> One of these two patches breaks ppc64le bootstrap with the assembler >>>> complaining "Error: view number mismatch" when compiling >>>> libdecnumber. >>>> >>> I've just passed along a similar failure (.i, .s and command line >>> options) to Alex for ppc64 (be) building glibc. > >> Thanks. So, I'm told there are more such issues, that non-asm insn >> length attrs can't be relied on at this time to be nonzero only when the >> actual length is not zero. > > I wonder... In the previously-posted patch, we still regard call insns > are advancing PC. I think that's a safe assumption, but... are there > any other kinds of patterns we could recognize that would certainly > generate an actual PC-changing insn? How about jump insns? How about > insns that are SETs, or PARALLELs containing at least one SET? Does > anyone see any risk in recognizing those when the length attr is, > conservatively or not, deemed unreliable? Any other paterns we could > recognize to that end? So given what I've seen in the ARM port, I don't think we can generally assume any insn advances the PC. Here's why. On the ARM there's a little state machine that's used to implement conditional execution. When we're in certain states the backend will generate no code for certain JUMP_INSNs and change the state. That's fine and dandy. THe problem is we can't actually tell outside the ARM backend when that's happened! You might think we could embed tests of the FSM within the length computation. But the state of the FSM is only valid during assembly output (e.g. final). But insn lengths are set up during branch shortening and if you query the length attribute you get value computed by branch shortening. The only way around this would be to do what I would consider some interface abuse and query insn_min_length (not to be confused with get_attr_min_length). Then we wouldn't get the cached version and we could do queries of the state machine on the fly in dwarf2out_var_location. But it seems rather icky and I haven't even been able to convince myself it's really safe. And while this is specific to the ARM and JUMP_INSNs, I can easily envision scenarios where other ports could use the same kind of little state machine for standard INSNs (PA to generate add,tr) or even CALL_INSNs (target dependent optimization of tail calls). You also have to worry about ports that try to optimize away nop-insns that snuck through the optimizers. I once worked on a port that couldn't encode a register self copy. So when one snuck through the optimizers, we had to deal with it in the output code and I know I've seen other instances where ports tried to compensate for nop-insns that snuck through. So in the end I don't think you can assume that any given insn advances the PC. The closest we have is the length attribute, but it has always supposed to have been conservatively correct for the purposes of branch shortening. ie, it can never return a length less than the actual length, but it is allowed to return a length longer than the actual length. Jeff
Hi, Joseph, On Feb 9, 2018, Joseph Myers <joseph@codesourcery.com> wrote: > sh4 is: > during RTL pass: final > In file included from strtof_l.c:45: > strtod_l.c: In function '____strtof_l_internal': > strtod_l.c:1769:1: internal compiler error: Segmentation fault > } > ^ > 0xb98e3f crash_signal > /scratch/jmyers/glibc-bot/src/gcc/gcc/toplev.c:325 > 0x856b18 maybe_output_next_view > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:1726 > 0x856b18 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2714 > 0xef36bc print_slot > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:2557 > 0xef6520 output_far_jump(rtx_insn*, rtx_def*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:2617 > 0x857098 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:3101 > 0x856c26 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2806 > 0x858469 final_1 > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2088 > 0x8591d8 rest_of_handle_final > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4637 > 0x8591d8 execute > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4711 > Since it's in maybe_output_next_view, I think it probably comes from these > patches. Thanks for the backtraces. The problem is that ports call final_scan_insn recursively passing NULL for seen, but now there is code that actually cares about carrying information around in it. I decided against silencing the errors by testing seen for non-NULL before dereferencing it, and instead arranging for the outermost pointer to be recovered in recursive calls. I hope this is enough to cover all the cases of final_scan_insn being called from ports. Could you possibly give it a spin, pretty please with sugar on top? ;-) Thanks a ton, (I'll provide a ChangeLog for formal review and hopefully approval once it has gone through more than my initial smoke testing on x86_64, but my own testing will take some time, and I wanted to give you something that at least stood a chance of fixing the problem before crashing in bed :-) [LVU] deal with md final_scan_insn From: Alexandre Oliva <aoliva@redhat.com> Ports call final_scan_insn with seen == NULL, and then maybe_output_next_view crashes because it assumes it's non-NULL. Oops. Fixed. --- gcc/final.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/gcc/final.c b/gcc/final.c index c311c198da67..a60be5ddc31c 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -2236,9 +2236,9 @@ asm_show_source (const char *filename, int linenum) debug information. We force the emission of a line note after both NOTE_INSN_PROLOGUE_END and NOTE_INSN_FUNCTION_BEG. */ -rtx_insn * -final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, - int nopeepholes ATTRIBUTE_UNUSED, int *seen) +static rtx_insn * +final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, + int nopeepholes ATTRIBUTE_UNUSED, int *seen) { #if HAVE_cc0 rtx set; @@ -3204,6 +3204,34 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, } return NEXT_INSN (insn); } + +/* This is a wrapper around final_scan_insn_1 that allows ports to + call it recursively without a known value for SEEN. The value is + saved at the outermost call, and recovered for recursive calls. */ + +rtx_insn * +final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p, + int nopeepholes, int *seen) +{ + static int *enclosing_seen; + static int recursion_counter; + + gcc_assert (seen || recursion_counter); + gcc_assert (!recursion_counter || !seen || seen == enclosing_seen); + + if (!recursion_counter++) + enclosing_seen = seen; + else if (!seen) + seen = enclosing_seen; + + rtx_insn *ret = final_scan_insn_1 (insn, file, optimize_p, nopeepholes, seen); + + if (!--recursion_counter) + enclosing_seen = NULL; + + return ret; +} + /* Return whether a source line note needs to be emitted before INSN. Sets IS_STMT to TRUE if the line should be marked as a possible >> (It's possible more failures might appear on other architectures once the >> bot builds the glibc testsuite, that's just failures from building GCC and >> glibc.) > hppa has a similar ICE building glibc tests, also in > maybe_output_next_view, so probably also from these patches: > during RTL pass: final > test-tgmath2.c: In function 'test_fma_3': > test-tgmath2.c:421:1: internal compiler error: Segmentation fault > } > ^ > 0xb7525f crash_signal > /scratch/jmyers/glibc-bot/src/gcc/gcc/toplev.c:325 > 0x835f14 maybe_output_next_view > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:1726 > 0x835f14 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2714 > 0xed7761 pa_output_call(rtx_insn*, rtx_def*, int) > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/pa/pa.c:8019 > 0x83642b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:3101 > 0x836026 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2806 > 0x837629 final_1 > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2088 > 0x8389d6 rest_of_handle_final > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4637 > 0x8389d6 execute > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4711
On Feb 10, 2018, Jeff Law <law@redhat.com> wrote: > So given what I've seen in the ARM port, I don't think we can generally > assume any insn advances the PC. Ugh. Thanks, I'll adjust the patch to not count call insns, I guess. Maybe what we should have is some target hook that, instead of vowing for ATTR_length (or not), tells us whether an insn might not advance the PC, with a default that assumes "reasonable" behavior and some more conservative alternatives for targets that do messy stuff. I'll give that some more thought. > So in the end I don't think you can assume that any given insn advances > the PC. The closest we have is the length attribute, but it has always > supposed to have been conservatively correct for the purposes of branch > shortening. ie, it can never return a length less than the actual > length, but it is allowed to return a length longer than the actual length. Interesting. I recall some cases back in my SH days in which I needed it to be quite precise; I guess this distorted my general expectation. Oh well... Back to the drawing board WRT the locview table optimizations. Maybe we'll find out my concern about them was unjustified, and the space they take up is tolerable. Or maybe we can find some way to get the most out of them without actually breaking anything. We'll see...
On 02/10/2018 06:04 AM, Alexandre Oliva wrote: > On Feb 10, 2018, Jeff Law <law@redhat.com> wrote: > >> So given what I've seen in the ARM port, I don't think we can generally >> assume any insn advances the PC. > > Ugh. Thanks, I'll adjust the patch to not count call insns, I guess. > > Maybe what we should have is some target hook that, instead of vowing > for ATTR_length (or not), tells us whether an insn might not advance the > PC, with a default that assumes "reasonable" behavior and some more > conservative alternatives for targets that do messy stuff. I'll give > that some more thought. > >> So in the end I don't think you can assume that any given insn advances >> the PC. The closest we have is the length attribute, but it has always >> supposed to have been conservatively correct for the purposes of branch >> shortening. ie, it can never return a length less than the actual >> length, but it is allowed to return a length longer than the actual length. > > Interesting. I recall some cases back in my SH days in which I needed > it to be quite precise; I guess this distorted my general expectation. > Oh well... Back to the drawing board WRT the locview table > optimizations. Maybe we'll find out my concern about them was > unjustified, and the space they take up is tolerable. Or maybe we can > find some way to get the most out of them without actually breaking > anything. We'll see... We have had ports at times try to be very accurate. You mention the sh, I also tried for high accuracy on the PA at one time. I think it was to support jumps in call delay slots where you have to compute the difference of two labels. I eventually gave up and fixed gas :-) But the process was still helpful in improving delay slot filling and instruction scheduling. jeff
On 02/10/2018 05:34 AM, Alexandre Oliva wrote: > Hi, Joseph, > > On Feb 9, 2018, Joseph Myers <joseph@codesourcery.com> wrote: > >> sh4 is: >> during RTL pass: final >> In file included from strtof_l.c:45: >> strtod_l.c: In function '____strtof_l_internal': >> strtod_l.c:1769:1: internal compiler error: Segmentation fault >> } >> ^ >> 0xb98e3f crash_signal >> /scratch/jmyers/glibc-bot/src/gcc/gcc/toplev.c:325 >> 0x856b18 maybe_output_next_view >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:1726 >> 0x856b18 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2714 >> 0xef36bc print_slot >> /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:2557 >> 0xef6520 output_far_jump(rtx_insn*, rtx_def*) >> /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:2617 >> 0x857098 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:3101 >> 0x856c26 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2806 >> 0x858469 final_1 >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:2088 >> 0x8591d8 rest_of_handle_final >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4637 >> 0x8591d8 execute >> /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4711 > >> Since it's in maybe_output_next_view, I think it probably comes from these >> patches. > Thanks for the backtraces. The problem is that ports call > final_scan_insn recursively passing NULL for seen, but now there is code > that actually cares about carrying information around in it. I decided > against silencing the errors by testing seen for non-NULL before > dereferencing it, and instead arranging for the outermost pointer to be > recovered in recursive calls. I hope this is enough to cover all the > cases of final_scan_insn being called from ports. Could you possibly > give it a spin, pretty please with sugar on top? ;-) Thanks a ton, > > (I'll provide a ChangeLog for formal review and hopefully approval once > it has gone through more than my initial smoke testing on x86_64, but my > own testing will take some time, and I wanted to give you something that > at least stood a chance of fixing the problem before crashing in bed :-) > > > [LVU] deal with md final_scan_insn > > From: Alexandre Oliva <aoliva@redhat.com> > > Ports call final_scan_insn with seen == NULL, and then > maybe_output_next_view crashes because it assumes it's > non-NULL. Oops. Fixed. A bit icky. But OK. Jeff
On Feb 10, 2018, Jeff Law <law@redhat.com> wrote: >> Ports call final_scan_insn with seen == NULL, and then >> maybe_output_next_view crashes because it assumes it's >> non-NULL. Oops. Fixed. > A bit icky. But OK. Thanks. Testing revealed some ports had already introduced their own 'seen' variables passed to final_scan_insn recursive calls, so I adjusted them, and the comments to the wrapper to indicate ports should not do that. Here's what I checked in. [LVU] deal with md final_scan_insn Ports call final_scan_insn with seen == NULL, and then maybe_output_next_view crashes because it assumes it's non-NULL. Oops. Fixed. for gcc/ChangeLog * final.c (final_scan_insn_1): Renamed from... (final_scan_insn): ... this. New wrapper, to recover seen from the outermost call in recursive ones. * config/sparc/sparc.c (output_return): Drop seen from call. (output_sibcall): Likewise. * config/visium/visium.c (output_branch): Likewise. --- gcc/config/sparc/sparc.c | 6 ++---- gcc/config/visium/visium.c | 3 +-- gcc/final.c | 36 +++++++++++++++++++++++++++++++++--- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c index 48669f177652..7126b57ba011 100644 --- a/gcc/config/sparc/sparc.c +++ b/gcc/config/sparc/sparc.c @@ -6422,7 +6422,6 @@ output_return (rtx_insn *insn) { rtx_insn *delay; rtx pat; - int seen; delay = NEXT_INSN (insn); gcc_assert (delay); @@ -6442,7 +6441,7 @@ output_return (rtx_insn *insn) Make sure to output its source location first. */ PATTERN (delay) = gen_blockage (); INSN_CODE (delay) = -1; - final_scan_insn (delay, asm_out_file, optimize, 0, &seen); + final_scan_insn (delay, asm_out_file, optimize, 0, NULL); INSN_LOCATION (delay) = UNKNOWN_LOCATION; output_restore (pat); @@ -6503,7 +6502,6 @@ output_sibcall (rtx_insn *insn, rtx call_operand) { rtx_insn *delay; rtx pat; - int seen; delay = NEXT_INSN (insn); gcc_assert (delay); @@ -6514,7 +6512,7 @@ output_sibcall (rtx_insn *insn, rtx call_operand) Make sure to output its source location first. */ PATTERN (delay) = gen_blockage (); INSN_CODE (delay) = -1; - final_scan_insn (delay, asm_out_file, optimize, 0, &seen); + final_scan_insn (delay, asm_out_file, optimize, 0, NULL); INSN_LOCATION (delay) = UNKNOWN_LOCATION; output_restore (pat); diff --git a/gcc/config/visium/visium.c b/gcc/config/visium/visium.c index 8751156c4065..106cdaf9e3f9 100644 --- a/gcc/config/visium/visium.c +++ b/gcc/config/visium/visium.c @@ -3094,10 +3094,9 @@ output_branch (rtx label, const char *cond, rtx_insn *insn) if (final_sequence) { rtx_insn *delay = NEXT_INSN (insn); - int seen; gcc_assert (delay); - final_scan_insn (delay, asm_out_file, optimize, 0, &seen); + final_scan_insn (delay, asm_out_file, optimize, 0, NULL); PATTERN (delay) = gen_blockage (); INSN_CODE (delay) = -1; } diff --git a/gcc/final.c b/gcc/final.c index 99a7cadd7c9f..cbebbfdf5b07 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -2236,9 +2236,9 @@ asm_show_source (const char *filename, int linenum) debug information. We force the emission of a line note after both NOTE_INSN_PROLOGUE_END and NOTE_INSN_FUNCTION_BEG. */ -rtx_insn * -final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, - int nopeepholes ATTRIBUTE_UNUSED, int *seen) +static rtx_insn * +final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, + int nopeepholes ATTRIBUTE_UNUSED, int *seen) { #if HAVE_cc0 rtx set; @@ -3189,6 +3189,36 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, } return NEXT_INSN (insn); } + +/* This is a wrapper around final_scan_insn_1 that allows ports to + call it recursively without a known value for SEEN. The value is + saved at the outermost call, and recovered for recursive calls. + Recursive calls MUST pass NULL, or the same pointer if they can + otherwise get to it. */ + +rtx_insn * +final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p, + int nopeepholes, int *seen) +{ + static int *enclosing_seen; + static int recursion_counter; + + gcc_assert (seen || recursion_counter); + gcc_assert (!recursion_counter || !seen || seen == enclosing_seen); + + if (!recursion_counter++) + enclosing_seen = seen; + else if (!seen) + seen = enclosing_seen; + + rtx_insn *ret = final_scan_insn_1 (insn, file, optimize_p, nopeepholes, seen); + + if (!--recursion_counter) + enclosing_seen = NULL; + + return ret; +} + /* Return whether a source line note needs to be emitted before INSN. Sets IS_STMT to TRUE if the line should be marked as a possible
On Feb 9, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > On Feb 9, 2018, Jakub Jelinek <jakub@redhat.com> wrote: >> On Fri, Feb 09, 2018 at 07:01:25PM -0200, Alexandre Oliva wrote: >>> So, as discussed on IRC, I'm trying to use a target hook to allow >>> targets to indicate that their length attrs have been assessed for this >>> purpose, and a param to make that overridable, but I'm having trouble >>> initializing the param from the target hook. How does one do that? >> Better in the default version of the target hook check the param >> whether it should return true or false, and for analyzed targets >> just use an always true (or false, depending on what the hook is) >> as the hook. > I want it to be overridable, so here's what I ended up with. > Testing underway; ok to install if it succeeds? This patch supersedes the previous one. Testing underway... Ok if it succeeds? Sorry for combining so many not-entirely-related issues in a single patch, but there would be lots of overlaps and conflicts otherwise, and in the end they're all about allowing finer-tuning of markers and views, so I hope tha's ok. Well, not all: there are formatting fixes to docs that are totally unrelated, but still overlapping. Anyway... [LVU, IEPM] several new controlling options Given that the minimum insn length is not generally reliable to tell whether an insn actually advances PC, this patch disables the locview list optimizations that can only be done when can tell it. The preexisting logic is retained, however, and can be enabled with the newly-introduced -ginternal-reset-location-view. This is now enabled by default only if the target defines a hook that may override or defer to the preexisting logic. The negated command line option can then be used should errors still be encountered. We also introduce options to control whether to assume .loc and view support in the assembler, and to control whether to output inline entry points (and views) from markers. This patch also fixes a number of documentation formatting errors, namely using @item rather than @itemx for all but the first of several options before a description. for gcc/ChangeLog * common.opt (gas-loc-support, gas-locview-support): New. (ginline-points, ginternal-reset-location-views): New. * doc/invoke.texi: Document them. Use @itemx where intended. (gvariable-location-views): Adjust. * target.def (reset_location_view): New. * doc/tm.texi.in (DWARF2_ASM_VIEW_DEBUG_INFO): New. (TARGET_RESET_LOCATION_VIEW): New. * doc/tm.texi: Rebuilt. * dwarf2out.c (dwarf2out_default_as_loc_support): New. (dwarf2out_default_as_locview_support): New. (output_asm_line_debug_info): Use option variables. (dwarf2out_maybe_output_loclist_view_pair): Likewise. (output_loc_list): Likewise. (add_high_low_attributes): Check option variables. Don't output entry view attribute in strict mode. (gen_inlined_subroutine_die): Check option variables. (dwarf2out_inline_entry): Likewise. (init_sections_and_labels): Likewise. (dwarf2out_early_finish): Likewise. (maybe_reset_location_view): New, from... (dwarf2out_var_location): ... here. Call it. * debug.h (dwarf2out_default_as_loc_support): Declare. (dwarf2out_default_as_locview_support): Declare. * hooks.c (hook_int_rtx_insn_0): New. * hooks.h (hook_int_rtx_insn_0): Declare. * toplev.c (process_options): Take -gas-loc-support and -gas-locview-support from dwarf2out. Enable -gvariable-location-views by default only with locview assembler support. Enable -ginternal-reset-location-views by default only if the target defines the corresponding hook. Enable -ginline-points by default if location views are enabled; force it disabled if statement frontiers are disabled. * tree-inline.c (expand_call_inline): Check option variables. * tree-ssa-live.c (remove_unused_scope_block_p): Likewise. --- gcc/common.opt | 16 ++++ gcc/debug.h | 2 + gcc/doc/invoke.texi | 106 +++++++++++++++++++++++------ gcc/doc/tm.texi | 23 ++++++ gcc/doc/tm.texi.in | 9 ++ gcc/dwarf2out.c | 186 +++++++++++++++++++++++++++++++++------------------ gcc/hooks.c | 6 ++ gcc/hooks.h | 1 gcc/target.def | 17 +++++ gcc/toplev.c | 45 +++++++++++- gcc/tree-inline.c | 2 - gcc/tree-ssa-live.c | 4 + 12 files changed, 324 insertions(+), 93 deletions(-) diff --git a/gcc/common.opt b/gcc/common.opt index 40ec0088c57e..e0bc4d1bb18d 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2880,6 +2880,14 @@ g Common Driver RejectNegative JoinedOrMissing Generate debug information in default format. +gas-loc-support +Common Driver Var(dwarf2out_as_loc_support) Init(2) +Assume assembler support for (DWARF2+) .loc directives + +gas-locview-support +Common Driver Var(dwarf2out_as_locview_support) Init(2) +Assume assembler support for view in (DWARF2+) .loc directives + gcoff Common Driver Ignore Warn(switch %qs no longer supported) Does nothing. Preserved for backward compatibility. @@ -2912,6 +2920,14 @@ ggdb Common Driver JoinedOrMissing Generate debug information in default extended format. +ginline-points +Common Driver Var(debug_inline_points) Init(2) +Generate extended entry point information for inlined functions + +ginternal-reset-location-views +Common Driver Var(debug_internal_reset_location_views) Init(2) +Compute locview reset points based on insn length estimates + gno- RejectNegative Joined Undocumented ; Catch the gno- prefix, so it doesn't backtrack to g<level>. diff --git a/gcc/debug.h b/gcc/debug.h index e1dfe4befaa1..126e56e8c8d7 100644 --- a/gcc/debug.h +++ b/gcc/debug.h @@ -248,6 +248,8 @@ extern bool dwarf2out_do_eh_frame (void); extern bool dwarf2out_do_frame (void); extern bool dwarf2out_do_cfi_asm (void); extern void dwarf2out_switch_text_section (void); +extern bool dwarf2out_default_as_loc_support (void); +extern bool dwarf2out_default_as_locview_support (void); /* For -fdump-go-spec. */ diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index df357bea7dc2..f45577e77b4e 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -346,9 +346,13 @@ Objective-C and Objective-C++ Dialects}. @gccoptlist{-g -g@var{level} -gdwarf -gdwarf-@var{version} @gol -ggdb -grecord-gcc-switches -gno-record-gcc-switches @gol -gstabs -gstabs+ -gstrict-dwarf -gno-strict-dwarf @gol +-gas-loc-support -gno-as-loc-support @gol +-gas-locview-support -gno-as-locview-support @gol -gcolumn-info -gno-column-info @gol -gstatement-frontiers -gno-statement-frontiers @gol -gvariable-location-views -gno-variable-location-views @gol +-ginternal-reset-location-views -gno-internal-reset-location-views @gol +-ginline-points -gno-inline-points @gol -gvms -gxcoff -gxcoff+ -gz@r{[}=@var{type}@r{]} @gol -fdebug-prefix-map=@var{old}=@var{new} -fdebug-types-section @gol -fno-eliminate-unused-debug-types @gol @@ -4159,7 +4163,7 @@ result in false positives. @table @gcctabopt @item -Wformat-overflow -@item -Wformat-overflow=1 +@itemx -Wformat-overflow=1 @opindex Wformat-overflow @opindex Wno-format-overflow Level @var{1} of @option{-Wformat-overflow} enabled by @option{-Wformat} @@ -4278,7 +4282,7 @@ logic @option{-Wformat-overflow}. @table @gcctabopt @item -Wformat-truncation -@item -Wformat-truncation=1 +@itemx -Wformat-truncation=1 @opindex Wformat-truncation @opindex Wno-format-overflow Level @var{1} of @option{-Wformat-truncation} enabled by @option{-Wformat} @@ -5239,7 +5243,7 @@ Option @option{-Wstringop-overflow=2} is enabled by default. @table @gcctabopt @item -Wstringop-overflow -@item -Wstringop-overflow=1 +@itemx -Wstringop-overflow=1 @opindex Wstringop-overflow @opindex Wno-stringop-overflow The @option{-Wstringop-overflow=1} option uses type-zero Object Size Checking @@ -7214,7 +7218,7 @@ and on some objects @code{.debug_types} produces larger instead of smaller debugging information. @item -grecord-gcc-switches -@item -gno-record-gcc-switches +@itemx -gno-record-gcc-switches @opindex grecord-gcc-switches @opindex gno-record-gcc-switches This switch causes the command-line options used to invoke the @@ -7237,8 +7241,38 @@ DWARF extensions from later standard versions is allowed. Allow using extensions of later DWARF standard version than selected with @option{-gdwarf-@var{version}}. +@item -gas-loc-support +@opindex gas-loc-support +Inform the compiler that the assembler supports @code{.loc} directives. +It may then use them for the assembler to generate DWARF2+ line number +tables. + +This is generally desirable, because assembler-generated line-number +tables are a lot more compact than those the compiler can generate +itself. + +This option will be enabled by default if, at GCC configure time, the +assembler was found to support such directives. + +@item -gno-as-loc-support +@opindex gno-as-loc-support +Force GCC to generate DWARF2+ line number tables internally, if DWARF2+ +line number tables are to be generated. + +@item gas-locview-support +@opindex gas-locview-support +Inform the compiler that the assembler supports @code{view} assignment +and reset assertion checking in @code{.loc} directives. + +This option will be enabled by default if, at GCC configure time, the +assembler was found to support them. + +@item gno-as-locview-support +Force GCC to assign view numbers internally, if +@option{-gvariable-location-views} are explicitly requested. + @item -gcolumn-info -@item -gno-column-info +@itemx -gno-column-info @opindex gcolumn-info @opindex gno-column-info Emit location column information into DWARF debugging information, rather @@ -7246,7 +7280,7 @@ than just file and line. This option is enabled by default. @item -gstatement-frontiers -@item -gno-statement-frontiers +@itemx -gno-statement-frontiers @opindex gstatement-frontiers @opindex gno-statement-frontiers This option causes GCC to create markers in the internal representation @@ -7257,8 +7291,8 @@ compiling with optimization (@option{-Os}, @option{-O}, @option{-O2}, @dots{}), and outputting DWARF 2 debug information at the normal level. @item -gvariable-location-views -@item -gvariable-location-views=incompat5 -@item -gno-variable-location-views +@itemx -gvariable-location-views=incompat5 +@itemx -gno-variable-location-views @opindex gvariable-location-views @opindex gvariable-location-views=incompat5 @opindex gno-variable-location-views @@ -7272,9 +7306,15 @@ which generally makes them somewhat less compact. The augmented line number tables and location lists are fully backward-compatible, so they can be consumed by debug information consumers that are not aware of these augmentations, but they won't derive any benefit from them either. + This is enabled by default when outputting DWARF 2 debug information at -the normal level, as long as @option{-fvar-tracking-assignments} is -enabled and @option{-gstrict-dwarf} is not. +the normal level, as long as there is assembler support, +@option{-fvar-tracking-assignments} is enabled and +@option{-gstrict-dwarf} is not. When assembler support is not +available, this may still be enabled, but it will force GCC to output +internal line number tables, and if +@option{-ginternal-reset-location-views} is not enabled, that will most +certainly lead to silently mismatching location views. There is a proposed representation for view numbers that is not backward compatible with the location list format introduced in DWARF 5, that can @@ -7284,6 +7324,30 @@ implementation of the proposed representation. Debug information consumers are not expected to support this extended format, and they would be rendered unable to decode location lists using it. +@item -ginternal-reset-location-views +@itemx -gnointernal-reset-location-views +@opindex ginternal-reset-location-views +@opindex gno-internal-reset-location-views +Attempt to determine location views that can be omitted from location +view lists. This requires the compiler to have very accurate insn +length estimates, which isn't always the case, and it may cause +incorrect view lists to be generated silently when using an assembler +that does not support location view lists. The GNU assembler will flag +any such error as a @code{view number mismatch}. This is only enabled +on ports that define a reliable estimation function. + +@item -ginline-points +@itemx -gno-inline-points +@opindex ginline-points +@opindex gno-inline-points +Generate extended debug information for inlined functions. Location +view tracking markers are inserted at inlined entry points, so that +address and view numbers can be computed and output in debug +information. This can be enabled independently of location views, in +which case the view numbers won't be output, but it can only be enabled +along with statement frontiers, and it is only enabled by default if +location views are enabled. + @item -gz@r{[}=@var{type}@r{]} @opindex gz Produce compressed debug sections in DWARF format, if that is supported. @@ -10043,7 +10107,7 @@ also use other heuristics to decide whether if-conversion is likely to be profitable. @item max-rtl-if-conversion-predictable-cost -@item max-rtl-if-conversion-unpredictable-cost +@itemx max-rtl-if-conversion-unpredictable-cost RTL if-conversion will try to remove conditional branches around a block and replace them with conditionally executed instructions. These parameters give the maximum permissible cost for the sequence that would be generated @@ -10767,7 +10831,7 @@ parameters only when their cumulative size is less or equal to pointer parameter. @item sra-max-scalarization-size-Ospeed -@item sra-max-scalarization-size-Osize +@itemx sra-max-scalarization-size-Osize The two Scalar Reduction of Aggregates passes (SRA and IPA-SRA) aim to replace scalar parts of aggregates with uses of independent scalar variables. These parameters control the maximum size, in storage units, @@ -14544,7 +14608,7 @@ This erratum workaround is made at link time and this will only pass the corresponding flag to the linker. @item -mlow-precision-recip-sqrt -@item -mno-low-precision-recip-sqrt +@itemx -mno-low-precision-recip-sqrt @opindex mlow-precision-recip-sqrt @opindex mno-low-precision-recip-sqrt Enable or disable the reciprocal square root approximation. @@ -14554,7 +14618,7 @@ precision of reciprocal square root results to about 16 bits for single precision and to 32 bits for double precision. @item -mlow-precision-sqrt -@item -mno-low-precision-sqrt +@itemx -mno-low-precision-sqrt @opindex -mlow-precision-sqrt @opindex -mno-low-precision-sqrt Enable or disable the square root approximation. @@ -14565,7 +14629,7 @@ single precision and to 32 bits for double precision. If enabled, it implies @option{-mlow-precision-recip-sqrt}. @item -mlow-precision-div -@item -mno-low-precision-div +@itemx -mno-low-precision-div @opindex -mlow-precision-div @opindex -mno-low-precision-div Enable or disable the division approximation. @@ -20193,7 +20257,7 @@ for regression testing of mixed MIPS16/non-MIPS16 code generation, and is not intended for ordinary use in compiling user code. @item -minterlink-compressed -@item -mno-interlink-compressed +@itemx -mno-interlink-compressed @opindex minterlink-compressed @opindex mno-interlink-compressed Require (do not require) that code using the standard (uncompressed) MIPS ISA @@ -20774,7 +20838,7 @@ Tell the MIPS assembler to not run its preprocessor over user assembler files (with a @samp{.s} suffix) when assembling them. @item -mfix-24k -@item -mno-fix-24k +@itemx -mno-fix-24k @opindex mfix-24k @opindex mno-fix-24k Work around the 24K E48 (lost data on stores during refill) errata. @@ -21534,7 +21598,7 @@ into the small data or BSS sections instead of the normal data or BSS sections. The default value of @var{num} is 8. @item -mgpopt=@var{option} -@item -mgpopt +@itemx -mgpopt @itemx -mno-gpopt @opindex mgpopt @opindex mno-gpopt @@ -23093,7 +23157,7 @@ or 32 bits (@option{-m32bit-doubles}) in size. The default is @option{-m32bit-doubles}. @item -msave-mduc-in-interrupts -@item -mno-save-mduc-in-interrupts +@itemx -mno-save-mduc-in-interrupts @opindex msave-mduc-in-interrupts @opindex mno-save-mduc-in-interrupts Specifies that interrupt handler functions should preserve the @@ -26726,13 +26790,13 @@ comparisons. These correctly handle the case where the result of a comparison is unordered. @item -m80387 -@item -mhard-float +@itemx -mhard-float @opindex 80387 @opindex mhard-float Generate output containing 80387 instructions for floating point. @item -mno-80387 -@item -msoft-float +@itemx -msoft-float @opindex no-80387 @opindex msoft-float Generate output containing library calls for floating point. diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index ddf48cb4b4d2..bd8b917ba829 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -9966,6 +9966,29 @@ line debug info sections. This will result in much more compact line number tables, and hence is desirable if it works. @end defmac +@defmac DWARF2_ASM_VIEW_DEBUG_INFO +Define this macro to be a nonzero value if the assembler supports view +assignment and verification in @code{.loc}. If it does not, but the +user enables location views, the compiler may have to fallback to +internal line number tables. +@end defmac + +@deftypefn {Target Hook} int TARGET_RESET_LOCATION_VIEW (rtx_insn *@var{}) +This hook, if defined, enables -ginternal-reset-location-views, and +uses its result to override cases in which the estimated min insn +length might be nonzero even when a PC advance (i.e., a view reset) +cannot be taken for granted. + +If the hook is defined, it must return a positive value to indicate +the insn definitely advances the PC, and so the view number can be +safely assumed to be reset; a negative value to mean the insn +definitely does not advance the PC, and os the view number must not +be reset; or zero to decide based on the estimated insn length. + +If insn length is to be regarded as reliable, set the hook to +@code{hook_int_rtx_insn_0}. +@end deftypefn + @deftypevr {Target Hook} bool TARGET_WANT_DEBUG_PUB_SECTIONS True if the @code{.debug_pubtypes} and @code{.debug_pubnames} sections should be emitted. These sections are not used on most platforms, and in particular GDB does not use them. @end deftypevr diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 0aab45f4992c..b0207146e8c2 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -6921,6 +6921,15 @@ line debug info sections. This will result in much more compact line number tables, and hence is desirable if it works. @end defmac +@defmac DWARF2_ASM_VIEW_DEBUG_INFO +Define this macro to be a nonzero value if the assembler supports view +assignment and verification in @code{.loc}. If it does not, but the +user enables location views, the compiler may have to fallback to +internal line number tables. +@end defmac + +@hook TARGET_RESET_LOCATION_VIEW + @hook TARGET_WANT_DEBUG_PUB_SECTIONS @hook TARGET_DELAY_SCHED2 diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 984df9fe4e9a..4e8d40bc91f2 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2957,6 +2957,37 @@ struct GTY(()) dw_line_info_table { vec<dw_line_info_entry, va_gc> *entries; }; +/* If we're keep track of location views and their reset points, and + INSN is a reset point (i.e., it necessarily advances the PC), mark + the next view in TABLE as reset. */ + +static void +maybe_reset_location_view (rtx_insn *insn, dw_line_info_table *table) +{ + if (!debug_internal_reset_location_views) + return; + + /* Maybe turn (part of?) this test into a default target hook. */ + int reset = 0; + + if (targetm.reset_location_view) + reset = targetm.reset_location_view (insn); + + if (reset) + ; + else if (JUMP_TABLE_DATA_P (insn)) + reset = 1; + else if (GET_CODE (insn) == USE + || GET_CODE (insn) == CLOBBER + || GET_CODE (insn) == ASM_INPUT + || asm_noperands (insn) >= 0) + ; + else if (get_attr_min_length (insn) > 0) + reset = 1; + + if (reset > 0) + RESET_NEXT_VIEW (table->view); +} /* Each DIE attribute has a field specifying the attribute kind, a link to the next attribute in the chain, and an attribute value. @@ -3164,6 +3195,29 @@ skeleton_chain_node; #endif #endif +/* Return true if GCC configure detected assembler support for .loc. */ + +bool +dwarf2out_default_as_loc_support (void) +{ + return DWARF2_ASM_LINE_DEBUG_INFO; +#if (GCC_VERSION >= 3000) + #pragma GCC poison DWARF2_ASM_LINE_DEBUG_INFO +#endif +} + +/* Return true if GCC configure detected assembler support for views + in .loc directives. */ + +bool +dwarf2out_default_as_locview_support (void) +{ + return DWARF2_ASM_VIEW_DEBUG_INFO; +#if (GCC_VERSION >= 3000) + #pragma GCC poison DWARF2_ASM_VIEW_DEBUG_INFO +#endif +} + /* A bit is set in ZERO_VIEW_P if we are using the assembler-supported view computation, and it refers to a view identifier for which we will not emit a label because it is known to map to a view number @@ -3215,9 +3269,9 @@ static GTY(()) bitmap zero_view_p; static bool output_asm_line_debug_info (void) { - return (DWARF2_ASM_VIEW_DEBUG_INFO - || (DWARF2_ASM_LINE_DEBUG_INFO - && !debug_variable_location_views)); + return (dwarf2out_as_loc_support + && (dwarf2out_as_locview_support + || !debug_variable_location_views)); } /* Minimum line offset in a special line info. opcode. @@ -9950,28 +10004,31 @@ dwarf2out_maybe_output_loclist_view_pair (dw_loc_list_ref curr) #ifdef DW_LLE_view_pair dw2_asm_output_data (1, DW_LLE_view_pair, "DW_LLE_view_pair"); -# if DWARF2_ASM_VIEW_DEBUG_INFO - if (ZERO_VIEW_P (curr->vbegin)) - dw2_asm_output_data_uleb128 (0, "Location view begin"); - else + if (dwarf2out_as_locview_support) { - char label[MAX_ARTIFICIAL_LABEL_BYTES]; - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); - dw2_asm_output_symname_uleb128 (label, "Location view begin"); - } + if (ZERO_VIEW_P (curr->vbegin)) + dw2_asm_output_data_uleb128 (0, "Location view begin"); + else + { + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); + dw2_asm_output_symname_uleb128 (label, "Location view begin"); + } - if (ZERO_VIEW_P (curr->vend)) - dw2_asm_output_data_uleb128 (0, "Location view end"); + if (ZERO_VIEW_P (curr->vend)) + dw2_asm_output_data_uleb128 (0, "Location view end"); + else + { + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); + dw2_asm_output_symname_uleb128 (label, "Location view end"); + } + } else { - char label[MAX_ARTIFICIAL_LABEL_BYTES]; - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); - dw2_asm_output_symname_uleb128 (label, "Location view end"); + dw2_asm_output_data_uleb128 (curr->vbegin, "Location view begin"); + dw2_asm_output_data_uleb128 (curr->vend, "Location view end"); } -# else /* !DWARF2_ASM_VIEW_DEBUG_INFO */ - dw2_asm_output_data_uleb128 (curr->vbegin, "Location view begin"); - dw2_asm_output_data_uleb128 (curr->vend, "Location view end"); -# endif /* DWARF2_ASM_VIEW_DEBUG_INFO */ #endif /* DW_LLE_view_pair */ return; @@ -10001,40 +10058,43 @@ output_loc_list (dw_loc_list_ref list_head) vcount++; /* ?? dwarf_split_debug_info? */ -#if DWARF2_ASM_VIEW_DEBUG_INFO - char label[MAX_ARTIFICIAL_LABEL_BYTES]; - - if (!ZERO_VIEW_P (curr->vbegin)) + if (dwarf2out_as_locview_support) { - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); - dw2_asm_output_symname_uleb128 (label, - "View list begin (%s)", - list_head->vl_symbol); + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + + if (!ZERO_VIEW_P (curr->vbegin)) + { + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); + dw2_asm_output_symname_uleb128 (label, + "View list begin (%s)", + list_head->vl_symbol); + } + else + dw2_asm_output_data_uleb128 (0, + "View list begin (%s)", + list_head->vl_symbol); + + if (!ZERO_VIEW_P (curr->vend)) + { + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); + dw2_asm_output_symname_uleb128 (label, + "View list end (%s)", + list_head->vl_symbol); + } + else + dw2_asm_output_data_uleb128 (0, + "View list end (%s)", + list_head->vl_symbol); } else - dw2_asm_output_data_uleb128 (0, - "View list begin (%s)", - list_head->vl_symbol); - - if (!ZERO_VIEW_P (curr->vend)) { - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); - dw2_asm_output_symname_uleb128 (label, - "View list end (%s)", - list_head->vl_symbol); + dw2_asm_output_data_uleb128 (curr->vbegin, + "View list begin (%s)", + list_head->vl_symbol); + dw2_asm_output_data_uleb128 (curr->vend, + "View list end (%s)", + list_head->vl_symbol); } - else - dw2_asm_output_data_uleb128 (0, - "View list end (%s)", - list_head->vl_symbol); -#else /* !DWARF2_ASM_VIEW_DEBUG_INFO */ - dw2_asm_output_data_uleb128 (curr->vbegin, - "View list begin (%s)", - list_head->vl_symbol); - dw2_asm_output_data_uleb128 (curr->vend, - "View list end (%s)", - list_head->vl_symbol); -#endif } } @@ -23721,11 +23781,14 @@ add_high_low_attributes (tree stmt, dw_die_ref die) { inline_entry_data *ied = *iedp; gcc_assert (MAY_HAVE_DEBUG_MARKER_INSNS); + gcc_assert (debug_inline_points); gcc_assert (inlined_function_outer_scope_p (stmt)); + ASM_GENERATE_INTERNAL_LABEL (label, ied->label_pfx, ied->label_num); add_AT_lbl_id (die, DW_AT_entry_pc, label); - if (debug_variable_location_views && !ZERO_VIEW_P (ied->view)) + if (debug_variable_location_views && !ZERO_VIEW_P (ied->view) + && !dwarf_strict) { if (!output_asm_line_debug_info ()) add_AT_unsigned (die, DW_AT_GNU_entry_view, ied->view); @@ -23756,7 +23819,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die) dw_die_ref pdie; dw_attr_node *attr = NULL; - if (!MAY_HAVE_DEBUG_MARKER_INSNS && inlined_function_outer_scope_p (stmt)) + if (!debug_inline_points && inlined_function_outer_scope_p (stmt)) { ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_BEGIN_LABEL, BLOCK_NUMBER (stmt)); @@ -23921,7 +23984,7 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die) dw_die_ref subr_die = new_die (DW_TAG_inlined_subroutine, context_die, stmt); - if (call_arg_locations || MAY_HAVE_DEBUG_MARKER_INSNS) + if (call_arg_locations || debug_inline_points) BLOCK_DIE (stmt) = subr_die; add_abstract_origin_attribute (subr_die, decl); if (TREE_ASM_WRITTEN (stmt)) @@ -26906,7 +26969,7 @@ dwarf2out_var_location (rtx_insn *loc_note) { if (CALL_P (loc_note)) { - RESET_NEXT_VIEW (cur_line_info_table->view); + maybe_reset_location_view (loc_note, cur_line_info_table); call_site_count++; if (SIBLING_CALL_P (loc_note)) tail_call_site_count++; @@ -26942,15 +27005,8 @@ dwarf2out_var_location (rtx_insn *loc_note) } else if (!debug_variable_location_views) gcc_unreachable (); - else if (JUMP_TABLE_DATA_P (loc_note)) - RESET_NEXT_VIEW (cur_line_info_table->view); - else if (GET_CODE (loc_note) == USE - || GET_CODE (loc_note) == CLOBBER - || GET_CODE (loc_note) == ASM_INPUT - || asm_noperands (loc_note) >= 0) - ; - else if (get_attr_min_length (loc_note) > 0) - RESET_NEXT_VIEW (cur_line_info_table->view); + else + maybe_reset_location_view (loc_note, cur_line_info_table); return; } @@ -27219,6 +27275,8 @@ block_within_block_p (tree block, tree outer, bool bothways) static void dwarf2out_inline_entry (tree block) { + gcc_assert (debug_inline_points); + /* If we can't represent it, don't bother. */ if (!(dwarf_version >= 3 || !dwarf_strict)) return; @@ -28233,7 +28291,7 @@ init_sections_and_labels (bool early_lto_debug) debug_str_section = get_section (DEBUG_LTO_STR_SECTION, DEBUG_STR_SECTION_FLAGS | SECTION_EXCLUDE, NULL); - if (!dwarf_split_debug_info && !DWARF2_ASM_LINE_DEBUG_INFO) + if (!dwarf_split_debug_info && !dwarf2out_as_loc_support) debug_line_str_section = get_section (DEBUG_LTO_LINE_STR_SECTION, DEBUG_STR_SECTION_FLAGS | SECTION_EXCLUDE, NULL); @@ -31468,7 +31526,7 @@ dwarf2out_early_finish (const char *filename) /* When emitting DWARF5 .debug_line_str, move DW_AT_name and DW_AT_comp_dir into .debug_line_str section. */ - if (!DWARF2_ASM_LINE_DEBUG_INFO + if (!dwarf2out_as_loc_support && dwarf_version >= 5 && DWARF5_USE_DEBUG_LINE_STR) { diff --git a/gcc/hooks.c b/gcc/hooks.c index 61719606a36d..780cc1e08631 100644 --- a/gcc/hooks.c +++ b/gcc/hooks.c @@ -235,6 +235,12 @@ hook_int_rtx_1 (rtx) return 1; } +int +hook_int_rtx_insn_0 (rtx_insn *) +{ + return 0; +} + int hook_int_rtx_insn_unreachable (rtx_insn *) { diff --git a/gcc/hooks.h b/gcc/hooks.h index 8caedd429a69..0ed5b952b48e 100644 --- a/gcc/hooks.h +++ b/gcc/hooks.h @@ -93,6 +93,7 @@ extern int hook_int_const_tree_0 (const_tree); extern int hook_int_const_tree_const_tree_1 (const_tree, const_tree); extern int hook_int_rtx_0 (rtx); extern int hook_int_rtx_1 (rtx); +extern int hook_int_rtx_insn_0 (rtx_insn *); extern int hook_int_rtx_insn_unreachable (rtx_insn *); extern int hook_int_rtx_bool_0 (rtx, bool); extern int hook_int_rtx_mode_as_bool_0 (rtx, machine_mode, addr_space_t, diff --git a/gcc/target.def b/gcc/target.def index aeb41df19454..c5b2a1e7e71f 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -6447,6 +6447,23 @@ This will suppress generation of the normal debug frame unwind information.", enum unwind_info_type, (void), default_debug_unwind_info) +DEFHOOK +(reset_location_view, "\ +This hook, if defined, enables -ginternal-reset-location-views, and\n\ +uses its result to override cases in which the estimated min insn\n\ +length might be nonzero even when a PC advance (i.e., a view reset)\n\ +cannot be taken for granted.\n\ +\n\ +If the hook is defined, it must return a positive value to indicate\n\ +the insn definitely advances the PC, and so the view number can be\n\ +safely assumed to be reset; a negative value to mean the insn\n\ +definitely does not advance the PC, and os the view number must not\n\ +be reset; or zero to decide based on the estimated insn length.\n\ +\n\ +If insn length is to be regarded as reliable, set the hook to\n\ +@code{hook_int_rtx_insn_0}.", + int, (rtx_insn *), NULL) + /* The code parameter should be of type enum rtx_code but this is not defined at this time. */ DEFHOOK diff --git a/gcc/toplev.c b/gcc/toplev.c index 23db0636fc79..b066bcc72297 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1558,13 +1558,23 @@ process_options (void) || write_symbols == VMS_AND_DWARF2_DEBUG) && !(flag_selective_scheduling || flag_selective_scheduling2)); + if (dwarf2out_as_loc_support == AUTODETECT_VALUE) + dwarf2out_as_loc_support + = dwarf2out_default_as_loc_support (); + if (dwarf2out_as_locview_support == AUTODETECT_VALUE) + dwarf2out_as_locview_support + = dwarf2out_default_as_locview_support (); + if (debug_variable_location_views == AUTODETECT_VALUE) { - debug_variable_location_views = flag_var_tracking - && debug_info_level >= DINFO_LEVEL_NORMAL - && (write_symbols == DWARF2_DEBUG - || write_symbols == VMS_AND_DWARF2_DEBUG) - && !dwarf_strict; + debug_variable_location_views + = (flag_var_tracking + && debug_info_level >= DINFO_LEVEL_NORMAL + && (write_symbols == DWARF2_DEBUG + || write_symbols == VMS_AND_DWARF2_DEBUG) + && !dwarf_strict + && dwarf2out_as_loc_support + && dwarf2out_as_locview_support); } else if (debug_variable_location_views == -1 && dwarf_version != 5) { @@ -1574,6 +1584,31 @@ process_options (void) debug_variable_location_views = 1; } + if (debug_internal_reset_location_views == 2) + { + debug_internal_reset_location_views + = (debug_variable_location_views + && targetm.reset_location_view); + } + else if (debug_internal_reset_location_views + && !debug_variable_location_views) + { + warning_at (UNKNOWN_LOCATION, 0, + "-ginternal-reset-location-views is forced disabled " + "without -gvariable-location-views"); + debug_internal_reset_location_views = 0; + } + + if (debug_inline_points == AUTODETECT_VALUE) + debug_inline_points = debug_variable_location_views; + else if (debug_inline_points && !debug_nonbind_markers_p) + { + warning_at (UNKNOWN_LOCATION, 0, + "-ginline-points is forced disabled without " + "-gstatement-frontiers"); + debug_inline_points = 0; + } + if (flag_tree_cselim == AUTODETECT_VALUE) { if (HAVE_conditional_move) diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 7f9ec770e197..811829e85299 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -4605,7 +4605,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id) GSI_NEW_STMT); } initialize_inlined_parameters (id, stmt, fn, bb); - if (debug_nonbind_markers_p && id->block + if (debug_nonbind_markers_p && debug_inline_points && id->block && inlined_function_outer_scope_p (id->block)) { gimple_stmt_iterator si = gsi_last_bb (bb); diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c index 26da31f74cb2..62bb3c5de659 100644 --- a/gcc/tree-ssa-live.c +++ b/gcc/tree-ssa-live.c @@ -522,7 +522,7 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block) unused = false; /* Preserve the block, it is referenced by at least the inline entry point marker. */ - else if (debug_nonbind_markers_p + else if (debug_inline_points && inlined_function_outer_scope_p (scope)) unused = false; /* Innermost blocks with no live variables nor statements can be always @@ -558,7 +558,7 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_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 + else if (!debug_inline_points && inlined_function_outer_scope_p (scope)) unused = false; else
On Feb 12, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > This patch supersedes the previous one. Testing underway... Ok if it > succeeds? I failed to update the patch I posted after making a correct to symbol poisoning, that had caused builds to fail right away, sorry. Thanks, Rainer, for catching the error. Here's the patch that actually passed regstrap on native i686 and x86_64-linux-gnu, and fixed numerous regressions on cross builds. Ok to install? [LVU, IEPM] several new controlling options Given that the minimum insn length is not generally reliable to tell whether an insn actually advances PC, this patch disables the locview list optimizations that can only be done when can tell it. The preexisting logic is retained, however, and can be enabled with the newly-introduced -ginternal-reset-location-view. This is now enabled by default only if the target defines a hook that may override or defer to the preexisting logic. The negated command line option can then be used should errors still be encountered. We also introduce options to control whether to assume .loc and view support in the assembler, and to control whether to output inline entry points (and views) from markers. This patch also fixes a number of documentation formatting errors, namely using @item rather than @itemx for all but the first of several options before a description. for gcc/ChangeLog * common.opt (gas-loc-support, gas-locview-support): New. (ginline-points, ginternal-reset-location-views): New. * doc/invoke.texi: Document them. Use @itemx where intended. (gvariable-location-views): Adjust. * target.def (reset_location_view): New. * doc/tm.texi.in (DWARF2_ASM_VIEW_DEBUG_INFO): New. (TARGET_RESET_LOCATION_VIEW): New. * doc/tm.texi: Rebuilt. * dwarf2out.c (dwarf2out_default_as_loc_support): New. (dwarf2out_default_as_locview_support): New. (output_asm_line_debug_info): Use option variables. (dwarf2out_maybe_output_loclist_view_pair): Likewise. (output_loc_list): Likewise. (add_high_low_attributes): Check option variables. Don't output entry view attribute in strict mode. (gen_inlined_subroutine_die): Check option variables. (dwarf2out_inline_entry): Likewise. (init_sections_and_labels): Likewise. (dwarf2out_early_finish): Likewise. (maybe_reset_location_view): New, from... (dwarf2out_var_location): ... here. Call it. * debug.h (dwarf2out_default_as_loc_support): Declare. (dwarf2out_default_as_locview_support): Declare. * hooks.c (hook_int_rtx_insn_0): New. * hooks.h (hook_int_rtx_insn_0): Declare. * toplev.c (process_options): Take -gas-loc-support and -gas-locview-support from dwarf2out. Enable -gvariable-location-views by default only with locview assembler support. Enable -ginternal-reset-location-views by default only if the target defines the corresponding hook. Enable -ginline-points by default if location views are enabled; force it disabled if statement frontiers are disabled. * tree-inline.c (expand_call_inline): Check option variables. * tree-ssa-live.c (remove_unused_scope_block_p): Likewise. --- gcc/common.opt | 16 ++++ gcc/debug.h | 2 + gcc/doc/invoke.texi | 106 +++++++++++++++++++++++------ gcc/doc/tm.texi | 23 ++++++ gcc/doc/tm.texi.in | 9 ++ gcc/dwarf2out.c | 188 ++++++++++++++++++++++++++++++++++----------------- gcc/hooks.c | 6 ++ gcc/hooks.h | 1 gcc/target.def | 17 +++++ gcc/toplev.c | 45 +++++++++++- gcc/tree-inline.c | 2 - gcc/tree-ssa-live.c | 4 + 12 files changed, 326 insertions(+), 93 deletions(-) diff --git a/gcc/common.opt b/gcc/common.opt index 40ec0088c57e..e0bc4d1bb18d 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2880,6 +2880,14 @@ g Common Driver RejectNegative JoinedOrMissing Generate debug information in default format. +gas-loc-support +Common Driver Var(dwarf2out_as_loc_support) Init(2) +Assume assembler support for (DWARF2+) .loc directives + +gas-locview-support +Common Driver Var(dwarf2out_as_locview_support) Init(2) +Assume assembler support for view in (DWARF2+) .loc directives + gcoff Common Driver Ignore Warn(switch %qs no longer supported) Does nothing. Preserved for backward compatibility. @@ -2912,6 +2920,14 @@ ggdb Common Driver JoinedOrMissing Generate debug information in default extended format. +ginline-points +Common Driver Var(debug_inline_points) Init(2) +Generate extended entry point information for inlined functions + +ginternal-reset-location-views +Common Driver Var(debug_internal_reset_location_views) Init(2) +Compute locview reset points based on insn length estimates + gno- RejectNegative Joined Undocumented ; Catch the gno- prefix, so it doesn't backtrack to g<level>. diff --git a/gcc/debug.h b/gcc/debug.h index e1dfe4befaa1..126e56e8c8d7 100644 --- a/gcc/debug.h +++ b/gcc/debug.h @@ -248,6 +248,8 @@ extern bool dwarf2out_do_eh_frame (void); extern bool dwarf2out_do_frame (void); extern bool dwarf2out_do_cfi_asm (void); extern void dwarf2out_switch_text_section (void); +extern bool dwarf2out_default_as_loc_support (void); +extern bool dwarf2out_default_as_locview_support (void); /* For -fdump-go-spec. */ diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 9db9d083a475..48194c825f33 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -346,9 +346,13 @@ Objective-C and Objective-C++ Dialects}. @gccoptlist{-g -g@var{level} -gdwarf -gdwarf-@var{version} @gol -ggdb -grecord-gcc-switches -gno-record-gcc-switches @gol -gstabs -gstabs+ -gstrict-dwarf -gno-strict-dwarf @gol +-gas-loc-support -gno-as-loc-support @gol +-gas-locview-support -gno-as-locview-support @gol -gcolumn-info -gno-column-info @gol -gstatement-frontiers -gno-statement-frontiers @gol -gvariable-location-views -gno-variable-location-views @gol +-ginternal-reset-location-views -gno-internal-reset-location-views @gol +-ginline-points -gno-inline-points @gol -gvms -gxcoff -gxcoff+ -gz@r{[}=@var{type}@r{]} @gol -fdebug-prefix-map=@var{old}=@var{new} -fdebug-types-section @gol -fno-eliminate-unused-debug-types @gol @@ -4159,7 +4163,7 @@ result in false positives. @table @gcctabopt @item -Wformat-overflow -@item -Wformat-overflow=1 +@itemx -Wformat-overflow=1 @opindex Wformat-overflow @opindex Wno-format-overflow Level @var{1} of @option{-Wformat-overflow} enabled by @option{-Wformat} @@ -4278,7 +4282,7 @@ logic @option{-Wformat-overflow}. @table @gcctabopt @item -Wformat-truncation -@item -Wformat-truncation=1 +@itemx -Wformat-truncation=1 @opindex Wformat-truncation @opindex Wno-format-overflow Level @var{1} of @option{-Wformat-truncation} enabled by @option{-Wformat} @@ -5239,7 +5243,7 @@ Option @option{-Wstringop-overflow=2} is enabled by default. @table @gcctabopt @item -Wstringop-overflow -@item -Wstringop-overflow=1 +@itemx -Wstringop-overflow=1 @opindex Wstringop-overflow @opindex Wno-stringop-overflow The @option{-Wstringop-overflow=1} option uses type-zero Object Size Checking @@ -7214,7 +7218,7 @@ and on some objects @code{.debug_types} produces larger instead of smaller debugging information. @item -grecord-gcc-switches -@item -gno-record-gcc-switches +@itemx -gno-record-gcc-switches @opindex grecord-gcc-switches @opindex gno-record-gcc-switches This switch causes the command-line options used to invoke the @@ -7237,8 +7241,38 @@ DWARF extensions from later standard versions is allowed. Allow using extensions of later DWARF standard version than selected with @option{-gdwarf-@var{version}}. +@item -gas-loc-support +@opindex gas-loc-support +Inform the compiler that the assembler supports @code{.loc} directives. +It may then use them for the assembler to generate DWARF2+ line number +tables. + +This is generally desirable, because assembler-generated line-number +tables are a lot more compact than those the compiler can generate +itself. + +This option will be enabled by default if, at GCC configure time, the +assembler was found to support such directives. + +@item -gno-as-loc-support +@opindex gno-as-loc-support +Force GCC to generate DWARF2+ line number tables internally, if DWARF2+ +line number tables are to be generated. + +@item gas-locview-support +@opindex gas-locview-support +Inform the compiler that the assembler supports @code{view} assignment +and reset assertion checking in @code{.loc} directives. + +This option will be enabled by default if, at GCC configure time, the +assembler was found to support them. + +@item gno-as-locview-support +Force GCC to assign view numbers internally, if +@option{-gvariable-location-views} are explicitly requested. + @item -gcolumn-info -@item -gno-column-info +@itemx -gno-column-info @opindex gcolumn-info @opindex gno-column-info Emit location column information into DWARF debugging information, rather @@ -7246,7 +7280,7 @@ than just file and line. This option is enabled by default. @item -gstatement-frontiers -@item -gno-statement-frontiers +@itemx -gno-statement-frontiers @opindex gstatement-frontiers @opindex gno-statement-frontiers This option causes GCC to create markers in the internal representation @@ -7257,8 +7291,8 @@ compiling with optimization (@option{-Os}, @option{-O}, @option{-O2}, @dots{}), and outputting DWARF 2 debug information at the normal level. @item -gvariable-location-views -@item -gvariable-location-views=incompat5 -@item -gno-variable-location-views +@itemx -gvariable-location-views=incompat5 +@itemx -gno-variable-location-views @opindex gvariable-location-views @opindex gvariable-location-views=incompat5 @opindex gno-variable-location-views @@ -7272,9 +7306,15 @@ which generally makes them somewhat less compact. The augmented line number tables and location lists are fully backward-compatible, so they can be consumed by debug information consumers that are not aware of these augmentations, but they won't derive any benefit from them either. + This is enabled by default when outputting DWARF 2 debug information at -the normal level, as long as @option{-fvar-tracking-assignments} is -enabled and @option{-gstrict-dwarf} is not. +the normal level, as long as there is assembler support, +@option{-fvar-tracking-assignments} is enabled and +@option{-gstrict-dwarf} is not. When assembler support is not +available, this may still be enabled, but it will force GCC to output +internal line number tables, and if +@option{-ginternal-reset-location-views} is not enabled, that will most +certainly lead to silently mismatching location views. There is a proposed representation for view numbers that is not backward compatible with the location list format introduced in DWARF 5, that can @@ -7284,6 +7324,30 @@ implementation of the proposed representation. Debug information consumers are not expected to support this extended format, and they would be rendered unable to decode location lists using it. +@item -ginternal-reset-location-views +@itemx -gnointernal-reset-location-views +@opindex ginternal-reset-location-views +@opindex gno-internal-reset-location-views +Attempt to determine location views that can be omitted from location +view lists. This requires the compiler to have very accurate insn +length estimates, which isn't always the case, and it may cause +incorrect view lists to be generated silently when using an assembler +that does not support location view lists. The GNU assembler will flag +any such error as a @code{view number mismatch}. This is only enabled +on ports that define a reliable estimation function. + +@item -ginline-points +@itemx -gno-inline-points +@opindex ginline-points +@opindex gno-inline-points +Generate extended debug information for inlined functions. Location +view tracking markers are inserted at inlined entry points, so that +address and view numbers can be computed and output in debug +information. This can be enabled independently of location views, in +which case the view numbers won't be output, but it can only be enabled +along with statement frontiers, and it is only enabled by default if +location views are enabled. + @item -gz@r{[}=@var{type}@r{]} @opindex gz Produce compressed debug sections in DWARF format, if that is supported. @@ -10043,7 +10107,7 @@ also use other heuristics to decide whether if-conversion is likely to be profitable. @item max-rtl-if-conversion-predictable-cost -@item max-rtl-if-conversion-unpredictable-cost +@itemx max-rtl-if-conversion-unpredictable-cost RTL if-conversion will try to remove conditional branches around a block and replace them with conditionally executed instructions. These parameters give the maximum permissible cost for the sequence that would be generated @@ -10768,7 +10832,7 @@ parameters only when their cumulative size is less or equal to pointer parameter. @item sra-max-scalarization-size-Ospeed -@item sra-max-scalarization-size-Osize +@itemx sra-max-scalarization-size-Osize The two Scalar Reduction of Aggregates passes (SRA and IPA-SRA) aim to replace scalar parts of aggregates with uses of independent scalar variables. These parameters control the maximum size, in storage units, @@ -14545,7 +14609,7 @@ This erratum workaround is made at link time and this will only pass the corresponding flag to the linker. @item -mlow-precision-recip-sqrt -@item -mno-low-precision-recip-sqrt +@itemx -mno-low-precision-recip-sqrt @opindex mlow-precision-recip-sqrt @opindex mno-low-precision-recip-sqrt Enable or disable the reciprocal square root approximation. @@ -14555,7 +14619,7 @@ precision of reciprocal square root results to about 16 bits for single precision and to 32 bits for double precision. @item -mlow-precision-sqrt -@item -mno-low-precision-sqrt +@itemx -mno-low-precision-sqrt @opindex -mlow-precision-sqrt @opindex -mno-low-precision-sqrt Enable or disable the square root approximation. @@ -14566,7 +14630,7 @@ single precision and to 32 bits for double precision. If enabled, it implies @option{-mlow-precision-recip-sqrt}. @item -mlow-precision-div -@item -mno-low-precision-div +@itemx -mno-low-precision-div @opindex -mlow-precision-div @opindex -mno-low-precision-div Enable or disable the division approximation. @@ -20194,7 +20258,7 @@ for regression testing of mixed MIPS16/non-MIPS16 code generation, and is not intended for ordinary use in compiling user code. @item -minterlink-compressed -@item -mno-interlink-compressed +@itemx -mno-interlink-compressed @opindex minterlink-compressed @opindex mno-interlink-compressed Require (do not require) that code using the standard (uncompressed) MIPS ISA @@ -20775,7 +20839,7 @@ Tell the MIPS assembler to not run its preprocessor over user assembler files (with a @samp{.s} suffix) when assembling them. @item -mfix-24k -@item -mno-fix-24k +@itemx -mno-fix-24k @opindex mfix-24k @opindex mno-fix-24k Work around the 24K E48 (lost data on stores during refill) errata. @@ -21535,7 +21599,7 @@ into the small data or BSS sections instead of the normal data or BSS sections. The default value of @var{num} is 8. @item -mgpopt=@var{option} -@item -mgpopt +@itemx -mgpopt @itemx -mno-gpopt @opindex mgpopt @opindex mno-gpopt @@ -23094,7 +23158,7 @@ or 32 bits (@option{-m32bit-doubles}) in size. The default is @option{-m32bit-doubles}. @item -msave-mduc-in-interrupts -@item -mno-save-mduc-in-interrupts +@itemx -mno-save-mduc-in-interrupts @opindex msave-mduc-in-interrupts @opindex mno-save-mduc-in-interrupts Specifies that interrupt handler functions should preserve the @@ -26727,13 +26791,13 @@ comparisons. These correctly handle the case where the result of a comparison is unordered. @item -m80387 -@item -mhard-float +@itemx -mhard-float @opindex 80387 @opindex mhard-float Generate output containing 80387 instructions for floating point. @item -mno-80387 -@item -msoft-float +@itemx -msoft-float @opindex no-80387 @opindex msoft-float Generate output containing library calls for floating point. diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index ddf48cb4b4d2..bd8b917ba829 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -9966,6 +9966,29 @@ line debug info sections. This will result in much more compact line number tables, and hence is desirable if it works. @end defmac +@defmac DWARF2_ASM_VIEW_DEBUG_INFO +Define this macro to be a nonzero value if the assembler supports view +assignment and verification in @code{.loc}. If it does not, but the +user enables location views, the compiler may have to fallback to +internal line number tables. +@end defmac + +@deftypefn {Target Hook} int TARGET_RESET_LOCATION_VIEW (rtx_insn *@var{}) +This hook, if defined, enables -ginternal-reset-location-views, and +uses its result to override cases in which the estimated min insn +length might be nonzero even when a PC advance (i.e., a view reset) +cannot be taken for granted. + +If the hook is defined, it must return a positive value to indicate +the insn definitely advances the PC, and so the view number can be +safely assumed to be reset; a negative value to mean the insn +definitely does not advance the PC, and os the view number must not +be reset; or zero to decide based on the estimated insn length. + +If insn length is to be regarded as reliable, set the hook to +@code{hook_int_rtx_insn_0}. +@end deftypefn + @deftypevr {Target Hook} bool TARGET_WANT_DEBUG_PUB_SECTIONS True if the @code{.debug_pubtypes} and @code{.debug_pubnames} sections should be emitted. These sections are not used on most platforms, and in particular GDB does not use them. @end deftypevr diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 0aab45f4992c..b0207146e8c2 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -6921,6 +6921,15 @@ line debug info sections. This will result in much more compact line number tables, and hence is desirable if it works. @end defmac +@defmac DWARF2_ASM_VIEW_DEBUG_INFO +Define this macro to be a nonzero value if the assembler supports view +assignment and verification in @code{.loc}. If it does not, but the +user enables location views, the compiler may have to fallback to +internal line number tables. +@end defmac + +@hook TARGET_RESET_LOCATION_VIEW + @hook TARGET_WANT_DEBUG_PUB_SECTIONS @hook TARGET_DELAY_SCHED2 diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 984df9fe4e9a..4e2bf3b4c88b 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2957,6 +2957,37 @@ struct GTY(()) dw_line_info_table { vec<dw_line_info_entry, va_gc> *entries; }; +/* If we're keep track of location views and their reset points, and + INSN is a reset point (i.e., it necessarily advances the PC), mark + the next view in TABLE as reset. */ + +static void +maybe_reset_location_view (rtx_insn *insn, dw_line_info_table *table) +{ + if (!debug_internal_reset_location_views) + return; + + /* Maybe turn (part of?) this test into a default target hook. */ + int reset = 0; + + if (targetm.reset_location_view) + reset = targetm.reset_location_view (insn); + + if (reset) + ; + else if (JUMP_TABLE_DATA_P (insn)) + reset = 1; + else if (GET_CODE (insn) == USE + || GET_CODE (insn) == CLOBBER + || GET_CODE (insn) == ASM_INPUT + || asm_noperands (insn) >= 0) + ; + else if (get_attr_min_length (insn) > 0) + reset = 1; + + if (reset > 0) + RESET_NEXT_VIEW (table->view); +} /* Each DIE attribute has a field specifying the attribute kind, a link to the next attribute in the chain, and an attribute value. @@ -3164,6 +3195,31 @@ skeleton_chain_node; #endif #endif +/* Return true if GCC configure detected assembler support for .loc. */ + +bool +dwarf2out_default_as_loc_support (void) +{ + return DWARF2_ASM_LINE_DEBUG_INFO; +#if (GCC_VERSION >= 3000) +# undef DWARF2_ASM_LINE_DEBUG_INFO +# pragma GCC poison DWARF2_ASM_LINE_DEBUG_INFO +#endif +} + +/* Return true if GCC configure detected assembler support for views + in .loc directives. */ + +bool +dwarf2out_default_as_locview_support (void) +{ + return DWARF2_ASM_VIEW_DEBUG_INFO; +#if (GCC_VERSION >= 3000) +# undef DWARF2_ASM_VIEW_DEBUG_INFO +# pragma GCC poison DWARF2_ASM_VIEW_DEBUG_INFO +#endif +} + /* A bit is set in ZERO_VIEW_P if we are using the assembler-supported view computation, and it refers to a view identifier for which we will not emit a label because it is known to map to a view number @@ -3215,9 +3271,9 @@ static GTY(()) bitmap zero_view_p; static bool output_asm_line_debug_info (void) { - return (DWARF2_ASM_VIEW_DEBUG_INFO - || (DWARF2_ASM_LINE_DEBUG_INFO - && !debug_variable_location_views)); + return (dwarf2out_as_loc_support + && (dwarf2out_as_locview_support + || !debug_variable_location_views)); } /* Minimum line offset in a special line info. opcode. @@ -9950,28 +10006,31 @@ dwarf2out_maybe_output_loclist_view_pair (dw_loc_list_ref curr) #ifdef DW_LLE_view_pair dw2_asm_output_data (1, DW_LLE_view_pair, "DW_LLE_view_pair"); -# if DWARF2_ASM_VIEW_DEBUG_INFO - if (ZERO_VIEW_P (curr->vbegin)) - dw2_asm_output_data_uleb128 (0, "Location view begin"); - else + if (dwarf2out_as_locview_support) { - char label[MAX_ARTIFICIAL_LABEL_BYTES]; - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); - dw2_asm_output_symname_uleb128 (label, "Location view begin"); - } + if (ZERO_VIEW_P (curr->vbegin)) + dw2_asm_output_data_uleb128 (0, "Location view begin"); + else + { + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); + dw2_asm_output_symname_uleb128 (label, "Location view begin"); + } - if (ZERO_VIEW_P (curr->vend)) - dw2_asm_output_data_uleb128 (0, "Location view end"); + if (ZERO_VIEW_P (curr->vend)) + dw2_asm_output_data_uleb128 (0, "Location view end"); + else + { + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); + dw2_asm_output_symname_uleb128 (label, "Location view end"); + } + } else { - char label[MAX_ARTIFICIAL_LABEL_BYTES]; - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); - dw2_asm_output_symname_uleb128 (label, "Location view end"); + dw2_asm_output_data_uleb128 (curr->vbegin, "Location view begin"); + dw2_asm_output_data_uleb128 (curr->vend, "Location view end"); } -# else /* !DWARF2_ASM_VIEW_DEBUG_INFO */ - dw2_asm_output_data_uleb128 (curr->vbegin, "Location view begin"); - dw2_asm_output_data_uleb128 (curr->vend, "Location view end"); -# endif /* DWARF2_ASM_VIEW_DEBUG_INFO */ #endif /* DW_LLE_view_pair */ return; @@ -10001,40 +10060,43 @@ output_loc_list (dw_loc_list_ref list_head) vcount++; /* ?? dwarf_split_debug_info? */ -#if DWARF2_ASM_VIEW_DEBUG_INFO - char label[MAX_ARTIFICIAL_LABEL_BYTES]; - - if (!ZERO_VIEW_P (curr->vbegin)) + if (dwarf2out_as_locview_support) { - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); - dw2_asm_output_symname_uleb128 (label, - "View list begin (%s)", - list_head->vl_symbol); + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + + if (!ZERO_VIEW_P (curr->vbegin)) + { + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vbegin); + dw2_asm_output_symname_uleb128 (label, + "View list begin (%s)", + list_head->vl_symbol); + } + else + dw2_asm_output_data_uleb128 (0, + "View list begin (%s)", + list_head->vl_symbol); + + if (!ZERO_VIEW_P (curr->vend)) + { + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); + dw2_asm_output_symname_uleb128 (label, + "View list end (%s)", + list_head->vl_symbol); + } + else + dw2_asm_output_data_uleb128 (0, + "View list end (%s)", + list_head->vl_symbol); } else - dw2_asm_output_data_uleb128 (0, - "View list begin (%s)", - list_head->vl_symbol); - - if (!ZERO_VIEW_P (curr->vend)) { - ASM_GENERATE_INTERNAL_LABEL (label, "LVU", curr->vend); - dw2_asm_output_symname_uleb128 (label, - "View list end (%s)", - list_head->vl_symbol); + dw2_asm_output_data_uleb128 (curr->vbegin, + "View list begin (%s)", + list_head->vl_symbol); + dw2_asm_output_data_uleb128 (curr->vend, + "View list end (%s)", + list_head->vl_symbol); } - else - dw2_asm_output_data_uleb128 (0, - "View list end (%s)", - list_head->vl_symbol); -#else /* !DWARF2_ASM_VIEW_DEBUG_INFO */ - dw2_asm_output_data_uleb128 (curr->vbegin, - "View list begin (%s)", - list_head->vl_symbol); - dw2_asm_output_data_uleb128 (curr->vend, - "View list end (%s)", - list_head->vl_symbol); -#endif } } @@ -23721,11 +23783,14 @@ add_high_low_attributes (tree stmt, dw_die_ref die) { inline_entry_data *ied = *iedp; gcc_assert (MAY_HAVE_DEBUG_MARKER_INSNS); + gcc_assert (debug_inline_points); gcc_assert (inlined_function_outer_scope_p (stmt)); + ASM_GENERATE_INTERNAL_LABEL (label, ied->label_pfx, ied->label_num); add_AT_lbl_id (die, DW_AT_entry_pc, label); - if (debug_variable_location_views && !ZERO_VIEW_P (ied->view)) + if (debug_variable_location_views && !ZERO_VIEW_P (ied->view) + && !dwarf_strict) { if (!output_asm_line_debug_info ()) add_AT_unsigned (die, DW_AT_GNU_entry_view, ied->view); @@ -23756,7 +23821,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die) dw_die_ref pdie; dw_attr_node *attr = NULL; - if (!MAY_HAVE_DEBUG_MARKER_INSNS && inlined_function_outer_scope_p (stmt)) + if (!debug_inline_points && inlined_function_outer_scope_p (stmt)) { ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_BEGIN_LABEL, BLOCK_NUMBER (stmt)); @@ -23921,7 +23986,7 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die) dw_die_ref subr_die = new_die (DW_TAG_inlined_subroutine, context_die, stmt); - if (call_arg_locations || MAY_HAVE_DEBUG_MARKER_INSNS) + if (call_arg_locations || debug_inline_points) BLOCK_DIE (stmt) = subr_die; add_abstract_origin_attribute (subr_die, decl); if (TREE_ASM_WRITTEN (stmt)) @@ -26906,7 +26971,7 @@ dwarf2out_var_location (rtx_insn *loc_note) { if (CALL_P (loc_note)) { - RESET_NEXT_VIEW (cur_line_info_table->view); + maybe_reset_location_view (loc_note, cur_line_info_table); call_site_count++; if (SIBLING_CALL_P (loc_note)) tail_call_site_count++; @@ -26942,15 +27007,8 @@ dwarf2out_var_location (rtx_insn *loc_note) } else if (!debug_variable_location_views) gcc_unreachable (); - else if (JUMP_TABLE_DATA_P (loc_note)) - RESET_NEXT_VIEW (cur_line_info_table->view); - else if (GET_CODE (loc_note) == USE - || GET_CODE (loc_note) == CLOBBER - || GET_CODE (loc_note) == ASM_INPUT - || asm_noperands (loc_note) >= 0) - ; - else if (get_attr_min_length (loc_note) > 0) - RESET_NEXT_VIEW (cur_line_info_table->view); + else + maybe_reset_location_view (loc_note, cur_line_info_table); return; } @@ -27219,6 +27277,8 @@ block_within_block_p (tree block, tree outer, bool bothways) static void dwarf2out_inline_entry (tree block) { + gcc_assert (debug_inline_points); + /* If we can't represent it, don't bother. */ if (!(dwarf_version >= 3 || !dwarf_strict)) return; @@ -28233,7 +28293,7 @@ init_sections_and_labels (bool early_lto_debug) debug_str_section = get_section (DEBUG_LTO_STR_SECTION, DEBUG_STR_SECTION_FLAGS | SECTION_EXCLUDE, NULL); - if (!dwarf_split_debug_info && !DWARF2_ASM_LINE_DEBUG_INFO) + if (!dwarf_split_debug_info && !dwarf2out_as_loc_support) debug_line_str_section = get_section (DEBUG_LTO_LINE_STR_SECTION, DEBUG_STR_SECTION_FLAGS | SECTION_EXCLUDE, NULL); @@ -31468,7 +31528,7 @@ dwarf2out_early_finish (const char *filename) /* When emitting DWARF5 .debug_line_str, move DW_AT_name and DW_AT_comp_dir into .debug_line_str section. */ - if (!DWARF2_ASM_LINE_DEBUG_INFO + if (!dwarf2out_as_loc_support && dwarf_version >= 5 && DWARF5_USE_DEBUG_LINE_STR) { diff --git a/gcc/hooks.c b/gcc/hooks.c index 61719606a36d..780cc1e08631 100644 --- a/gcc/hooks.c +++ b/gcc/hooks.c @@ -235,6 +235,12 @@ hook_int_rtx_1 (rtx) return 1; } +int +hook_int_rtx_insn_0 (rtx_insn *) +{ + return 0; +} + int hook_int_rtx_insn_unreachable (rtx_insn *) { diff --git a/gcc/hooks.h b/gcc/hooks.h index 8caedd429a69..0ed5b952b48e 100644 --- a/gcc/hooks.h +++ b/gcc/hooks.h @@ -93,6 +93,7 @@ extern int hook_int_const_tree_0 (const_tree); extern int hook_int_const_tree_const_tree_1 (const_tree, const_tree); extern int hook_int_rtx_0 (rtx); extern int hook_int_rtx_1 (rtx); +extern int hook_int_rtx_insn_0 (rtx_insn *); extern int hook_int_rtx_insn_unreachable (rtx_insn *); extern int hook_int_rtx_bool_0 (rtx, bool); extern int hook_int_rtx_mode_as_bool_0 (rtx, machine_mode, addr_space_t, diff --git a/gcc/target.def b/gcc/target.def index aeb41df19454..c5b2a1e7e71f 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -6447,6 +6447,23 @@ This will suppress generation of the normal debug frame unwind information.", enum unwind_info_type, (void), default_debug_unwind_info) +DEFHOOK +(reset_location_view, "\ +This hook, if defined, enables -ginternal-reset-location-views, and\n\ +uses its result to override cases in which the estimated min insn\n\ +length might be nonzero even when a PC advance (i.e., a view reset)\n\ +cannot be taken for granted.\n\ +\n\ +If the hook is defined, it must return a positive value to indicate\n\ +the insn definitely advances the PC, and so the view number can be\n\ +safely assumed to be reset; a negative value to mean the insn\n\ +definitely does not advance the PC, and os the view number must not\n\ +be reset; or zero to decide based on the estimated insn length.\n\ +\n\ +If insn length is to be regarded as reliable, set the hook to\n\ +@code{hook_int_rtx_insn_0}.", + int, (rtx_insn *), NULL) + /* The code parameter should be of type enum rtx_code but this is not defined at this time. */ DEFHOOK diff --git a/gcc/toplev.c b/gcc/toplev.c index 23db0636fc79..b066bcc72297 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1558,13 +1558,23 @@ process_options (void) || write_symbols == VMS_AND_DWARF2_DEBUG) && !(flag_selective_scheduling || flag_selective_scheduling2)); + if (dwarf2out_as_loc_support == AUTODETECT_VALUE) + dwarf2out_as_loc_support + = dwarf2out_default_as_loc_support (); + if (dwarf2out_as_locview_support == AUTODETECT_VALUE) + dwarf2out_as_locview_support + = dwarf2out_default_as_locview_support (); + if (debug_variable_location_views == AUTODETECT_VALUE) { - debug_variable_location_views = flag_var_tracking - && debug_info_level >= DINFO_LEVEL_NORMAL - && (write_symbols == DWARF2_DEBUG - || write_symbols == VMS_AND_DWARF2_DEBUG) - && !dwarf_strict; + debug_variable_location_views + = (flag_var_tracking + && debug_info_level >= DINFO_LEVEL_NORMAL + && (write_symbols == DWARF2_DEBUG + || write_symbols == VMS_AND_DWARF2_DEBUG) + && !dwarf_strict + && dwarf2out_as_loc_support + && dwarf2out_as_locview_support); } else if (debug_variable_location_views == -1 && dwarf_version != 5) { @@ -1574,6 +1584,31 @@ process_options (void) debug_variable_location_views = 1; } + if (debug_internal_reset_location_views == 2) + { + debug_internal_reset_location_views + = (debug_variable_location_views + && targetm.reset_location_view); + } + else if (debug_internal_reset_location_views + && !debug_variable_location_views) + { + warning_at (UNKNOWN_LOCATION, 0, + "-ginternal-reset-location-views is forced disabled " + "without -gvariable-location-views"); + debug_internal_reset_location_views = 0; + } + + if (debug_inline_points == AUTODETECT_VALUE) + debug_inline_points = debug_variable_location_views; + else if (debug_inline_points && !debug_nonbind_markers_p) + { + warning_at (UNKNOWN_LOCATION, 0, + "-ginline-points is forced disabled without " + "-gstatement-frontiers"); + debug_inline_points = 0; + } + if (flag_tree_cselim == AUTODETECT_VALUE) { if (HAVE_conditional_move) diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 7f9ec770e197..811829e85299 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -4605,7 +4605,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id) GSI_NEW_STMT); } initialize_inlined_parameters (id, stmt, fn, bb); - if (debug_nonbind_markers_p && id->block + if (debug_nonbind_markers_p && debug_inline_points && id->block && inlined_function_outer_scope_p (id->block)) { gimple_stmt_iterator si = gsi_last_bb (bb); diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c index 26da31f74cb2..62bb3c5de659 100644 --- a/gcc/tree-ssa-live.c +++ b/gcc/tree-ssa-live.c @@ -522,7 +522,7 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block) unused = false; /* Preserve the block, it is referenced by at least the inline entry point marker. */ - else if (debug_nonbind_markers_p + else if (debug_inline_points && inlined_function_outer_scope_p (scope)) unused = false; /* Innermost blocks with no live variables nor statements can be always @@ -558,7 +558,7 @@ remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_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 + else if (!debug_inline_points && inlined_function_outer_scope_p (scope)) unused = false; else
On 02/13/2018 06:43 AM, Alexandre Oliva wrote: > On Feb 12, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > >> This patch supersedes the previous one. Testing underway... Ok if it >> succeeds? > > I failed to update the patch I posted after making a correct to symbol > poisoning, that had caused builds to fail right away, sorry. Thanks, > Rainer, for catching the error. > > Here's the patch that actually passed regstrap on native i686 and > x86_64-linux-gnu, and fixed numerous regressions on cross builds. > Ok to install? > > > [LVU, IEPM] several new controlling options > > Given that the minimum insn length is not generally reliable to tell > whether an insn actually advances PC, this patch disables the locview > list optimizations that can only be done when can tell it. > > The preexisting logic is retained, however, and can be enabled with > the newly-introduced -ginternal-reset-location-view. This is now > enabled by default only if the target defines a hook that may override > or defer to the preexisting logic. The negated command line option > can then be used should errors still be encountered. > > > We also introduce options to control whether to assume .loc and view > support in the assembler, and to control whether to output inline > entry points (and views) from markers. > > > This patch also fixes a number of documentation formatting errors, > namely using @item rather than @itemx for all but the first of several > options before a description. > > for gcc/ChangeLog > > * common.opt (gas-loc-support, gas-locview-support): New. > (ginline-points, ginternal-reset-location-views): New. > * doc/invoke.texi: Document them. Use @itemx where intended. > (gvariable-location-views): Adjust. > * target.def (reset_location_view): New. > * doc/tm.texi.in (DWARF2_ASM_VIEW_DEBUG_INFO): New. > (TARGET_RESET_LOCATION_VIEW): New. > * doc/tm.texi: Rebuilt. > * dwarf2out.c (dwarf2out_default_as_loc_support): New. > (dwarf2out_default_as_locview_support): New. > (output_asm_line_debug_info): Use option variables. > (dwarf2out_maybe_output_loclist_view_pair): Likewise. > (output_loc_list): Likewise. > (add_high_low_attributes): Check option variables. > Don't output entry view attribute in strict mode. > (gen_inlined_subroutine_die): Check option variables. > (dwarf2out_inline_entry): Likewise. > (init_sections_and_labels): Likewise. > (dwarf2out_early_finish): Likewise. > (maybe_reset_location_view): New, from... > (dwarf2out_var_location): ... here. Call it. > * debug.h (dwarf2out_default_as_loc_support): Declare. > (dwarf2out_default_as_locview_support): Declare. > * hooks.c (hook_int_rtx_insn_0): New. > * hooks.h (hook_int_rtx_insn_0): Declare. > * toplev.c (process_options): Take -gas-loc-support and > -gas-locview-support from dwarf2out. Enable > -gvariable-location-views by default only with locview > assembler support. Enable -ginternal-reset-location-views by > default only if the target defines the corresponding hook. > Enable -ginline-points by default if location views are > enabled; force it disabled if statement frontiers are > disabled. > * tree-inline.c (expand_call_inline): Check option variables. > * tree-ssa-live.c (remove_unused_scope_block_p): Likewise. OK. I probably would have gone with the initial version or something closer to that -- this seems to have expanded in scope a bit. But it's not enough to object to since the scope creep is really just giving options to turn on/off the various bits for testing purposes. I plan to commit it momentarily given the number of targets that are failing in my tester due to this issue. jeff
On 13/02/18 13:43, Alexandre Oliva wrote: > On Feb 12, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > >> This patch supersedes the previous one. Testing underway... Ok if it >> succeeds? > > I failed to update the patch I posted after making a correct to symbol > poisoning, that had caused builds to fail right away, sorry. Thanks, > Rainer, for catching the error. > > Here's the patch that actually passed regstrap on native i686 and > x86_64-linux-gnu, and fixed numerous regressions on cross builds. > Ok to install? > > > [LVU, IEPM] several new controlling options > > Given that the minimum insn length is not generally reliable to tell > whether an insn actually advances PC, this patch disables the locview > list optimizations that can only be done when can tell it. > > The preexisting logic is retained, however, and can be enabled with > the newly-introduced -ginternal-reset-location-view. This is now > enabled by default only if the target defines a hook that may override > or defer to the preexisting logic. The negated command line option > can then be used should errors still be encountered. > > > We also introduce options to control whether to assume .loc and view > support in the assembler, and to control whether to output inline > entry points (and views) from markers. > i see assembler slow downs with these location view patches i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408
On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > i see assembler slow downs with these location view patches > i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 [LVU] reset view at function entry, omit views at line zero Location views might be associated with locations that lack line number information (line number zero), but since we omit .loc directives that would have been issued with line number zero, we also omit the symbolic view numbers that would have been issued at such points. Resetting views at function entry points address some of these issues, and alleviate the huge chains of symbolic views that have burdened assemblers since we disabled -ginternal-reset-location-views by default, but other problems of undefined views remain when it's not the whole function that lacks line number info, just parts of it. So, when we encounter a request to output a view that may have been referenced, but we decide to omit the .loc because the line is zero, we will now omit the view as well, i.e., we will internally regard that view as zero-numbered. Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? Uros, could you please confirm whether this fixes the 84404 go problem you reported on alpha? I'm guessing it's the same issue. TIA, for gcc/ChangeLog PR debug/84404 PR debug/84408 * dwarf2out.c (struct dw_line_info_table): Update comments for view == -1. (FORCE_RESET_NEXT_VIEW): New. (FORCE_RESETTING_VIEW_P): New. (RESETTING_VIEW_P): Check for -1 too. (ZERO_VIEW_P): Likewise. (new_line_info_table): Force-reset next view. (dwarf2out_begin_function): Likewise. (dwarf2out_source_line): Simplify zero_view_p initialization. Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of view directly. Omit view when omitting .loc at line 0. for gcc/testsuite/ChangeLog PR debug/84404 PR debug/84408 * gcc.dg/graphite/pr84404.c: New. --- gcc/dwarf2out.c | 89 ++++++++++++++++++++++++------- gcc/testsuite/gcc.dg/graphite/pr84404.c | 18 ++++++ 2 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/graphite/pr84404.c diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 5e88c7bacf06..7bbe20e495ac 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2940,8 +2940,8 @@ struct GTY(()) dw_line_info_table { If it is 0, it is known that the NEXT view will be the first view at the given PC. - If it is -1, we've advanced PC but we haven't emitted a line location yet, - so we shouldn't use this view number. + If it is -1, we're forcing the view number to be reset, e.g. at a + function entry. The meaning of other nonzero values depends on whether we're computing views internally or leaving it for the assembler to do @@ -2951,8 +2951,10 @@ struct GTY(()) dw_line_info_table { going to ask the assembler to assign. */ var_loc_view view; +#define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1) #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0) -#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0) +#define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1) +#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0 || FORCE_RESETTING_VIEW_P (x)) vec<dw_line_info_entry, va_gc> *entries; }; @@ -2985,7 +2987,7 @@ maybe_reset_location_view (rtx_insn *insn, dw_line_info_table *table) else if (get_attr_min_length (insn) > 0) reset = 1; - if (reset > 0) + if (reset > 0 && !RESETTING_VIEW_P (table->view)) RESET_NEXT_VIEW (table->view); } @@ -3235,9 +3237,10 @@ static GTY(()) bitmap zero_view_p; that must be view number zero. Otherwise, ZERO_VIEW_P is allocated and views label numbers recorded in it are the ones known to be zero. */ -#define ZERO_VIEW_P(N) (zero_view_p \ - ? bitmap_bit_p (zero_view_p, (N)) \ - : (N) == 0) +#define ZERO_VIEW_P(N) ((N) == (var_loc_view)0 \ + || (N) == (var_loc_view)-1 \ + || (zero_view_p \ + && bitmap_bit_p (zero_view_p, (N)))) /* Return true iff we're to emit .loc directives for the assembler to generate line number sections. @@ -27210,6 +27213,18 @@ create_label: last_postcall_label = ggc_strdup (loclabel); } newloc->label = last_postcall_label; + /* ??? This view is at last_label, not last_label-1, but we + could only assume view at last_label-1 is zero if we could + assume calls always have length greater than one. This is + probably true in general, though there might be a rare + exception to this rule, e.g. if a call insn is optimized out + by target magic. Then, even the -1 in the label will be + wrong, which might invalidate the range. Anyway, using view, + though technically possibly incorrect, will work as far as + ranges go: since L-1 is in the middle of the call insn, + (L-1).0 and (L-1).V shouldn't make any difference, and having + the loclist entry refer to the .loc entry might be useful, so + leave it like this. */ newloc->view = view; } @@ -27391,7 +27406,7 @@ new_line_info_table (void) table->file_num = 1; table->line_num = 1; table->is_stmt = DWARF_LINE_DEFAULT_IS_STMT_START; - RESET_NEXT_VIEW (table->view); + FORCE_RESET_NEXT_VIEW (table->view); return table; } @@ -27475,6 +27490,7 @@ dwarf2out_begin_function (tree fun) tail_call_site_count = 0; set_cur_line_info_table (sec); + FORCE_RESET_NEXT_VIEW (cur_line_info_table->view); } /* Helper function of dwarf2out_end_function, called only after emitting @@ -27572,10 +27588,44 @@ dwarf2out_source_line (unsigned int line, unsigned int column, { unsigned int file_num; dw_line_info_table *table; + static var_loc_view lvugid; - if (debug_info_level < DINFO_LEVEL_TERSE || line == 0) + if (debug_info_level < DINFO_LEVEL_TERSE) return; + table = cur_line_info_table; + + if (line == 0) + { + if (debug_variable_location_views + && output_asm_line_debug_info () + && table && !RESETTING_VIEW_P (table->view)) + { + /* If we're using the assembler to compute view numbers, we + can't issue a .loc directive for line zero, so we can't + get a view number at this point. We might attempt to + compute it from the previous view, but since we're + omitting the line number entry, we might as well omit the + view number as well. That means pretending it's a view + number zero, which might very well turn out to be + correct. */ + if (!zero_view_p) + zero_view_p = BITMAP_GGC_ALLOC (); + bitmap_set_bit (zero_view_p, table->view); + if (flag_debug_asm) + { + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view); + fprintf (asm_out_file, "\t%s line 0, omitted view ", + ASM_COMMENT_START); + assemble_name (asm_out_file, label); + putc ('\n', asm_out_file); + } + table->view = ++lvugid; + } + return; + } + /* The discriminator column was added in dwarf4. Simplify the below by simply removing it if we're not supposed to output it. */ if (dwarf_version < 4 && dwarf_strict) @@ -27584,7 +27634,6 @@ dwarf2out_source_line (unsigned int line, unsigned int column, if (!debug_column_info) column = 0; - table = cur_line_info_table; file_num = maybe_emit_file (lookup_filename (filename)); /* ??? TODO: Elide duplicate line number entries. Traditionally, @@ -27646,13 +27695,6 @@ dwarf2out_source_line (unsigned int line, unsigned int column, } if (debug_variable_location_views) { - static var_loc_view lvugid; - if (!lvugid) - { - gcc_assert (!zero_view_p); - zero_view_p = BITMAP_GGC_ALLOC (); - bitmap_set_bit (zero_view_p, 0); - } if (!RESETTING_VIEW_P (table->view)) { /* When we're using the assembler to compute view @@ -27673,7 +27715,7 @@ dwarf2out_source_line (unsigned int line, unsigned int column, } else { - if (!table->in_use) + if (FORCE_RESETTING_VIEW_P (table->view)) fputs (" view -0", asm_out_file); else fputs (" view 0", asm_out_file); @@ -27684,6 +27726,8 @@ dwarf2out_source_line (unsigned int line, unsigned int column, known to be zero, because then we may be able to optimize out locviews that are all zeros, so take note of it in zero_view_p. */ + if (!zero_view_p) + zero_view_p = BITMAP_GGC_ALLOC (); bitmap_set_bit (zero_view_p, lvugid); table->view = ++lvugid; } @@ -27696,17 +27740,22 @@ dwarf2out_source_line (unsigned int line, unsigned int column, targetm.asm_out.internal_label (asm_out_file, LINE_CODE_LABEL, label_num); - if (debug_variable_location_views && table->view) + if (debug_variable_location_views && !RESETTING_VIEW_P (table->view)) push_dw_line_info_entry (table, LI_adv_address, label_num); else push_dw_line_info_entry (table, LI_set_address, label_num); if (debug_variable_location_views) { + bool resetting = FORCE_RESETTING_VIEW_P (table->view); + if (resetting) + table->view = 0; + if (flag_debug_asm) fprintf (asm_out_file, "\t%s view %s%d\n", ASM_COMMENT_START, - table->in_use ? "" : "-", + resetting ? "-" : "", table->view); + table->view++; } if (file_num != table->file_num) diff --git a/gcc/testsuite/gcc.dg/graphite/pr84404.c b/gcc/testsuite/gcc.dg/graphite/pr84404.c new file mode 100644 index 000000000000..858e651cfab7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/graphite/pr84404.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -ftree-parallelize-loops=2 -floop-nest-optimize -g" } */ + +int te[9]; + +void +dt (int cz) +{ + while (cz < 1) + { + int xy; + + for (xy = 0; xy < 9; ++xy) + te[xy] = 0; + + ++cz; + } +}
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? 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))
On Wed, Feb 21, 2018 at 11:11 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > >> i see assembler slow downs with these location view patches >> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 > > > [LVU] reset view at function entry, omit views at line zero > > Location views might be associated with locations that lack line > number information (line number zero), but since we omit .loc > directives that would have been issued with line number zero, we also > omit the symbolic view numbers that would have been issued at such > points. > > Resetting views at function entry points address some of these issues, > and alleviate the huge chains of symbolic views that have burdened > assemblers since we disabled -ginternal-reset-location-views by > default, but other problems of undefined views remain when it's not > the whole function that lacks line number info, just parts of it. > > So, when we encounter a request to output a view that may have been > referenced, but we decide to omit the .loc because the line is zero, > we will now omit the view as well, i.e., we will internally regard > that view as zero-numbered. > > Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? > > Uros, could you please confirm whether this fixes the 84404 go problem > you reported on alpha? I'm guessing it's the same issue. TIA, I can confirm, that all "leb128 operand is an undefined symbol" errors are gone. I'm running full alpha native bootstrap & regression test, but I don't expect any surprises here. Thanks, Uros. > for gcc/ChangeLog > > PR debug/84404 > PR debug/84408 > * dwarf2out.c (struct dw_line_info_table): Update comments for > view == -1. > (FORCE_RESET_NEXT_VIEW): New. > (FORCE_RESETTING_VIEW_P): New. > (RESETTING_VIEW_P): Check for -1 too. > (ZERO_VIEW_P): Likewise. > (new_line_info_table): Force-reset next view. > (dwarf2out_begin_function): Likewise. > (dwarf2out_source_line): Simplify zero_view_p initialization. > Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of > view directly. Omit view when omitting .loc at line 0. > > for gcc/testsuite/ChangeLog > > PR debug/84404 > PR debug/84408 > * gcc.dg/graphite/pr84404.c: New. > --- > gcc/dwarf2out.c | 89 ++++++++++++++++++++++++------- > gcc/testsuite/gcc.dg/graphite/pr84404.c | 18 ++++++ > 2 files changed, 87 insertions(+), 20 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/graphite/pr84404.c > > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index 5e88c7bacf06..7bbe20e495ac 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -2940,8 +2940,8 @@ struct GTY(()) dw_line_info_table { > If it is 0, it is known that the NEXT view will be the first view > at the given PC. > > - If it is -1, we've advanced PC but we haven't emitted a line location yet, > - so we shouldn't use this view number. > + If it is -1, we're forcing the view number to be reset, e.g. at a > + function entry. > > The meaning of other nonzero values depends on whether we're > computing views internally or leaving it for the assembler to do > @@ -2951,8 +2951,10 @@ struct GTY(()) dw_line_info_table { > going to ask the assembler to assign. */ > var_loc_view view; > > +#define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1) > #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0) > -#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0) > +#define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1) > +#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0 || FORCE_RESETTING_VIEW_P (x)) > > vec<dw_line_info_entry, va_gc> *entries; > }; > @@ -2985,7 +2987,7 @@ maybe_reset_location_view (rtx_insn *insn, dw_line_info_table *table) > else if (get_attr_min_length (insn) > 0) > reset = 1; > > - if (reset > 0) > + if (reset > 0 && !RESETTING_VIEW_P (table->view)) > RESET_NEXT_VIEW (table->view); > } > > @@ -3235,9 +3237,10 @@ static GTY(()) bitmap zero_view_p; > that must be view number zero. Otherwise, ZERO_VIEW_P is allocated > and views label numbers recorded in it are the ones known to be > zero. */ > -#define ZERO_VIEW_P(N) (zero_view_p \ > - ? bitmap_bit_p (zero_view_p, (N)) \ > - : (N) == 0) > +#define ZERO_VIEW_P(N) ((N) == (var_loc_view)0 \ > + || (N) == (var_loc_view)-1 \ > + || (zero_view_p \ > + && bitmap_bit_p (zero_view_p, (N)))) > > /* Return true iff we're to emit .loc directives for the assembler to > generate line number sections. > @@ -27210,6 +27213,18 @@ create_label: > last_postcall_label = ggc_strdup (loclabel); > } > newloc->label = last_postcall_label; > + /* ??? This view is at last_label, not last_label-1, but we > + could only assume view at last_label-1 is zero if we could > + assume calls always have length greater than one. This is > + probably true in general, though there might be a rare > + exception to this rule, e.g. if a call insn is optimized out > + by target magic. Then, even the -1 in the label will be > + wrong, which might invalidate the range. Anyway, using view, > + though technically possibly incorrect, will work as far as > + ranges go: since L-1 is in the middle of the call insn, > + (L-1).0 and (L-1).V shouldn't make any difference, and having > + the loclist entry refer to the .loc entry might be useful, so > + leave it like this. */ > newloc->view = view; > } > > @@ -27391,7 +27406,7 @@ new_line_info_table (void) > table->file_num = 1; > table->line_num = 1; > table->is_stmt = DWARF_LINE_DEFAULT_IS_STMT_START; > - RESET_NEXT_VIEW (table->view); > + FORCE_RESET_NEXT_VIEW (table->view); > > return table; > } > @@ -27475,6 +27490,7 @@ dwarf2out_begin_function (tree fun) > tail_call_site_count = 0; > > set_cur_line_info_table (sec); > + FORCE_RESET_NEXT_VIEW (cur_line_info_table->view); > } > > /* Helper function of dwarf2out_end_function, called only after emitting > @@ -27572,10 +27588,44 @@ dwarf2out_source_line (unsigned int line, unsigned int column, > { > unsigned int file_num; > dw_line_info_table *table; > + static var_loc_view lvugid; > > - if (debug_info_level < DINFO_LEVEL_TERSE || line == 0) > + if (debug_info_level < DINFO_LEVEL_TERSE) > return; > > + table = cur_line_info_table; > + > + if (line == 0) > + { > + if (debug_variable_location_views > + && output_asm_line_debug_info () > + && table && !RESETTING_VIEW_P (table->view)) > + { > + /* If we're using the assembler to compute view numbers, we > + can't issue a .loc directive for line zero, so we can't > + get a view number at this point. We might attempt to > + compute it from the previous view, but since we're > + omitting the line number entry, we might as well omit the > + view number as well. That means pretending it's a view > + number zero, which might very well turn out to be > + correct. */ > + if (!zero_view_p) > + zero_view_p = BITMAP_GGC_ALLOC (); > + bitmap_set_bit (zero_view_p, table->view); > + if (flag_debug_asm) > + { > + char label[MAX_ARTIFICIAL_LABEL_BYTES]; > + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view); > + fprintf (asm_out_file, "\t%s line 0, omitted view ", > + ASM_COMMENT_START); > + assemble_name (asm_out_file, label); > + putc ('\n', asm_out_file); > + } > + table->view = ++lvugid; > + } > + return; > + } > + > /* The discriminator column was added in dwarf4. Simplify the below > by simply removing it if we're not supposed to output it. */ > if (dwarf_version < 4 && dwarf_strict) > @@ -27584,7 +27634,6 @@ dwarf2out_source_line (unsigned int line, unsigned int column, > if (!debug_column_info) > column = 0; > > - table = cur_line_info_table; > file_num = maybe_emit_file (lookup_filename (filename)); > > /* ??? TODO: Elide duplicate line number entries. Traditionally, > @@ -27646,13 +27695,6 @@ dwarf2out_source_line (unsigned int line, unsigned int column, > } > if (debug_variable_location_views) > { > - static var_loc_view lvugid; > - if (!lvugid) > - { > - gcc_assert (!zero_view_p); > - zero_view_p = BITMAP_GGC_ALLOC (); > - bitmap_set_bit (zero_view_p, 0); > - } > if (!RESETTING_VIEW_P (table->view)) > { > /* When we're using the assembler to compute view > @@ -27673,7 +27715,7 @@ dwarf2out_source_line (unsigned int line, unsigned int column, > } > else > { > - if (!table->in_use) > + if (FORCE_RESETTING_VIEW_P (table->view)) > fputs (" view -0", asm_out_file); > else > fputs (" view 0", asm_out_file); > @@ -27684,6 +27726,8 @@ dwarf2out_source_line (unsigned int line, unsigned int column, > known to be zero, because then we may be able to > optimize out locviews that are all zeros, so take > note of it in zero_view_p. */ > + if (!zero_view_p) > + zero_view_p = BITMAP_GGC_ALLOC (); > bitmap_set_bit (zero_view_p, lvugid); > table->view = ++lvugid; > } > @@ -27696,17 +27740,22 @@ dwarf2out_source_line (unsigned int line, unsigned int column, > > targetm.asm_out.internal_label (asm_out_file, LINE_CODE_LABEL, label_num); > > - if (debug_variable_location_views && table->view) > + if (debug_variable_location_views && !RESETTING_VIEW_P (table->view)) > push_dw_line_info_entry (table, LI_adv_address, label_num); > else > push_dw_line_info_entry (table, LI_set_address, label_num); > if (debug_variable_location_views) > { > + bool resetting = FORCE_RESETTING_VIEW_P (table->view); > + if (resetting) > + table->view = 0; > + > if (flag_debug_asm) > fprintf (asm_out_file, "\t%s view %s%d\n", > ASM_COMMENT_START, > - table->in_use ? "" : "-", > + resetting ? "-" : "", > table->view); > + > table->view++; > } > if (file_num != table->file_num) > diff --git a/gcc/testsuite/gcc.dg/graphite/pr84404.c b/gcc/testsuite/gcc.dg/graphite/pr84404.c > new file mode 100644 > index 000000000000..858e651cfab7 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/graphite/pr84404.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -ftree-parallelize-loops=2 -floop-nest-optimize -g" } */ > + > +int te[9]; > + > +void > +dt (int cz) > +{ > + while (cz < 1) > + { > + int xy; > + > + for (xy = 0; xy < 9; ++xy) > + te[xy] = 0; > + > + ++cz; > + } > +} > > > -- > 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
On 21/02/18 10:11, Alexandre Oliva wrote: > On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > >> i see assembler slow downs with these location view patches >> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 > > > [LVU] reset view at function entry, omit views at line zero > > Location views might be associated with locations that lack line > number information (line number zero), but since we omit .loc > directives that would have been issued with line number zero, we also > omit the symbolic view numbers that would have been issued at such > points. > > Resetting views at function entry points address some of these issues, > and alleviate the huge chains of symbolic views that have burdened > assemblers since we disabled -ginternal-reset-location-views by > default, but other problems of undefined views remain when it's not > the whole function that lacks line number info, just parts of it. > > So, when we encounter a request to output a view that may have been > referenced, but we decide to omit the .loc because the line is zero, > we will now omit the view as well, i.e., we will internally regard > that view as zero-numbered. > > Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? > fixes 84408 for me, now i get $ ./xg++ -B. -g -O -fPIC -shared -fno-rtti -time poly-int-07_plugin.ii # cc1plus 47.39 0.60 # as 6.78 0.25 # collect2 0.27 0.04
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
On Feb 21, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: >> i see assembler slow downs with these location view patches >> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 > [LVU] reset view at function entry, omit views at line zero Ping? https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01224.html > for gcc/ChangeLog > PR debug/84404 > PR debug/84408 > * dwarf2out.c (struct dw_line_info_table): Update comments for > view == -1. > (FORCE_RESET_NEXT_VIEW): New. > (FORCE_RESETTING_VIEW_P): New. > (RESETTING_VIEW_P): Check for -1 too. > (ZERO_VIEW_P): Likewise. > (new_line_info_table): Force-reset next view. > (dwarf2out_begin_function): Likewise. > (dwarf2out_source_line): Simplify zero_view_p initialization. > Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of > view directly. Omit view when omitting .loc at line 0. > for gcc/testsuite/ChangeLog > PR debug/84404 > PR debug/84408 > * gcc.dg/graphite/pr84404.c: New.
On 02/21/2018 03:11 AM, Alexandre Oliva wrote: > On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > >> i see assembler slow downs with these location view patches >> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 > > > [LVU] reset view at function entry, omit views at line zero > > Location views might be associated with locations that lack line > number information (line number zero), but since we omit .loc > directives that would have been issued with line number zero, we also > omit the symbolic view numbers that would have been issued at such > points. > > Resetting views at function entry points address some of these issues, > and alleviate the huge chains of symbolic views that have burdened > assemblers since we disabled -ginternal-reset-location-views by > default, but other problems of undefined views remain when it's not > the whole function that lacks line number info, just parts of it. > > So, when we encounter a request to output a view that may have been > referenced, but we decide to omit the .loc because the line is zero, > we will now omit the view as well, i.e., we will internally regard > that view as zero-numbered. > > Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? > > Uros, could you please confirm whether this fixes the 84404 go problem > you reported on alpha? I'm guessing it's the same issue. TIA, > > for gcc/ChangeLog > > PR debug/84404 > PR debug/84408 > * dwarf2out.c (struct dw_line_info_table): Update comments for > view == -1. > (FORCE_RESET_NEXT_VIEW): New. > (FORCE_RESETTING_VIEW_P): New. > (RESETTING_VIEW_P): Check for -1 too. > (ZERO_VIEW_P): Likewise. > (new_line_info_table): Force-reset next view. > (dwarf2out_begin_function): Likewise. > (dwarf2out_source_line): Simplify zero_view_p initialization. > Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of > view directly. Omit view when omitting .loc at line 0. > > for gcc/testsuite/ChangeLog > > PR debug/84404 > PR debug/84408 > * gcc.dg/graphite/pr84404.c: New. OK. jeff
On Wed, Feb 28, 2018 at 6:17 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Feb 21, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > >> On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: >>> i see assembler slow downs with these location view patches >>> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 > > >> [LVU] reset view at function entry, omit views at line zero > > Ping? https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01224.html Hi, The new test case failed on aarch64-none-elf, aarch64_be-none-elf and arm-none-eabi , which are all bare-metal toolchains with below message: xgcc: error: unrecognized command line option '-pthread' I assume pthread is unavailable on such targets if it's required. Thanks, bin > >> for gcc/ChangeLog > >> PR debug/84404 >> PR debug/84408 >> * dwarf2out.c (struct dw_line_info_table): Update comments for >> view == -1. >> (FORCE_RESET_NEXT_VIEW): New. >> (FORCE_RESETTING_VIEW_P): New. >> (RESETTING_VIEW_P): Check for -1 too. >> (ZERO_VIEW_P): Likewise. >> (new_line_info_table): Force-reset next view. >> (dwarf2out_begin_function): Likewise. >> (dwarf2out_source_line): Simplify zero_view_p initialization. >> Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of >> view directly. Omit view when omitting .loc at line 0. > >> for gcc/testsuite/ChangeLog > >> PR debug/84404 >> PR debug/84408 >> * gcc.dg/graphite/pr84404.c: New. > > -- > 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
On Fri, Mar 9, 2018 at 9:48 AM, Bin.Cheng <amker.cheng@gmail.com> wrote: > On Wed, Feb 28, 2018 at 6:17 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> On Feb 21, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: >> >>> On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: >>>> i see assembler slow downs with these location view patches >>>> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 >> >> >>> [LVU] reset view at function entry, omit views at line zero >> >> Ping? https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01224.html > > Hi, > The new test case failed on aarch64-none-elf, aarch64_be-none-elf and > arm-none-eabi , > which are all bare-metal toolchains with below message: > > xgcc: error: unrecognized command line option '-pthread' > > I assume pthread is unavailable on such targets if it's required. I think there's a dg-effective-target for pthread. Ramana > > Thanks, > bin >> >>> for gcc/ChangeLog >> >>> PR debug/84404 >>> PR debug/84408 >>> * dwarf2out.c (struct dw_line_info_table): Update comments for >>> view == -1. >>> (FORCE_RESET_NEXT_VIEW): New. >>> (FORCE_RESETTING_VIEW_P): New. >>> (RESETTING_VIEW_P): Check for -1 too. >>> (ZERO_VIEW_P): Likewise. >>> (new_line_info_table): Force-reset next view. >>> (dwarf2out_begin_function): Likewise. >>> (dwarf2out_source_line): Simplify zero_view_p initialization. >>> Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of >>> view directly. Omit view when omitting .loc at line 0. >> >>> for gcc/testsuite/ChangeLog >> >>> PR debug/84404 >>> PR debug/84408 >>> * gcc.dg/graphite/pr84404.c: New. >> >> -- >> 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
On Fri, Mar 09, 2018 at 09:48:45AM +0000, Bin.Cheng wrote: > On Wed, Feb 28, 2018 at 6:17 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > > On Feb 21, 2018, Alexandre Oliva <aoliva@redhat.com> wrote: > > > >> On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > >>> i see assembler slow downs with these location view patches > >>> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408 > > > > > >> [LVU] reset view at function entry, omit views at line zero > > > > Ping? https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01224.html > > Hi, > The new test case failed on aarch64-none-elf, aarch64_be-none-elf and > arm-none-eabi , > which are all bare-metal toolchains with below message: > > xgcc: error: unrecognized command line option '-pthread' > > I assume pthread is unavailable on such targets if it's required. I think this ought to fix it, committed as obvious: 2018-03-09 Jakub Jelinek <jakub@redhat.com> PR debug/84404 * gcc.dg/graphite/pr84404.c: Only compile on pthread effective targets. --- gcc/testsuite/gcc.dg/graphite/pr84404.c.jj 2018-03-08 21:53:46.096560261 +0100 +++ gcc/testsuite/gcc.dg/graphite/pr84404.c 2018-03-09 10:54:49.605773196 +0100 @@ -1,4 +1,5 @@ -/* { dg-do compile } */ +/* PR debug/84404 */ +/* { dg-do compile { target pthread } } */ /* { dg-options "-O2 -ftree-parallelize-loops=2 -floop-nest-optimize -g" } */ int te[9]; Jakub
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 3f17e90d50b3..a9531e86a662 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5715,6 +5715,15 @@ expand_gimple_basic_block (basic_block bb, bool disable_tail_calls) goto delink_debug_stmt; 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; + } else gcc_unreachable (); diff --git a/gcc/doc/gimple.texi b/gcc/doc/gimple.texi index 492678705c6f..3825378080fa 100644 --- a/gcc/doc/gimple.texi +++ b/gcc/doc/gimple.texi @@ -836,6 +836,11 @@ Return true if g is a @code{GIMPLE_DEBUG} that marks the beginning of a source statement. @end deftypefn +@deftypefn {GIMPLE function} gimple_debug_inline_entry_p (gimple g) +Return true if g is a @code{GIMPLE_DEBUG} that marks the entry +point of an inlined function. +@end deftypefn + @deftypefn {GIMPLE function} gimple_debug_nonbind_marker_p (gimple g) Return true if g is a @code{GIMPLE_DEBUG} that marks a program location, without any variable binding. @@ -1541,6 +1546,7 @@ Set the conditional @code{COND_STMT} to be of the form 'if (1 == 1)'. @cindex @code{GIMPLE_DEBUG} @cindex @code{GIMPLE_DEBUG_BIND} @cindex @code{GIMPLE_DEBUG_BEGIN_STMT} +@cindex @code{GIMPLE_DEBUG_INLINE_ENTRY} @deftypefn {GIMPLE function} gdebug *gimple_build_debug_bind (tree var, @ tree value, gimple stmt) @@ -1626,6 +1632,18 @@ observable, and that none of the side effects of subsequent user statements are. @end deftypefn +@deftypefn {GIMPLE function} gimple gimple_build_debug_inline_entry (tree block, location_t location) +Build a @code{GIMPLE_DEBUG} statement with +@code{GIMPLE_DEBUG_INLINE_ENTRY} @code{subcode}. The effect of this +statement is to tell debug information generation machinery that a +function call at @code{location} underwent inline substitution, that +@code{block} is the enclosing lexical block created for the +substitution, and that at the point of the program in which the stmt is +inserted, all parameters for the inlined function are bound to the +respective arguments, and none of the side effects of its stmts are +observable. +@end deftypefn + @node @code{GIMPLE_EH_FILTER} @subsection @code{GIMPLE_EH_FILTER} @cindex @code{GIMPLE_EH_FILTER} diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi index 898574a01ad0..8d7d32177c16 100644 --- a/gcc/doc/rtl.texi +++ b/gcc/doc/rtl.texi @@ -3453,7 +3453,10 @@ Refers to a parameter that was completely optimized out. @item (debug_marker:@var{mode}) Marks a program location. With @code{VOIDmode}, it stands for the beginning of a statement, a recommended inspection point logically after -all prior side effects, and before any subsequent side effects. +all prior side effects, and before any subsequent side effects. With +@code{BLKmode}, it indicates an inline entry point: the lexical block +encoded in the @code{INSN_LOCATION} is the enclosing block that encloses +the inlined function. @end table @@ -3737,6 +3740,13 @@ This note is used to generate @code{is_stmt} markers in line number debuggign information. It indicates the beginning of a user statement. +@findex NOTE_INSN_INLINE_ENTRY +@item NOTE_INSN_INLINE_ENTRY +This note is used to generate @code{entry_pc} for inlined subroutines in +debugging information. It indicates an inspection point at which all +arguments for the inlined function have been bound, and before its first +statement. + @end table These codes are printed symbolically when they appear in debugging dumps. @@ -3754,8 +3764,12 @@ binds a user variable tree to an RTL representation of the it stands for the value bound to the corresponding @code{DEBUG_EXPR_DECL}. -@code{GIMPLE_DEBUG_BEGIN_STMT} is expanded to RTL as a @code{DEBUG_INSN} -with a @code{VOIDmode} @code{DEBUG_MARKER} @code{PATTERN}. These +@code{GIMPLE_DEBUG_BEGIN_STMT} and @code{GIMPLE_DEBUG_INLINE_ENTRY} are +expanded to RTL as a @code{DEBUG_INSN} with a @code{DEBUG_MARKER} +@code{PATTERN}; the difference is the RTL mode: the former's +@code{DEBUG_MARKER} is @code{VOIDmode}, whereas the latter is +@code{BLKmode}; information about the inlined function can be taken from +the lexical block encoded in the @code{INSN_LOCATION}. These @code{DEBUG_INSN}s, that do not carry @code{VAR_LOCATION} information, just @code{DEBUG_MARKER}s, can be detected by testing @code{DEBUG_MARKER_INSN_P}, whereas those that do can be recognized as @@ -3766,8 +3780,8 @@ with respect to each other, particularly during scheduling. Binding information is kept in pseudo-instruction form, so that, unlike notes, it gets the same treatment and adjustments that regular instructions would. It is the variable tracking pass that turns these -pseudo-instructions into @code{NOTE_INSN_VAR_LOCATION} and -@code{NOTE_INSN_BEGIN_STMT} notes, +pseudo-instructions into @code{NOTE_INSN_VAR_LOCATION}, +@code{NOTE_INSN_BEGIN_STMT} and @code{NOTE_INSN_INLINE_ENTRY} notes, analyzing control flow, value equivalences and changes to registers and memory referenced in value expressions, propagating the values of debug temporaries and determining expressions that can be used to compute the diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 5fdac8269654..9e2f73525b6a 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2720,6 +2720,7 @@ static void dwarf2out_imported_module_or_decl_1 (tree, tree, tree, dw_die_ref); static void dwarf2out_abstract_function (tree); static void dwarf2out_var_location (rtx_insn *); +static void dwarf2out_inline_entry (tree); static void dwarf2out_size_function (tree); static void dwarf2out_begin_function (tree); static void dwarf2out_end_function (unsigned int); @@ -2773,7 +2774,7 @@ const struct gcc_debug_hooks dwarf2_debug_hooks = debug_nothing_rtx_code_label, /* label */ debug_nothing_int, /* handle_pch */ dwarf2out_var_location, - debug_nothing_tree, /* inline_entry */ + dwarf2out_inline_entry, /* inline_entry */ dwarf2out_size_function, /* size_function */ dwarf2out_switch_text_section, dwarf2out_set_name, @@ -4011,6 +4012,9 @@ static char ranges_base_label[2 * MAX_ARTIFICIAL_LABEL_BYTES]; #ifndef BLOCK_BEGIN_LABEL #define BLOCK_BEGIN_LABEL "LBB" #endif +#ifndef BLOCK_INLINE_ENTRY_LABEL +#define BLOCK_INLINE_ENTRY_LABEL "LBI" +#endif #ifndef BLOCK_END_LABEL #define BLOCK_END_LABEL "LBE" #endif @@ -23046,6 +23050,48 @@ block_die_hasher::equal (die_struct *x, die_struct *y) return x->decl_id == y->decl_id && x->die_parent == y->die_parent; } +/* Hold information about markers for inlined entry points. */ +struct GTY ((for_user)) inline_entry_data +{ + /* The block that's the inlined_function_outer_scope for an inlined + function. */ + tree block; + + /* The label at the inlined entry point. */ + const char *label_pfx; + unsigned int label_num; + + /* The view number to be used as the inlined entry point. */ + var_loc_view view; +}; + +struct inline_entry_data_hasher : ggc_ptr_hash <inline_entry_data> +{ + typedef tree compare_type; + static inline hashval_t hash (const inline_entry_data *); + static inline bool equal (const inline_entry_data *, const_tree); +}; + +/* Hash table routines for inline_entry_data. */ + +inline hashval_t +inline_entry_data_hasher::hash (const inline_entry_data *data) +{ + return htab_hash_pointer (data->block); +} + +inline bool +inline_entry_data_hasher::equal (const inline_entry_data *data, + const_tree block) +{ + return data->block == block; +} + +/* Inlined entry points pending DIE creation in this compilation unit. */ + +static GTY(()) hash_table<inline_entry_data_hasher> *inline_entry_data_table; + + /* Return TRUE if DECL, which may have been previously generated as OLD_DIE, is a candidate for a DW_AT_specification. DECLARATION is true if decl (or its origin) is either an extern declaration or a @@ -23500,6 +23546,42 @@ add_high_low_attributes (tree stmt, dw_die_ref die) { char label[MAX_ARTIFICIAL_LABEL_BYTES]; + if (inline_entry_data **iedp + = !inline_entry_data_table ? NULL + : inline_entry_data_table->find_slot_with_hash (stmt, + htab_hash_pointer (stmt), + NO_INSERT)) + { + inline_entry_data *ied = *iedp; + gcc_assert (MAY_HAVE_DEBUG_MARKER_INSNS); + gcc_assert (inlined_function_outer_scope_p (stmt)); + ASM_GENERATE_INTERNAL_LABEL (label, ied->label_pfx, ied->label_num); + add_AT_lbl_id (die, DW_AT_entry_pc, label); + + if (debug_variable_location_views && !ZERO_VIEW_P (ied->view)) + { + if (!output_asm_line_debug_info ()) + add_AT_unsigned (die, DW_AT_GNU_entry_view, ied->view); + else + { + ASM_GENERATE_INTERNAL_LABEL (label, "LVU", ied->view); + /* FIXME: this will resolve to a small number. Could we + possibly emit smaller data? Ideally we'd emit a + uleb128, but that would make the size of DIEs + impossible for the compiler to compute, since it's + the assembler that computes the value of the view + label in this case. Ideally, we'd have a single form + encompassing both the address and the view, and + indirecting them through a table might make things + easier, but even that would be more wasteful, + space-wise, than what we have now. */ + add_AT_lbl_id (die, DW_AT_GNU_entry_view, label); + } + } + + inline_entry_data_table->clear_slot (iedp); + } + if (BLOCK_FRAGMENT_CHAIN (stmt) && (dwarf_version >= 3 || !dwarf_strict)) { @@ -23507,7 +23589,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die) dw_die_ref pdie; dw_attr_node *attr = NULL; - if (inlined_function_outer_scope_p (stmt)) + if (!MAY_HAVE_DEBUG_MARKER_INSNS && inlined_function_outer_scope_p (stmt)) { ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_BEGIN_LABEL, BLOCK_NUMBER (stmt)); @@ -23672,7 +23754,7 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die) dw_die_ref subr_die = new_die (DW_TAG_inlined_subroutine, context_die, stmt); - if (call_arg_locations) + if (call_arg_locations || MAY_HAVE_DEBUG_MARKER_INSNS) BLOCK_DIE (stmt) = subr_die; add_abstract_origin_attribute (subr_die, decl); if (TREE_ASM_WRITTEN (stmt)) @@ -26701,6 +26783,7 @@ dwarf2out_var_location (rtx_insn *loc_note) || ! NOTE_P (next_note) || (NOTE_KIND (next_note) != NOTE_INSN_VAR_LOCATION && NOTE_KIND (next_note) != NOTE_INSN_BEGIN_STMT + && NOTE_KIND (next_note) != NOTE_INSN_INLINE_ENTRY && NOTE_KIND (next_note) != NOTE_INSN_CALL_ARG_LOCATION)) next_note = NULL; @@ -26889,6 +26972,113 @@ create_label: last_in_cold_section_p = in_cold_section_p; } +/* Check whether BLOCK, a lexical block, is nested within OUTER, or is + OUTER itself. If BOTHWAYS, check not only that BLOCK can reach + OUTER through BLOCK_SUPERCONTEXT links, but also that there is a + path from OUTER to BLOCK through BLOCK_SUBBLOCKs and + BLOCK_FRAGMENT_ORIGIN links. */ +static bool +block_within_block_p (tree block, tree outer, bool bothways) +{ + if (block == outer) + return true; + + /* Quickly check that OUTER is up BLOCK's supercontext chain. */ + for (tree context = BLOCK_SUPERCONTEXT (block); + context != outer; + context = BLOCK_SUPERCONTEXT (context)) + if (!context || TREE_CODE (context) != BLOCK) + return false; + + if (!bothways) + return true; + + /* Now check that each block is actually referenced by its + parent. */ + for (tree context = BLOCK_SUPERCONTEXT (block); ; + context = BLOCK_SUPERCONTEXT (context)) + { + if (BLOCK_FRAGMENT_ORIGIN (context)) + { + gcc_assert (!BLOCK_SUBBLOCKS (context)); + context = BLOCK_FRAGMENT_ORIGIN (context); + } + for (tree sub = BLOCK_SUBBLOCKS (context); + sub != block; + sub = BLOCK_CHAIN (sub)) + if (!sub) + return false; + if (context == outer) + return true; + else + block = context; + } +} + +/* Called during final while assembling the marker of the entry point + for an inlined function. */ + +static void +dwarf2out_inline_entry (tree block) +{ + gcc_assert (DECL_P (block_ultimate_origin (block))); + + /* Sanity check the block tree. This would catch a case in which + BLOCK got removed from the tree reachable from the outermost + lexical block, but got retained in markers. It would still link + back to its parents, but some ancestor would be missing a link + down the path to the sub BLOCK. If the block got removed, its + BLOCK_NUMBER will not be a usable value. */ + gcc_checking_assert (block_within_block_p (block, + DECL_INITIAL + (current_function_decl), + true)); + + gcc_assert (inlined_function_outer_scope_p (block)); + gcc_assert (!BLOCK_DIE (block)); + + /* If we can't represent it, don't bother. */ + if (!(dwarf_version >= 3 || !dwarf_strict)) + return; + + if (BLOCK_FRAGMENT_ORIGIN (block)) + block = BLOCK_FRAGMENT_ORIGIN (block); + /* Can the entry point ever not be at the beginning of an + unfragmented lexical block? */ + else if (!(BLOCK_FRAGMENT_CHAIN (block) + || (cur_line_info_table + && !ZERO_VIEW_P (cur_line_info_table->view)))) + return; + + if (!inline_entry_data_table) + inline_entry_data_table + = hash_table<inline_entry_data_hasher>::create_ggc (10); + + + inline_entry_data **iedp + = inline_entry_data_table->find_slot_with_hash (block, + htab_hash_pointer (block), + INSERT); + if (*iedp) + /* ??? Ideally, we'd record all entry points for the same inlined + function (some may have been duplicated by e.g. unrolling), but + we have no way to represent that ATM. */ + return; + + inline_entry_data *ied = *iedp = ggc_cleared_alloc<inline_entry_data> (); + ied->block = block; + ied->label_pfx = BLOCK_INLINE_ENTRY_LABEL; + ied->label_num = BLOCK_NUMBER (block); + if (cur_line_info_table) + ied->view = cur_line_info_table->view; + + char label[MAX_ARTIFICIAL_LABEL_BYTES]; + + ASM_GENERATE_INTERNAL_LABEL (label, BLOCK_INLINE_ENTRY_LABEL, + BLOCK_NUMBER (block)); + ASM_OUTPUT_LABEL (asm_out_file, label); +} + /* Called from finalize_size_functions for size functions so that their body can be encoded in the debug info to describe the layout of variable-length structures. */ @@ -30360,6 +30550,9 @@ dwarf2out_finish (const char *) /* Flush out any latecomers to the limbo party. */ flush_limbo_die_list (); + if (inline_entry_data_table) + gcc_assert (inline_entry_data_table->elements () == 0); + if (flag_checking) { verify_die (comp_unit_die ()); diff --git a/gcc/final.c b/gcc/final.c index c6a1d5b7e05a..afb2988f8207 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -1672,6 +1672,7 @@ reemit_insn_block_notes (void) break; case NOTE_INSN_BEGIN_STMT: + case NOTE_INSN_INLINE_ENTRY: this_block = LOCATION_BLOCK (NOTE_MARKER_LOCATION (insn)); goto set_cur_block_to_this_block; @@ -2531,6 +2532,7 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, if (!DECL_IGNORED_P (current_function_decl) && notice_source_line (insn, NULL)) { + output_source_line: (*debug_hooks->source_line) (last_linenum, last_columnnum, last_filename, last_discriminator, true); @@ -2538,6 +2540,18 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED, } break; + case NOTE_INSN_INLINE_ENTRY: + gcc_checking_assert (cfun->debug_nonbind_markers); + if (!DECL_IGNORED_P (current_function_decl)) + { + if (!notice_source_line (insn, NULL)) + break; + (*debug_hooks->inline_entry) (LOCATION_BLOCK + (NOTE_MARKER_LOCATION (insn))); + goto output_source_line; + } + break; + default: gcc_unreachable (); break; @@ -3241,6 +3255,17 @@ notice_source_line (rtx_insn *insn, bool *is_stmt) if (NOTE_MARKER_P (insn)) { location_t loc = NOTE_MARKER_LOCATION (insn); + /* The inline entry markers (gimple, insn, note) carry the + location of the call, because that's what we want to carry + during compilation, but the location we want to output in + debug information for the inline entry point is the location + of the function itself. */ + if (NOTE_KIND (insn) == NOTE_INSN_INLINE_ENTRY) + { + tree block = LOCATION_BLOCK (loc); + tree fn = block_ultimate_origin (block); + loc = DECL_SOURCE_LOCATION (fn); + } expanded_location xloc = expand_location (loc); if (xloc.line == 0) { @@ -4839,6 +4864,7 @@ rest_of_clean_state (void) && (!NOTE_P (insn) || (NOTE_KIND (insn) != NOTE_INSN_VAR_LOCATION && NOTE_KIND (insn) != NOTE_INSN_BEGIN_STMT + && NOTE_KIND (insn) != NOTE_INSN_INLINE_ENTRY && NOTE_KIND (insn) != NOTE_INSN_CALL_ARG_LOCATION && NOTE_KIND (insn) != NOTE_INSN_BLOCK_BEG && NOTE_KIND (insn) != NOTE_INSN_BLOCK_END diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 61b1f2c89cbb..5e832a78475d 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -1370,6 +1370,19 @@ dump_gimple_debug (pretty_printer *buffer, gdebug *gs, int spc, dump_gimple_fmt (buffer, spc, flags, "# DEBUG BEGIN_STMT"); break; + case GIMPLE_DEBUG_INLINE_ENTRY: + if (flags & TDF_RAW) + dump_gimple_fmt (buffer, spc, flags, "%G INLINE_ENTRY %T", gs, + gimple_block (gs) + ? block_ultimate_origin (gimple_block (gs)) + : NULL_TREE); + else + dump_gimple_fmt (buffer, spc, flags, "# DEBUG INLINE_ENTRY %T", + gimple_block (gs) + ? block_ultimate_origin (gimple_block (gs)) + : NULL_TREE); + break; + default: gcc_unreachable (); } diff --git a/gcc/gimple.c b/gcc/gimple.c index 66496d002a8d..da6df9898512 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -873,6 +873,27 @@ gimple_build_debug_begin_stmt (tree block, location_t location } +/* Build a new GIMPLE_DEBUG_INLINE_ENTRY statement in BLOCK at + LOCATION. The BLOCK links to the inlined function. */ + +gdebug * +gimple_build_debug_inline_entry (tree block, location_t location + MEM_STAT_DECL) +{ + gdebug *p + = as_a <gdebug *> ( + gimple_build_with_ops_stat (GIMPLE_DEBUG, + (unsigned)GIMPLE_DEBUG_INLINE_ENTRY, 0 + PASS_MEM_STAT)); + + gimple_set_location (p, location); + gimple_set_block (p, block); + cfun->debug_marker_count++; + + return p; +} + + /* Build a GIMPLE_OMP_CRITICAL statement. BODY is the sequence of statements for which only one thread can execute. diff --git a/gcc/gimple.h b/gcc/gimple.h index f44887ad7a9b..e925c329a347 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -204,7 +204,8 @@ enum gf_mask { enum gimple_debug_subcode { GIMPLE_DEBUG_BIND = 0, GIMPLE_DEBUG_SOURCE_BIND = 1, - GIMPLE_DEBUG_BEGIN_STMT = 2 + GIMPLE_DEBUG_BEGIN_STMT = 2, + GIMPLE_DEBUG_INLINE_ENTRY = 3 }; /* Masks for selecting a pass local flag (PLF) to work on. These @@ -1456,6 +1457,7 @@ geh_dispatch *gimple_build_eh_dispatch (int); gdebug *gimple_build_debug_bind (tree, tree, gimple * CXX_MEM_STAT_INFO); gdebug *gimple_build_debug_source_bind (tree, tree, gimple * CXX_MEM_STAT_INFO); gdebug *gimple_build_debug_begin_stmt (tree, location_t CXX_MEM_STAT_INFO); +gdebug *gimple_build_debug_inline_entry (tree, location_t CXX_MEM_STAT_INFO); gomp_critical *gimple_build_omp_critical (gimple_seq, tree, tree); gomp_for *gimple_build_omp_for (gimple_seq, int, tree, size_t, gimple_seq); gomp_parallel *gimple_build_omp_parallel (gimple_seq, tree, tree, tree); @@ -4786,13 +4788,25 @@ gimple_debug_begin_stmt_p (const gimple *s) return false; } +/* Return true if S is a GIMPLE_DEBUG INLINE_ENTRY statement. */ + +static inline bool +gimple_debug_inline_entry_p (const gimple *s) +{ + if (is_gimple_debug (s)) + return s->subcode == GIMPLE_DEBUG_INLINE_ENTRY; + + return false; +} + /* Return true if S is a GIMPLE_DEBUG non-binding marker statement. */ static inline bool gimple_debug_nonbind_marker_p (const gimple *s) { if (is_gimple_debug (s)) - return s->subcode == GIMPLE_DEBUG_BEGIN_STMT; + return s->subcode == GIMPLE_DEBUG_BEGIN_STMT + || s->subcode == GIMPLE_DEBUG_INLINE_ENTRY; return false; } diff --git a/gcc/insn-notes.def b/gcc/insn-notes.def index 960487b31db1..252e95722447 100644 --- a/gcc/insn-notes.def +++ b/gcc/insn-notes.def @@ -71,6 +71,10 @@ INSN_NOTE (CALL_ARG_LOCATION) /* The beginning of a statement. */ INSN_NOTE (BEGIN_STMT) +/* The entry point for an inlined function. Its NOTE_BLOCK references + the lexical block whose abstract origin is the inlined function. */ +INSN_NOTE (INLINE_ENTRY) + /* Record the struct for the following basic block. Uses NOTE_BASIC_BLOCK. FIXME: Redundant with the basic block pointer now included in every insn. NOTE: If there's no CFG anymore, in other words, diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c index acbe2f37748a..277ef16c8499 100644 --- a/gcc/print-rtl.c +++ b/gcc/print-rtl.c @@ -259,6 +259,7 @@ rtx_writer::print_rtx_operand_code_0 (const_rtx in_rtx ATTRIBUTE_UNUSED, break; case NOTE_INSN_BEGIN_STMT: + case NOTE_INSN_INLINE_ENTRY: #ifndef GENERATOR_FILE { expanded_location xloc @@ -1809,6 +1810,10 @@ print_insn (pretty_printer *pp, const rtx_insn *x, int verbose) pp_string (pp, "debug begin stmt marker"); break; + case NOTE_INSN_INLINE_ENTRY: + pp_string (pp, "debug inline entry marker"); + break; + default: gcc_unreachable (); } diff --git a/gcc/rtl.h b/gcc/rtl.h index 3ef687e5a371..63df02e0376a 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -1625,7 +1625,8 @@ extern const char * const reg_note_name[]; for which NOTE_MARKER_LOCATION can be used. */ #define NOTE_MARKER_P(INSN) \ (NOTE_P (INSN) && \ - (NOTE_KIND (INSN) == NOTE_INSN_BEGIN_STMT)) + (NOTE_KIND (INSN) == NOTE_INSN_BEGIN_STMT \ + || NOTE_KIND (INSN) == NOTE_INSN_INLINE_ENTRY)) /* Variable declaration and the location of a variable. */ #define PAT_VAR_LOCATION_DECL(PAT) (XCTREE ((PAT), 0, VAR_LOCATION)) @@ -1663,6 +1664,8 @@ extern const char * const reg_note_name[]; (GET_CODE (PATTERN (INSN)) == DEBUG_MARKER \ ? (GET_MODE (PATTERN (INSN)) == VOIDmode \ ? NOTE_INSN_BEGIN_STMT \ + : GET_MODE (PATTERN (INSN)) == BLKmode \ + ? NOTE_INSN_INLINE_ENTRY \ : (enum insn_note)-1) \ : (enum insn_note)-1) /* Create patterns for debug markers. These and the above abstract @@ -1672,6 +1675,8 @@ extern const char * const reg_note_name[]; wouldn't be a problem. */ #define GEN_RTX_DEBUG_MARKER_BEGIN_STMT_PAT() \ gen_rtx_DEBUG_MARKER (VOIDmode) +#define GEN_RTX_DEBUG_MARKER_INLINE_ENTRY_PAT() \ + gen_rtx_DEBUG_MARKER (BLKmode) /* The VAR_LOCATION rtx in a DEBUG_INSN. */ #define INSN_VAR_LOCATION(INSN) \ diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index f858d426a500..8fd757fd40e6 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -4631,6 +4631,13 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id) GSI_NEW_STMT); } initialize_inlined_parameters (id, stmt, fn, bb); + if (debug_nonbind_markers_p && id->block + && inlined_function_outer_scope_p (id->block)) + { + gimple_stmt_iterator si = gsi_last_bb (bb); + gsi_insert_after (&si, gimple_build_debug_inline_entry + (id->block, input_location), GSI_NEW_STMT); + } if (DECL_INITIAL (fn)) { diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c index 8738fe21a6e1..734d15df2c51 100644 --- 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; else /* Verfify that only blocks with source location set @@ -640,6 +647,16 @@ dump_scope_block (FILE *file, int indent, tree scope, dump_flags_t flags) fprintf (file, "#%i", BLOCK_NUMBER (origin)); } } + if (BLOCK_FRAGMENT_ORIGIN (scope)) + fprintf (file, " Fragment of : #%i", + BLOCK_NUMBER (BLOCK_FRAGMENT_ORIGIN (scope))); + else if (BLOCK_FRAGMENT_CHAIN (scope)) + { + fprintf (file, " Fragment chain :"); + for (t = BLOCK_FRAGMENT_CHAIN (scope); t ; + t = BLOCK_FRAGMENT_CHAIN (t)) + fprintf (file, " #%i", BLOCK_NUMBER (t)); + } fprintf (file, " \n"); for (var = BLOCK_VARS (scope); var; var = DECL_CHAIN (var)) { diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index 8e500b144712..4f8f9f182203 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -9938,6 +9938,7 @@ reemit_marker_as_note (rtx_insn *insn, basic_block *bb) switch (kind) { case NOTE_INSN_BEGIN_STMT: + case NOTE_INSN_INLINE_ENTRY: { rtx_insn *note = NULL; if (cfun->debug_nonbind_markers) diff --git a/include/dwarf2.def b/include/dwarf2.def index f3fa4009207b..8e731a3b87a6 100644 --- a/include/dwarf2.def +++ b/include/dwarf2.def @@ -444,6 +444,7 @@ DW_AT (DW_AT_GNU_pubtypes, 0x2135) See http://gcc.gnu.org/wiki/Discriminator */ DW_AT (DW_AT_GNU_discriminator, 0x2136) DW_AT (DW_AT_GNU_locviews, 0x2137) +DW_AT (DW_AT_GNU_entry_view, 0x2138) /* VMS extensions. */ DW_AT (DW_AT_VMS_rtnbeg_pd_address, 0x2201) /* GNAT extensions. */