[PATCH] D29670: [DebugInfo] Emit address space with DW_AT_address_class attribute for pointer and reference types

[David Blaikie via llvm-commits]

On Thu, Feb 16, 2017 at 1:53 PM Konstantin Zhuravlyov via Phabricator <
reviews at reviews.llvm.org> wrote:

> kzhuravl added inline comments.
>
>
> ================
> Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1117
> +    Optional<unsigned> AddressSpace = None;
> +    if (Record.size() > 12 && Record[12] != ~0U)
> +      AddressSpace = Record[12];
> ----------------
> dblaikie wrote:
> > What's the magical ~0U value about?
> I used it as a sentinel value to represent "None" optional dwarf address
> space in the bitcode (we always generate and read address space record, if
> it is ~0U, then we pass None when creating DIDerivedType).
>
> Would it be better to not write address space record if it is None?
>
> We can also use UINT24_MAX + 1 (we would need to define UINT24_MAX). or
> ADDRESS_SPACE_MAX?
>

I'll leave that to Adrian or Duncan, who know more about the bitcode
format/tradeoffs, I think.


>
>
> ================
> Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:847-848
> +  // types as DW_AT_address_class.
> +  if (DTy->getAddressSpace() && (Tag == dwarf::DW_TAG_pointer_type ||
> +                                 Tag == dwarf::DW_TAG_reference_type))
> +    addUInt(Buffer, dwarf::DW_AT_address_class, dwarf::DW_FORM_data4,
> ----------------
> dblaikie wrote:
> > why only on pointer and reference types? Should we just do it on all
> types and don't put an address space on osmething in the IR if it's not
> desired to be in the DWARF?
> According to dwarf specs [v2-v5], DW_AT_address_class only applies to
> pointer or reference types, subroutines or subroutine types.


Then seems reasonable that it shouldn't be present on them in the metadata
- this could be enforced by the validator, or I wouldn't terribly mind if
the IR allowed this to be expressed and emitted (the DWARF spec doesn't
preclude other uses of attributes, I think? I forget how that works)


> I just checked, and DW_AT_address_class is not present in dwarf spec v1.
> So we should only generate DW_AT_address_class for v2+.
>

That seems like something that should be checked/handled here (& tested) in
the backend.


>
>
>
> https://reviews.llvm.org/D29670
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170216/7ab98ee1/attachment.html>