dwarf2out: Don't call expand_expr during early_dwarf [PR104407]
Commit Message
Hi!
As mentioned in the PR, since PR96690 r11-2834 we call rtl_for_decl_init
which can call expand_expr already during early_dwarf. The comment and PR
explains it that the intent is to ensure the referenced vars and functions
are properly mangled because free_lang_data doesn't cover everything, like
template parameters etc. It doesn't work well though, because expand_expr
can set DECL_RTLs e.g. on referenced vars and keep them there, and they can
be created e.g. with different MEM_ALIGN compared to what they would be
created with if they were emitted later.
So, the following patch stops calling rtl_for_decl_init and instead
for cases for which rtl_for_decl_init does anything at all walks the
initializer and ensures referenced vars or functions are mangled.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2022-02-08 Jakub Jelinek <jakub@redhat.com>
PR debug/104407
* dwarf2out.cc (mangle_referenced_decls): New function.
(tree_add_const_value_attribute): Don't call rtl_for_decl_init if
early_dwarf. Instead walk the initializer and try to mangle vars or
functions referenced from it.
* g++.dg/debug/dwarf2/pr104407.C: New test.
Jakub
Comments
On Tue, 8 Feb 2022, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, since PR96690 r11-2834 we call rtl_for_decl_init
> which can call expand_expr already during early_dwarf. The comment and PR
> explains it that the intent is to ensure the referenced vars and functions
> are properly mangled because free_lang_data doesn't cover everything, like
> template parameters etc. It doesn't work well though, because expand_expr
free_lang_data mangling also only runs when generating LTO bytecode...
> can set DECL_RTLs e.g. on referenced vars and keep them there, and they can
> be created e.g. with different MEM_ALIGN compared to what they would be
> created with if they were emitted later.
> So, the following patch stops calling rtl_for_decl_init and instead
> for cases for which rtl_for_decl_init does anything at all walks the
> initializer and ensures referenced vars or functions are mangled.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2022-02-08 Jakub Jelinek <jakub@redhat.com>
>
> PR debug/104407
> * dwarf2out.cc (mangle_referenced_decls): New function.
> (tree_add_const_value_attribute): Don't call rtl_for_decl_init if
> early_dwarf. Instead walk the initializer and try to mangle vars or
> functions referenced from it.
>
> * g++.dg/debug/dwarf2/pr104407.C: New test.
>
> --- gcc/dwarf2out.cc.jj 2022-02-04 14:36:54.966605844 +0100
> +++ gcc/dwarf2out.cc 2022-02-07 12:28:13.000520666 +0100
> @@ -20881,6 +20881,19 @@ add_location_or_const_value_attribute (d
> return tree_add_const_value_attribute_for_decl (die, decl);
> }
>
> +/* Mangle referenced decls. */
> +static tree
> +mangle_referenced_decls (tree *tp, int *walk_subtrees, void *)
> +{
> + if (! EXPR_P (*tp) && ! CONSTANT_CLASS_P (*tp))
> + *walk_subtrees = 0;
> +
> + if (VAR_OR_FUNCTION_DECL_P (*tp))
> + assign_assembler_name_if_needed (*tp);
> +
> + return NULL_TREE;
> +}
> +
> /* Attach a DW_AT_const_value attribute to DIE. The value of the
> attribute is the const value T. */
>
> @@ -20889,7 +20902,6 @@ tree_add_const_value_attribute (dw_die_r
> {
> tree init;
> tree type = TREE_TYPE (t);
> - rtx rtl;
>
> if (!t || !TREE_TYPE (t) || TREE_TYPE (t) == error_mark_node)
> return false;
> @@ -20910,11 +20922,26 @@ tree_add_const_value_attribute (dw_die_r
> return true;
> }
> }
> - /* Generate the RTL even if early_dwarf to force mangling of all refered to
> - symbols. */
> - rtl = rtl_for_decl_init (init, type);
> - if (rtl && !early_dwarf)
> - return add_const_value_attribute (die, TYPE_MODE (type), rtl);
> + if (!early_dwarf)
> + {
> + rtx rtl = rtl_for_decl_init (init, type);
> + if (rtl && !early_dwarf)
the !early_dwarf is redundant
Otherwise looks good to me, please give Jason the opportunity to comment
though.
Thanks,
Richard.
> + return add_const_value_attribute (die, TYPE_MODE (type), rtl);
> + }
> + else
> + {
> + /* For early_dwarf force mangling of all referenced symbols. */
> + tree initializer = init;
> + STRIP_NOPS (initializer);
> + /* rtl_for_decl_init punts on other aggregates, and complex values. */
> + if (AGGREGATE_TYPE_P (type)
> + || (TREE_CODE (initializer) == VIEW_CONVERT_EXPR
> + && AGGREGATE_TYPE_P (TREE_TYPE (TREE_OPERAND (initializer, 0))))
> + || TREE_CODE (type) == COMPLEX_TYPE)
> + ;
> + else if (initializer_constant_valid_p (initializer, type))
> + walk_tree (&initializer, mangle_referenced_decls, NULL, NULL);
> + }
> /* If the host and target are sane, try harder. */
> if (CHAR_BIT == 8 && BITS_PER_UNIT == 8
> && initializer_constant_valid_p (init, type))
> --- gcc/testsuite/g++.dg/debug/dwarf2/pr104407.C.jj 2022-02-07 10:45:51.945041031 +0100
> +++ gcc/testsuite/g++.dg/debug/dwarf2/pr104407.C 2022-02-07 10:45:09.834635340 +0100
> @@ -0,0 +1,12 @@
> +// PR debug/104407
> +// { dg-do compile { target c++17 } }
> +// { dg-options "-O1 -fcompare-debug" }
> +
> +struct A { int i; long j; int k : 2; char l; } a;
> +
> +auto [ aa, bb, cc, dd ] = a;
> +
> +namespace N
> +{
> + auto & [ m, n, o, ppp ] = a;
> +}
>
> Jakub
>
>
On 2/8/22 03:35, Richard Biener wrote:
> On Tue, 8 Feb 2022, Jakub Jelinek wrote:
>
>> Hi!
>>
>> As mentioned in the PR, since PR96690 r11-2834 we call rtl_for_decl_init
>> which can call expand_expr already during early_dwarf. The comment and PR
>> explains it that the intent is to ensure the referenced vars and functions
>> are properly mangled because free_lang_data doesn't cover everything, like
>> template parameters etc. It doesn't work well though, because expand_expr
>
> free_lang_data mangling also only runs when generating LTO bytecode...
>
>> can set DECL_RTLs e.g. on referenced vars and keep them there, and they can
>> be created e.g. with different MEM_ALIGN compared to what they would be
>> created with if they were emitted later.
>> So, the following patch stops calling rtl_for_decl_init and instead
>> for cases for which rtl_for_decl_init does anything at all walks the
>> initializer and ensures referenced vars or functions are mangled.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>
>> 2022-02-08 Jakub Jelinek <jakub@redhat.com>
>>
>> PR debug/104407
>> * dwarf2out.cc (mangle_referenced_decls): New function.
>> (tree_add_const_value_attribute): Don't call rtl_for_decl_init if
>> early_dwarf. Instead walk the initializer and try to mangle vars or
>> functions referenced from it.
>>
>> * g++.dg/debug/dwarf2/pr104407.C: New test.
>>
>> --- gcc/dwarf2out.cc.jj 2022-02-04 14:36:54.966605844 +0100
>> +++ gcc/dwarf2out.cc 2022-02-07 12:28:13.000520666 +0100
>> @@ -20881,6 +20881,19 @@ add_location_or_const_value_attribute (d
>> return tree_add_const_value_attribute_for_decl (die, decl);
>> }
>>
>> +/* Mangle referenced decls. */
>> +static tree
>> +mangle_referenced_decls (tree *tp, int *walk_subtrees, void *)
>> +{
>> + if (! EXPR_P (*tp) && ! CONSTANT_CLASS_P (*tp))
>> + *walk_subtrees = 0;
>> +
>> + if (VAR_OR_FUNCTION_DECL_P (*tp))
>> + assign_assembler_name_if_needed (*tp);
>> +
>> + return NULL_TREE;
>> +}
>> +
>> /* Attach a DW_AT_const_value attribute to DIE. The value of the
>> attribute is the const value T. */
>>
>> @@ -20889,7 +20902,6 @@ tree_add_const_value_attribute (dw_die_r
>> {
>> tree init;
>> tree type = TREE_TYPE (t);
>> - rtx rtl;
>>
>> if (!t || !TREE_TYPE (t) || TREE_TYPE (t) == error_mark_node)
>> return false;
>> @@ -20910,11 +20922,26 @@ tree_add_const_value_attribute (dw_die_r
>> return true;
>> }
>> }
>> - /* Generate the RTL even if early_dwarf to force mangling of all refered to
>> - symbols. */
>> - rtl = rtl_for_decl_init (init, type);
>> - if (rtl && !early_dwarf)
>> - return add_const_value_attribute (die, TYPE_MODE (type), rtl);
>> + if (!early_dwarf)
>> + {
>> + rtx rtl = rtl_for_decl_init (init, type);
>> + if (rtl && !early_dwarf)
>
> the !early_dwarf is redundant
>
> Otherwise looks good to me, please give Jason the opportunity to comment
> though.
>
> Thanks,
> Richard.
>
>> + return add_const_value_attribute (die, TYPE_MODE (type), rtl);
>> + }
>> + else
>> + {
>> + /* For early_dwarf force mangling of all referenced symbols. */
>> + tree initializer = init;
>> + STRIP_NOPS (initializer);
>> + /* rtl_for_decl_init punts on other aggregates, and complex values. */
>> + if (AGGREGATE_TYPE_P (type)
>> + || (TREE_CODE (initializer) == VIEW_CONVERT_EXPR
>> + && AGGREGATE_TYPE_P (TREE_TYPE (TREE_OPERAND (initializer, 0))))
>> + || TREE_CODE (type) == COMPLEX_TYPE)
>> + ;
Hmm, I don't think we need to mirror the limitations of
rtl_for_decl_init here; IMO we might as well go ahead mangle everything
so we don't need to change this place as well when the FIXME is
resolved. OK either way, but if you decide to keep this condition
please update the FIXME in rtl_for_decl_init to point here.
>> + else if (initializer_constant_valid_p (initializer, type))
>> + walk_tree (&initializer, mangle_referenced_decls, NULL, NULL);
>> + }
>> /* If the host and target are sane, try harder. */
>> if (CHAR_BIT == 8 && BITS_PER_UNIT == 8
>> && initializer_constant_valid_p (init, type))
>> --- gcc/testsuite/g++.dg/debug/dwarf2/pr104407.C.jj 2022-02-07 10:45:51.945041031 +0100
>> +++ gcc/testsuite/g++.dg/debug/dwarf2/pr104407.C 2022-02-07 10:45:09.834635340 +0100
>> @@ -0,0 +1,12 @@
>> +// PR debug/104407
>> +// { dg-do compile { target c++17 } }
>> +// { dg-options "-O1 -fcompare-debug" }
>> +
>> +struct A { int i; long j; int k : 2; char l; } a;
>> +
>> +auto [ aa, bb, cc, dd ] = a;
>> +
>> +namespace N
>> +{
>> + auto & [ m, n, o, ppp ] = a;
>> +}
>>
>> Jakub
>>
>>
>
On Tue, Feb 08, 2022 at 02:20:28PM -0500, Jason Merrill wrote:
> > > + return add_const_value_attribute (die, TYPE_MODE (type), rtl);
> > > + }
> > > + else
> > > + {
> > > + /* For early_dwarf force mangling of all referenced symbols. */
> > > + tree initializer = init;
> > > + STRIP_NOPS (initializer);
> > > + /* rtl_for_decl_init punts on other aggregates, and complex values. */
> > > + if (AGGREGATE_TYPE_P (type)
> > > + || (TREE_CODE (initializer) == VIEW_CONVERT_EXPR
> > > + && AGGREGATE_TYPE_P (TREE_TYPE (TREE_OPERAND (initializer, 0))))
> > > + || TREE_CODE (type) == COMPLEX_TYPE)
> > > + ;
>
> Hmm, I don't think we need to mirror the limitations of rtl_for_decl_init
> here; IMO we might as well go ahead mangle everything so we don't need to
> change this place as well when the FIXME is resolved. OK either way, but if
> you decide to keep this condition please update the FIXME in
> rtl_for_decl_init to point here.
I did that because the aggregate case can mean very large initializers that
would need mangling a lot of symbols even when we actually don't need that
because we'd punt on it, and because I'm a little bit afraid how safe the
mangling is for -fcompare-debug if done only for the -g case (I vaguely
remember it can try to instantiate some templates etc.).
Can certainly adjust the FIXME comment.
Jakub
@@ -20881,6 +20881,19 @@ add_location_or_const_value_attribute (d
return tree_add_const_value_attribute_for_decl (die, decl);
}
+/* Mangle referenced decls. */
+static tree
+mangle_referenced_decls (tree *tp, int *walk_subtrees, void *)
+{
+ if (! EXPR_P (*tp) && ! CONSTANT_CLASS_P (*tp))
+ *walk_subtrees = 0;
+
+ if (VAR_OR_FUNCTION_DECL_P (*tp))
+ assign_assembler_name_if_needed (*tp);
+
+ return NULL_TREE;
+}
+
/* Attach a DW_AT_const_value attribute to DIE. The value of the
attribute is the const value T. */
@@ -20889,7 +20902,6 @@ tree_add_const_value_attribute (dw_die_r
{
tree init;
tree type = TREE_TYPE (t);
- rtx rtl;
if (!t || !TREE_TYPE (t) || TREE_TYPE (t) == error_mark_node)
return false;
@@ -20910,11 +20922,26 @@ tree_add_const_value_attribute (dw_die_r
return true;
}
}
- /* Generate the RTL even if early_dwarf to force mangling of all refered to
- symbols. */
- rtl = rtl_for_decl_init (init, type);
- if (rtl && !early_dwarf)
- return add_const_value_attribute (die, TYPE_MODE (type), rtl);
+ if (!early_dwarf)
+ {
+ rtx rtl = rtl_for_decl_init (init, type);
+ if (rtl && !early_dwarf)
+ return add_const_value_attribute (die, TYPE_MODE (type), rtl);
+ }
+ else
+ {
+ /* For early_dwarf force mangling of all referenced symbols. */
+ tree initializer = init;
+ STRIP_NOPS (initializer);
+ /* rtl_for_decl_init punts on other aggregates, and complex values. */
+ if (AGGREGATE_TYPE_P (type)
+ || (TREE_CODE (initializer) == VIEW_CONVERT_EXPR
+ && AGGREGATE_TYPE_P (TREE_TYPE (TREE_OPERAND (initializer, 0))))
+ || TREE_CODE (type) == COMPLEX_TYPE)
+ ;
+ else if (initializer_constant_valid_p (initializer, type))
+ walk_tree (&initializer, mangle_referenced_decls, NULL, NULL);
+ }
/* If the host and target are sane, try harder. */
if (CHAR_BIT == 8 && BITS_PER_UNIT == 8
&& initializer_constant_valid_p (init, type))
@@ -0,0 +1,12 @@
+// PR debug/104407
+// { dg-do compile { target c++17 } }
+// { dg-options "-O1 -fcompare-debug" }
+
+struct A { int i; long j; int k : 2; char l; } a;
+
+auto [ aa, bb, cc, dd ] = a;
+
+namespace N
+{
+ auto & [ m, n, o, ppp ] = a;
+}