r256962 - Module debugging: Defer emitting tag types until their definition
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 8 19:06:20 PST 2016
Thanks!
On Fri, Jan 8, 2016 at 5:15 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
> > On Jan 8, 2016, at 2:18 PM, Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >
> >>
> >> On Jan 8, 2016, at 1:59 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >>
> >>
> >>
> >> On Fri, Jan 8, 2016 at 1:34 PM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >>
> >>> On Jan 8, 2016, at 1:31 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >>>
> >>>
> >>>
> >>> On Wed, Jan 6, 2016 at 11:22 AM, Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >>> Author: adrian
> >>> Date: Wed Jan 6 13:22:19 2016
> >>> New Revision: 256962
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=256962&view=rev
> >>> Log:
> >>> Module debugging: Defer emitting tag types until their definition
> >>> was visited and all decls have been merged.
> >>>
> >>> We only get a single chance to emit the types for virtual classes
> because
> >>> CGDebugInfo::completeRequiredType() categorically doesn't complete
> them.
> >>>
> >>> Not sure I'm following this comment. Could you explain this in other
> words/detail?
> >>>
> >>> If we visit a declaration we shouldn't do anything, right? (no point
> putting declarations in the modules debug info, unless it's needed for
> something else?) & then when we see the definition we'd build the debug
> info for it?
> >>
> >> Yes. Your statement pretty much summarizes this commit :-)
> >>
> >> Then I'm still confused - were we emitting type declarations in modules
> prior to this change?
> >
> > Yes. Dsymutil used to expect every forward declaration in a skeleton CU
> to have a corresponding declaration in the module dwo. I just verified that
> dsymutil does no longer prune forward declarations from skeleton CUs that
> don’t have a definition in the DWO, so everything should be fine now.
> >
> >> Perhaps a test case for a lone declaration (with no follow up
> definition) would be in order, then?
> >
> > Yes, that would make sense. I’ll add one.
>
> Done in r257241.
> -- adrian
> >
> >>
> >> -- adrian
> >>
> >>>
> >>>
> >>> Modified:
> >>> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> >>> cfe/trunk/test/Modules/Inputs/DebugCXX.h
> >>> cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> >>>
> >>> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=256962&r1=256961&r2=256962&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> (original)
> >>> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Wed
> Jan 6 13:22:19 2016
> >>> @@ -59,8 +59,10 @@ class PCHContainerGenerator : public AST
> >>> struct DebugTypeVisitor : public
> RecursiveASTVisitor<DebugTypeVisitor> {
> >>> clang::CodeGen::CGDebugInfo &DI;
> >>> ASTContext &Ctx;
> >>> - DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx)
> >>> - : DI(DI), Ctx(Ctx) {}
> >>> + bool SkipTagDecls;
> >>> + DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx,
> >>> + bool SkipTagDecls)
> >>> + : DI(DI), Ctx(Ctx), SkipTagDecls(SkipTagDecls) {}
> >>>
> >>> /// Determine whether this type can be represented in DWARF.
> >>> static bool CanRepresent(const Type *Ty) {
> >>> @@ -75,6 +77,12 @@ class PCHContainerGenerator : public AST
> >>> }
> >>>
> >>> bool VisitTypeDecl(TypeDecl *D) {
> >>> + // TagDecls may be deferred until after all decls have been
> merged and we
> >>> + // know the complete type. Pure forward declarations will be
> skipped, but
> >>> + // they don't need to be emitted into the module anyway.
> >>> + if (SkipTagDecls && isa<TagDecl>(D))
> >>> + return true;
> >>> +
> >>> QualType QualTy = Ctx.getTypeDeclType(D);
> >>> if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
> >>> DI.getOrCreateStandaloneType(QualTy, D->getLocation());
> >>> @@ -165,7 +173,7 @@ public:
> >>> // Collect debug info for all decls in this group.
> >>> for (auto *I : D)
> >>> if (!I->isFromASTFile()) {
> >>> - DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx);
> >>> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx,
> true);
> >>> DTV.TraverseDecl(I);
> >>> }
> >>> return true;
> >>> @@ -179,6 +187,11 @@ public:
> >>> if (Diags.hasErrorOccurred())
> >>> return;
> >>>
> >>> + if (D->isFromASTFile())
> >>> + return;
> >>> +
> >>> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx, false);
> >>> + DTV.TraverseDecl(D);
> >>> Builder->UpdateCompletedType(D);
> >>> }
> >>>
> >>>
> >>> Modified: cfe/trunk/test/Modules/Inputs/DebugCXX.h
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=256962&r1=256961&r2=256962&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/test/Modules/Inputs/DebugCXX.h (original)
> >>> +++ cfe/trunk/test/Modules/Inputs/DebugCXX.h Wed Jan 6 13:22:19 2016
> >>> @@ -50,3 +50,9 @@ namespace DebugCXX {
> >>> typedef A<void> B;
> >>> void foo(B) {}
> >>> }
> >>> +
> >>> +// Virtual class with a forward declaration.
> >>> +class FwdVirtual;
> >>> +class FwdVirtual {
> >>> + virtual ~FwdVirtual() {}
> >>> +};
> >>>
> >>> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> >>> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=256962&r1=256961&r2=256962&view=diff
> >>>
> ==============================================================================
> >>> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
> >>> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Wed Jan 6 13:22:19 2016
> >>> @@ -7,13 +7,11 @@
> >>> // RUN: rm -rf %t
> >>> // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++
> -std=c++11 -debug-info-kind=limited -fmodules -fmodule-format=obj
> -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I
> %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll
> >>> // RUN: cat %t-mod.ll | FileCheck %s
> >>> -// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
> >>> // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-DWO %s
> >>>
> >>> // PCH:
> >>> // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11
> -emit-pch -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h
> -mllvm -debug-only=pchcontainer &>%t-pch.ll
> >>> // RUN: cat %t-pch.ll | FileCheck %s
> >>> -// RUN: cat %t-pch.ll | FileCheck --check-prefix=CHECK-NEG %s
> >>>
> >>> #ifdef MODULES
> >>> @import DebugCXX;
> >>> @@ -23,22 +21,32 @@
> >>> // CHECK-SAME: isOptimized: false,
> >>> // CHECK-SAME-NOT: splitDebugFilename:
> >>> // CHECK-DWO: dwoId:
> >>> +
> >>> // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Enum"
> >>> // CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE")
> >>> // CHECK: !DINamespace(name: "DebugCXX"
> >>> +
> >>> // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"
> >>> // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
> >>> +
> >>> // CHECK: !DICompositeType(tag: DW_TAG_class_type,
> >>> // CHECK-SAME: name: "Template<int, DebugCXX::traits<int>
> >"
> >>> // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")
> >>> +
> >>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"
> >>> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
> >>> +
> >>> // CHECK: !DICompositeType(tag: DW_TAG_class_type,
> >>> // CHECK-SAME: name: "Template<float,
> DebugCXX::traits<float> >"
> >>> // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
> >>> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"
> >>> -// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
> >>> +
> >>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "FwdVirtual"
> >>> +// CHECK-SAME: elements:
> >>> +// CHECK-SAME: identifier: "_ZTS10FwdVirtual")
> >>> +// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "_vptr$FwdVirtual"
> >>> +
> >>> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name:
> "FloatInstatiation"
> >>> // no mangled name here yet.
> >>> +
> >>> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
> >>> // no mangled name here yet.
> >>> -
> >>> -// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE"
> >>>
> >>>
> >>> _______________________________________________
> >>> cfe-commits mailing list
> >>> cfe-commits at lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>>
> >>
> >>
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160108/77d1d05d/attachment.html>
More information about the cfe-commits
mailing list