On Tue, Jul 13, 2021 at 5:13 AM Alexandre Oliva <ol...@adacore.com> wrote: > > > If some IPA pass replaces the only reference to a constant non-public > non-automatic variable with its initializer, namely the address of > another such variable, the former becomes unreferenced and it's > discarded by symbol_table::remove_unreachable_nodes. It calls > debug_hooks->late_global_decl while at that, and this expands the > initializer, which assigs RTL to the latter variable and forces it to > be retained by remove_unreferenced_decls, and eventually be output > despite not being referenced. Without debug information, it's not > output. > > This has caused a bootstrap-debug compare failure in > libdecnumber/decContext.o, while developing a transformation that ends > up enabling the above substitution in constprop. > > This patch makes reference_to_unused slightly more conservative about > such variables at the end of IPA passes, falling back onto > expand_debug_expr for expressions referencing symbols that might or > might not be output, avoiding the loss of debug information when the > symbol is output, while avoiding a symbol output only because of debug > info. > > Regstrapped on x86_64-linux-gnu. Ok to install? > > > for gcc/ChangeLog > > * dwarf2out.c (add_const_value_attribute): Return false if > resolve_one_addr fails. > (reference_to_unused): Don't assume local symbol presence > while it can still be optimized out. > (rtl_for_decl_init): Fallback to expand_debug_expr. > * cfgexpand.c (expand_debug_expr): Export. > * expr.h (expand_debug_expr): Declare. > --- > gcc/cfgexpand.c | 12 +++++++++--- > gcc/dwarf2out.c | 15 +++++++++++++-- > gcc/expr.h | 2 ++ > 3 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c > index 3edd53c37dcb3..b731a5598230c 100644 > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -91,8 +91,6 @@ struct ssaexpand SA; > of comminucating the profile info to the builtin expanders. */ > gimple *currently_expanding_gimple_stmt; > > -static rtx expand_debug_expr (tree); > - > static bool defer_stack_allocation (tree, bool); > > static void record_alignment_for_reg_var (unsigned int); > @@ -4413,7 +4411,7 @@ expand_debug_parm_decl (tree decl) > > /* Return an RTX equivalent to the value of the tree expression EXP. */ > > -static rtx > +rtx > expand_debug_expr (tree exp) > { > rtx op0 = NULL_RTX, op1 = NULL_RTX, op2 = NULL_RTX; > @@ -5285,6 +5283,14 @@ expand_debug_expr (tree exp) > else > goto flag_unsupported; > > + case COMPOUND_LITERAL_EXPR: > + exp = COMPOUND_LITERAL_EXPR_DECL_EXPR (exp); > + /* DECL_EXPR is a tcc_statement, which expand_debug_expr does > + not expect, so instead of recursing we take care of it right > + away. */ > + exp = DECL_EXPR_DECL (exp); > + return expand_debug_expr (exp); > + > case CALL_EXPR: > /* ??? Maybe handle some builtins? */ > return NULL; > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index 82783c4968b85..bb7e2b8dc4e2c 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -20170,7 +20170,8 @@ add_const_value_attribute (dw_die_ref die, rtx rtl) > if (dwarf_version >= 4 || !dwarf_strict) > { > dw_loc_descr_ref loc_result; > - resolve_one_addr (&rtl); > + if (!resolve_one_addr (&rtl)) > + return false; > rtl_addr: > loc_result = new_addr_loc_descr (rtl, dtprel_false); > add_loc_descr (&loc_result, new_loc_descr (DW_OP_stack_value, 0, > 0)); > @@ -20255,6 +20256,12 @@ reference_to_unused (tree * tp, int * walk_subtrees, > varpool_node *node = varpool_node::get (*tp); > if (!node || !node->definition) > return *tp; > + /* If it's local, it might still be optimized out, unless we've > + already committed to outputting it by assigning RTL to it. */ > + if (! TREE_PUBLIC (*tp) && ! TREE_ASM_WRITTEN (*tp) > + && symtab->state <= IPA_SSA_AFTER_INLINING
Hmm, elsewhere in this function we're not anticipating future removal but instead use ->global_info_ready which IIRC is when the unit was initially analyzed. So don't the other uses have the same issue? Maybe reference_to_unused is the wrong tool here and we need a reference_to_discardable or so? In other places we manage to use symbolic DIE references later resolved by note_variable_values, can we maybe do this unconditionally for the initializers of removed decls somehow? > + && ! DECL_RTL_SET_P (*tp)) > + return *tp; > } > else if (TREE_CODE (*tp) == FUNCTION_DECL > && (!DECL_EXTERNAL (*tp) || DECL_DECLARED_INLINE_P (*tp))) > @@ -20279,6 +20286,7 @@ static rtx > rtl_for_decl_init (tree init, tree type) > { > rtx rtl = NULL_RTX; > + bool valid_p = false; > > STRIP_NOPS (init); > > @@ -20322,7 +20330,7 @@ rtl_for_decl_init (tree init, tree type) > /* If the initializer is something that we know will expand into an > immediate RTL constant, expand it now. We must be careful not to > reference variables which won't be output. */ > - else if (initializer_constant_valid_p (init, type) > + else if ((valid_p = initializer_constant_valid_p (init, type)) > && ! walk_tree (&init, reference_to_unused, NULL, NULL)) > { > /* Convert vector CONSTRUCTOR initializers to VECTOR_CST if > @@ -20367,6 +20375,9 @@ rtl_for_decl_init (tree init, tree type) > /* If expand_expr returns a MEM, it wasn't immediate. */ > gcc_assert (!rtl || !MEM_P (rtl)); > } > + else if (valid_p) > + /* Perhaps we could just use this and skip all of the above? */ > + rtl = expand_debug_expr (init); > > return rtl; > } > diff --git a/gcc/expr.h b/gcc/expr.h > index a4f44265759ce..7b060462020be 100644 > --- a/gcc/expr.h > +++ b/gcc/expr.h > @@ -307,6 +307,8 @@ expand_normal (tree exp) > return expand_expr_real (exp, NULL_RTX, VOIDmode, EXPAND_NORMAL, NULL, > false); > } > > +/* This one is defined in cfgexpand.c. */ > +extern rtx expand_debug_expr (tree); > > /* Return STRING_CST and set offset, size and decl, if the first > argument corresponds to a string constant. */ > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Disinformation flourishes because many people care deeply about injustice > but very few check the facts. Ask me about <https://stallmansupport.org>