r256962 - Module debugging: Defer emitting tag types until their definition

Adrian Prantl via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 8 17:15:42 PST 2016


> 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



More information about the cfe-commits mailing list