[PATCH] Add a UnitKind field to DWARFUnit so that we can discriminate the unit's origin section.

[David Blaikie]

On Wed, Sep 10, 2014 at 9:20 AM, Frédéric Riss <friss at apple.com> wrote:

>
> On 10 Sep 2014, at 18:10, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Sep 10, 2014 at 12:45 AM, Frédéric Riss <friss at apple.com> wrote:
>
>>
>> > On 09 Sep 2014, at 23:40, David Blaikie <dblaikie at gmail.com> wrote:
>> >
>> >
>> >
>> > On Tue, Sep 9, 2014 at 5:53 AM, Frederic Riss <friss at apple.com> wrote:
>> > Hi dblaikie, samsonov, echristo, aprantl,
>> >
>> > There is no simple way to get the origin section of a DWARFUnit. It is
>> necessary
>> > to know from which section a Unit has been extracted to be able to
>> resolve
>> > cross-unit references (to find the right list to search fo the reference
>> > target).
>> >
>> > Would it make more sense to give it the actual section (or some kind of
>> alias/handle to it) rather than a flag? That way it'd work for fission
>> (split-dwarf) as well, and type units that may be in comdat sections?
>>
>> Although this discriminates on origin section, the way it’s used is just
>> to lookup the right Unit list in the context.
>
>
> OK - then perhaps it should just be a pointer/reference/whatever to the
> relevant unit list? Or do you need this for other purposes as well?
>
>
> Why not, but what type would you make that list then as it needs to be the
> same across all Units? Shall I change the
> SmallVector<std::unique_ptr<DWARF(Compile|Type)Unit> > to
> SmallVector<std::unique_ptr<DWARFUnit> > and add casts in all current
> users? Would that look better?
>


Nope, that wouldn't be much use. So the commonality is the result of
 "getUnitForOffset" - that's generic (DWARFUnit*), though covariant if we
cared.

You could do this without faux-RTTI as follows:

* Add virtual DWARFUnit *DWARFUnit::getSiblingUnitForOffset(uint32_t
Offset) = 0
* add a CRTP between DWARFUnit and the derived classes, that would contain
the type-specific SmallVector<std::unique_ptr<Derived>> reference (Or
pointer, etc) - derived classes are constructed with a reference to that,
pass it to their CRTP base. The CRTP base would implement
getSiblingUnitForOffset.

Does that make sense? (at least is my explanation understandable - not
necessarily a good idea, but making sure I've communicated it clearly)

Good/bad/indifferent?


>
> I have no other current use for the UnitKind than the one bellow.
>
> Fred
>
>
>
>> It’s not the actual section that matters (the unit already contains a
>> StringRef pointing to the section data). The next patch introduces:
>>
>> DWARFUnit *DWARFContext::getUnitForOffset(const DWARFUnit *Unit, uint32_t
>> Offset) {
>>    switch (Unit->getKind()) {
>>    case DWARFUnit::UK_Compile:
>>      return getCompileUnitForOffset(Offset);
>>    case DWARFUnit::UK_DWOCompile:
>>      return getDWOCompileUnitForOffset(Offset);
>>    case DWARFUnit::UK_Type:
>>      return getTypeUnitForOffset(Offset);
>>    case DWARFUnit::UK_DWOType:
>>      return getDWOTypeUnitForOffset(Offset);
>>    }
>>    llvm_unreachable("Unkown unit type in getUnitForOffset.");
>>  }
>>
>> I’m open to any other way to achieve that. And this handles LTO and
>> fission uses (or I can’t see what issues you’re thinking about).
>>
>> Fred
>>
>> >
>> > This patch adds a field and an accessor to the DWARFUnit that indicates
>> where
>> > it originated from. This information will be used in a subsequent patch
>> to
>> > implement a generic version of
>> DWARFContext::getCompilationUnitForOffset()
>> > that will work with arbitrary unit types.
>> >
>> > Note that this is nearly implemented like the start of LLVM-style RTTI
>> for
>> > the DWARFUnit, but it seems that we don't need separate classes to
>> represent
>> > the DWO variants of Units as only their origin changes. Thus we only
>> > introduce this discriminant field.
>> >
>> > http://reviews.llvm.org/D5264
>> >
>> > Files:
>> >   lib/DebugInfo/DWARFCompileUnit.h
>> >   lib/DebugInfo/DWARFContext.cpp
>> >   lib/DebugInfo/DWARFTypeUnit.h
>> >   lib/DebugInfo/DWARFUnit.cpp
>> >   lib/DebugInfo/DWARFUnit.h
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140910/4f3bf33b/attachment.html>