Re: [PATCH] Fix debug info for enumeration types with reverse Scalar_Storage_Order
On Wed, Jan 10, 2024 at 12:53 PM Eric Botcazou wrote:
>
> > Can you elaborate on the DIE order constraint and why it was chosen? That
> > is,
> >
> > + /* The DIE with DW_AT_endianity is placed right after the naked DIE.
> > */ + if (reverse)
> > + {
> > + gcc_assert (type_die);
> > ...
> >
> > and
> >
> > + /* The DIE with DW_AT_endianity is placed right after the naked DIE.
> > */ + if (reverse_type)
> > + {
> > + dw_die_ref after_die
> > + = modified_type_die (type, cv_quals, false, context_die);
> > + gen_type_die (type, context_die, true);
> > + gcc_assert (after_die->die_sib
> > + && get_AT_unsigned (after_die->die_sib,
> > DW_AT_endianity)); + return after_die->die_sib;
> >
> > ?
>
> That's preexisting though, see line 13730 where there is a small blurb.
>
> The crux of the matter is that there is no scalar *_TYPE node with a reverse
> SSO, so there is nothing to equate with for the DIE carrying DW_AT_endianity,
> unlike for type variants (the reverse SSO is on the enclosing aggregate type
> instead but this does not match the way DWARF describes it).
>
> Therefore, in order to avoid building a new DIE with DW_AT_endianity each
> time, the DIE with DW_AT_endianity is placed right after the naked DIE, so
> that the lookup done at line 13730 for reverse SSO is immediate.
Hmm, I see. The patch is OK then.
Thanks,
Richard.
> > Likewise the extra argument to the functions is odd - is that not available
> > on the tree type?
>
> No, for the reason described above, so the extra parameter is preexisting for
> base_type_die, modified_type_die and add_type_attribute.
>
> --
> Eric Botcazou
>
>
Re: [PATCH] Fix debug info for enumeration types with reverse Scalar_Storage_Order
> Can you elaborate on the DIE order constraint and why it was chosen? That
> is,
>
> + /* The DIE with DW_AT_endianity is placed right after the naked DIE.
> */ + if (reverse)
> + {
> + gcc_assert (type_die);
> ...
>
> and
>
> + /* The DIE with DW_AT_endianity is placed right after the naked DIE.
> */ + if (reverse_type)
> + {
> + dw_die_ref after_die
> + = modified_type_die (type, cv_quals, false, context_die);
> + gen_type_die (type, context_die, true);
> + gcc_assert (after_die->die_sib
> + && get_AT_unsigned (after_die->die_sib,
> DW_AT_endianity)); + return after_die->die_sib;
>
> ?
That's preexisting though, see line 13730 where there is a small blurb.
The crux of the matter is that there is no scalar *_TYPE node with a reverse
SSO, so there is nothing to equate with for the DIE carrying DW_AT_endianity,
unlike for type variants (the reverse SSO is on the enclosing aggregate type
instead but this does not match the way DWARF describes it).
Therefore, in order to avoid building a new DIE with DW_AT_endianity each
time, the DIE with DW_AT_endianity is placed right after the naked DIE, so
that the lookup done at line 13730 for reverse SSO is immediate.
> Likewise the extra argument to the functions is odd - is that not available
> on the tree type?
No, for the reason described above, so the extra parameter is preexisting for
base_type_die, modified_type_die and add_type_attribute.
--
Eric Botcazou
Re: [PATCH] Fix debug info for enumeration types with reverse Scalar_Storage_Order
On Tue, Jan 9, 2024 at 9:18 PM Eric Botcazou wrote:
>
> Hi,
>
> this is not really a regression but the patch was written last week and is
> quite straightforward, so hopefully can nevertheless be OK. It implements the
> support of DW_AT_endianity for enumeration types because they are scalar and,
> therefore, reverse Scalar_Storage_Order is supported for them, but only when
> the -gstrict-dwarf switch is not passed because this is an extension.
>
> There is an associated GDB patch to be submitted by Tom to grok the new DWARF.
>
> Tested on x86-64/Linux, OK for the mainline? It may also help the GDB side to
> backport it for the upcoming 13.3 release.
Can you elaborate on the DIE order constraint and why it was chosen? That is,
+ /* The DIE with DW_AT_endianity is placed right after the naked DIE. */
+ if (reverse)
+ {
+ gcc_assert (type_die);
...
and
+ /* The DIE with DW_AT_endianity is placed right after the naked DIE. */
+ if (reverse_type)
+ {
+ dw_die_ref after_die
+ = modified_type_die (type, cv_quals, false, context_die);
+ gen_type_die (type, context_die, true);
+ gcc_assert (after_die->die_sib
+ && get_AT_unsigned (after_die->die_sib, DW_AT_endianity));
+ return after_die->die_sib;
?
Likewise the extra argument to the functions is odd - is that not available
on the tree type?
Richard.
>
> 2024-01-09 Eric Botcazou
>
> * dwarf2out.cc (modified_type_die): Extend the support of reverse
> storage order to enumeration types if -gstrict-dwarf is not passed.
> (gen_enumeration_type_die): Add REVERSE parameter and generate the
> DIE immediately after the existing one if it is true.
> (gen_tagged_type_die): Add REVERSE parameter and pass it in the
> call to gen_enumeration_type_die.
> (gen_type_die_with_usage): Add REVERSE parameter and pass it in the
> first recursive call as well as the call to gen_tagged_type_die.
> (gen_type_die): Add REVERSE parameter and pass it in the call to
> gen_type_die_with_usage.
>
> --
> Eric Botcazou
