[llvm] r204107 - DwarfDebug: Only unique retained types instead of all types.

[WenHan Gu (谷汶翰)]

Hi Adrian,

After this commit, there's sometimes assert fail on large bitcodes on my
environment.
According to my result, two types seems the same, and I take out the assert
fail can still build and run.
Is any possible that the contained MDNode is not the same (since it is a
pointer comparison indeed) but this is reasonable?
Thanks!


Below is the detail information:

I try to dump, and it shows:  (The contents are the same)

Ty:
[ DW_TAG_class_type ] [Vector2<int>] [line 12, size 64, align 32, offset 0]
[def] [from ]
resolve(Ty.getRef()):
[ DW_TAG_class_type ] [Vector2<int>] [line 12, size 64, align 32, offset 0]
[def] [from ]

and I dump the MDNode

!833 = metadata !{i32 786434, metadata !834, metadata !59, metadata
!"list<Vector2<int>, std::allocator<Vector2<int> > >", i32 438, i64 128,
i64 64, i32 0, i32 0, null, metadata !835, i32 0, null, metadata !970,
metadata !"_ZTSSt4listI7Vector2IiESaIS1_EE"} ; [ DW_TAG_class_type ]
[list<Vector2<int>, std::allocator<Vector2<int> > >] [line 438, size 128,
align 64, offset 0] [def] [from ]

_ZTSSt4listI7Vector2IiESaIS1_EE is "typeinfo name of ..."

In other cases, the error is on the "typeinfo name of  ..." either.



2014-03-25 5:39 GMT+08:00 Adrian Prantl <aprantl at apple.com>:

>
> On Mar 24, 2014, at 2:08 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> > On Mon, Mar 17, 2014 at 7:53 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> On Mon, Mar 17, 2014 at 7:35 PM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >>> Author: adrian
> >>> Date: Mon Mar 17 21:35:03 2014
> >>> New Revision: 204107
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=204107&view=rev
> >>> Log:
> >>> DwarfDebug: Only unique retained types instead of all types.
> >>> This is a follow-up to r203983 based on feedback from dblaikie and
> mren (Thanks!)
> >>> No functionality change.
> >>
> >> Thanks!
> >>
> >>>
> >>> Modified:
> >>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> >>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> >>>
> >>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=204107&r1=204106&r2=204107&view=diff
> >>>
> ==============================================================================
> >>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> >>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Mar 17
> 21:35:03 2014
> >>> @@ -846,8 +846,13 @@ void DwarfDebug::beginModule() {
> >>>     for (unsigned i = 0, e = EnumTypes.getNumElements(); i != e; ++i)
> >>>       CU->getOrCreateTypeDIE(EnumTypes.getElement(i));
> >>>     DIArray RetainedTypes = CUNode.getRetainedTypes();
> >>> -    for (unsigned i = 0, e = RetainedTypes.getNumElements(); i != e;
> ++i)
> >>> -      CU->getOrCreateTypeDIE(RetainedTypes.getElement(i));
> >>> +    for (unsigned i = 0, e = RetainedTypes.getNumElements(); i != e;
> ++i) {
> >>> +      DIType Ty(RetainedTypes.getElement(i));
> >>> +      // The retained types array by design contains pointers to
> >>> +      // MDNodes rather than DIRefs. Unique them here.
> >>> +      DIType UniqueTy(resolve(Ty.getRef()));
> >>> +      CU->getOrCreateTypeDIE(UniqueTy);
> >>
> >> One thought I've had here: should we, rather than resolving the
> >> references and emitting them, avoid emitting the type if the resolved
> >> version isn't from this compilation unit?
> >>
> >> I'm not sure there's a good reason here, but I'm just tossing it
> >> around in my head - I wonder if we could get into confusing situations
> >> (with types as you've described in other recent threads - where two
> >> ODR-correct definitions, written in different files, may end up
> >> emitted under the "wrong" compilation unit). I'm not sure if this
> >> would actually be bad - but, for example, with my line table work it
> >> might be a tiny bit more verbose to emit a type from the wrong
> >> compilation unit since it wouldn't take advantage of the comp_dir as
> >> much (if the type was actually in a header in that directory) or might
> >> introduce an extra file name in the line table that would  otherwise
> >> have been avoided.
> >>
> >> This is ultra low priority - just sanity checking to see if maybe
> >> there are issues with emitting the type from the wrong unit.
> >>
> >
> > I think you're correct here. The way the code is written we're adding
> > retained types as we go over the compile unit. We should probably do a
> > check if the current compile unit is the same as the compile unit for
> > the type that we're about to emit since the type merging would have
> > put it in doubt which compile unit a retained type actually belonged
> > to.
> >
> >>> +    }
> >>>     // Emit imported_modules last so that the relevant context is
> already
> >>>     // available.
> >>>     for (unsigned i = 0, e = ImportedEntities.getNumElements(); i !=
> e; ++i)
> >>>
> >>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=204107&r1=204106&r2=204107&view=diff
> >>>
> ==============================================================================
> >>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> >>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Mon Mar 17
> 21:35:03 2014
> >>> @@ -971,6 +971,8 @@ DIE *DwarfUnit::getOrCreateTypeDIE(const
> >>>
> >>>   DIType Ty(TyNode);
> >>>   assert(Ty.isType());
> >>> +  assert(*&Ty == resolve(Ty.getRef()) &&
> >>
> >> Why's the "*&Ty" required? That seems a bit strange/subtle...
> >>
> >
> > Agreed. Adrian?
>
> Absolutely. Removed in r204673.
>
> -- adrian
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Best Regards,
WenHan Gu (Nowar)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140327/f25bde22/attachment.html>