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

[Frédéric Riss]

> 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 <mailto:friss at apple.com>> wrote:
> 
> > On 09 Sep 2014, at 23:40, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
> >
> >
> >
> > On Tue, Sep 9, 2014 at 5:53 AM, Frederic Riss <friss at apple.com <mailto: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?

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 <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/c1b2442b/attachment.html>