r247369 - Module Debugging: Emit forward declarations for types that are defined in
Adrian Prantl via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 11 10:25:34 PDT 2015
> On Sep 11, 2015, at 8:27 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Sep 11, 2015 at 8:18 AM, Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
>
>> On Sep 10, 2015, at 6:56 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>>
>> On Thu, Sep 10, 2015 at 6:40 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>> On Thu, Sep 10, 2015 at 6:03 PM, Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>> Author: adrian
>> Date: Thu Sep 10 20:03:26 2015
>> New Revision: 247369
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=247369&view=rev <http://llvm.org/viewvc/llvm-project?rev=247369&view=rev>
>> Log:
>> Module Debugging: Emit forward declarations for types that are defined in
>> clang modules, if -dwarf-ext-refs (DebugTypesExtRefs) is specified.
>>
>> This change seems to have a lot more code in it than I was expecting...
>>
>> I was rather expecting something a lot like the flimit-debug-info support. Specifically, I would've expected one more conditional added to CGDebugInfo::shouldOmitDefinition.
>>
>> Why the extra complexity?
>>
>> I guess part of it is to be able to omit definitions of things other than record types - is there much value in that? (especially typedefs - it seems like a typedef is too small to benefit from a declaration (even if we could emit one)?)
>
> The typedef itself is not interesting, but it can be used to anchor template instantiations like std::string. The contract between the debug info in the module and the debug info referencing the module is that all explicit template specializations
>
> This I get ^ (though does that case need any special handling, or will the isFromASTFile return true for the location of the explicit specialization declaration/definition?)
>
> and template instantiations referenced used in a typedef can be expected to exist in the module.
>
> This I do not get & would imagine it could cause substantial size increase...
>
> Might be worth splitting out that change to look at it more carefully. I'd love to see numbers here.
>> Also, we could possibly solve both the problem of "don't emit definitions for module things when compiling the main source file" and "don't emit definitions for module things defined in other modules" with the same tool. If there's a way to say "is this in a foreign AST file" then testing for that in CGDebugInfo::shouldOmitDefinition would solve both problems, I think (conditionalized on the appropriate flags, either dwarf-ext-refs or "I'm building a module here”).
>
> I believe that this is a good point. Really, getExtRefOrNull is a separate function mostly for evolutionary reasons — it used to do different things like mangling names of ObjC types etc. I will see if I can merge the existing functionality into shouldOmitDefinition().
>
> From a review perspective it might be easier to revert this & commit a new patch - I haven't reviewed all these changes in detail because they just seemed more complex than necessary, but if you're going to evolve it from here I'll need to dig into the current implementation/be more careful about checking that future changes remove the complexity along with whatever they add.
>
I just did this in r24743[12].
-- adrian
> - Dave
>
>
> -- adrian
>
>>
>>
>>
>> Added:
>> cfe/trunk/test/Modules/ExtDebugInfo.cpp
>> cfe/trunk/test/Modules/ExtDebugInfo.m
>> Modified:
>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> cfe/trunk/lib/CodeGen/CGDebugInfo.h
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=247369&r1=247368&r2=247369&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=247369&r1=247368&r2=247369&view=diff>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep 10 20:03:26 2015
>> @@ -148,7 +148,9 @@ void CGDebugInfo::setLocation(SourceLoca
>> }
>>
>> llvm::DIScope *CGDebugInfo::getDeclContextDescriptor(const Decl *D) {
>> - return getContextDescriptor(cast<Decl>(D->getDeclContext()), TheCU);
>> + llvm::DIScope *Mod = getParentModuleOrNull(D);
>> + return getContextDescriptor(cast<Decl>(D->getDeclContext()),
>> + Mod ? Mod : TheCU);
>> }
>>
>> llvm::DIScope *CGDebugInfo::getContextDescriptor(const Decl *Context,
>> @@ -1448,6 +1450,9 @@ void CGDebugInfo::completeRequiredType(c
>> if (CXXDecl->isDynamicClass())
>> return;
>>
>> + if (DebugTypeExtRefs && RD->isFromASTFile())
>> + return;
>> +
>> QualType Ty = CGM.getContext().getRecordType(RD);
>> llvm::DIType *T = getTypeOrNull(Ty);
>> if (T && T->isForwardDecl())
>> @@ -1669,9 +1674,9 @@ CGDebugInfo::getOrCreateModuleRef(Extern
>> TheCU->getSourceLanguage(), internString(Mod.ModuleName),
>> internString(Mod.Path), TheCU->getProducer(), true, StringRef(), 0,
>> internString(Mod.ASTFile), llvm::DIBuilder::FullDebug, Mod.Signature);
>> - llvm::DIModule *M =
>> - DIB.createModule(CU, Mod.ModuleName, ConfigMacros, internString(Mod.Path),
>> - internString(CGM.getHeaderSearchOpts().Sysroot));
>> + llvm::DIModule *M = DIB.createModule(
>> + CU, Mod.ModuleName, ConfigMacros, internString(Mod.Path),
>> + internString(CGM.getHeaderSearchOpts().Sysroot));
>> DIB.finalize();
>> ModRef.reset(M);
>> return M;
>> @@ -2081,9 +2086,16 @@ llvm::DIType *CGDebugInfo::getOrCreateTy
>> if (auto *T = getTypeOrNull(Ty))
>> return T;
>>
>> + llvm::DIType *Res = nullptr;
>> + if (DebugTypeExtRefs)
>> + // Make a forward declaration of an external type.
>> + Res = getTypeExtRefOrNull(Ty, Unit);
>> +
>> // Otherwise create the type.
>> - llvm::DIType *Res = CreateTypeNode(Ty, Unit);
>> - void *TyPtr = Ty.getAsOpaquePtr();
>> + if (!Res)
>> + Res = CreateTypeNode(Ty, Unit);
>> +
>> + void* TyPtr = Ty.getAsOpaquePtr();
>>
>> // And update the type cache.
>> TypeCache[TyPtr].reset(Res);
>> @@ -2115,6 +2127,123 @@ ObjCInterfaceDecl *CGDebugInfo::getObjCI
>> }
>> }
>>
>> +llvm::DIModule *CGDebugInfo::getParentModuleOrNull(const Decl *D) {
>> + if (!DebugTypeExtRefs || !D || !D->isFromASTFile())
>> + return nullptr;
>> +
>> + llvm::DIModule *ModuleRef = nullptr;
>> + auto *Reader = CGM.getContext().getExternalSource();
>> + auto Idx = D->getOwningModuleID();
>> + auto Info = Reader->getSourceDescriptor(Idx);
>> + if (Info)
>> + ModuleRef = getOrCreateModuleRef(*Info);
>> + return ModuleRef;
>> +}
>> +
>> +llvm::DIType *CGDebugInfo::getTypeExtRefOrNull(QualType Ty, llvm::DIFile *F,
>> + bool Anchored) {
>> + assert(DebugTypeExtRefs && "module debugging only");
>> + Decl *TyDecl = nullptr;
>> + StringRef Name;
>> + SmallString<256> UID;
>> + unsigned Tag = 0;
>> +
>> + // Handle all types that have a declaration.
>> + switch (Ty->getTypeClass()) {
>> + case Type::Typedef: {
>> + TyDecl = cast<TypedefType>(Ty)->getDecl();
>> + if (!TyDecl->isFromASTFile())
>> + return nullptr;
>> +
>> + // A typedef will anchor a type in the module.
>> + if (auto *TD = dyn_cast<TypedefDecl>(TyDecl)) {
>> + // This is a working around the fact that LLVM does not allow
>> + // typedefs to be forward declarations.
>> + QualType Ty = TD->getUnderlyingType();
>> + Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
>> + if (auto *AnchoredTy = getTypeExtRefOrNull(Ty, F, /*Anchored=*/true)) {
>> + TypeCache[Ty.getAsOpaquePtr()].reset(AnchoredTy);
>> + SourceLocation Loc = TD->getLocation();
>> + return DBuilder.createTypedef(AnchoredTy, TD->getName(),
>> + getOrCreateFile(Loc), getLineNumber(Loc),
>> + getDeclContextDescriptor(TD));
>> + }
>> + }
>> + break;
>> + }
>> +
>> + case Type::Record: {
>> + TyDecl = cast<RecordType>(Ty)->getDecl();
>> + if (!TyDecl->isFromASTFile())
>> + return nullptr;
>> +
>> + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(TyDecl))
>> + if (!CTSD->isExplicitInstantiationOrSpecialization() && !Anchored)
>> + // We may not assume that this type made it into the module.
>> + return nullptr;
>> + // C++ classes and template instantiations.
>> + if (auto *RD = dyn_cast<CXXRecordDecl>(TyDecl)) {
>> + if (!RD->getDefinition())
>> + return nullptr;
>> + Tag = getTagForRecord(RD);
>> + UID =
>> + getUniqueTagTypeName(cast<TagType>(RD->getTypeForDecl()), CGM, TheCU);
>> + Name = getClassName(RD);
>> + } else if (auto *RD = dyn_cast<RecordDecl>(TyDecl)) {
>> + // C-style structs.
>> + if (!RD->getDefinition())
>> + return nullptr;
>> + Tag = getTagForRecord(RD);
>> + Name = getClassName(RD);
>> + }
>> + break;
>> + }
>> +
>> + case Type::Enum: {
>> + TyDecl = cast<EnumType>(Ty)->getDecl();
>> + if (!TyDecl->isFromASTFile())
>> + return nullptr;
>> +
>> + if (auto *ED = dyn_cast<EnumDecl>(TyDecl)) {
>> + if (!ED->getDefinition())
>> + return nullptr;
>> + Tag = llvm::dwarf::DW_TAG_enumeration_type;
>> + if ((TheCU->getSourceLanguage() == llvm::dwarf::DW_LANG_C_plus_plus) ||
>> + (TheCU->getSourceLanguage() == llvm::dwarf::DW_LANG_ObjC_plus_plus)) {
>> + UID = getUniqueTagTypeName(cast<TagType>(ED->getTypeForDecl()), CGM,
>> + TheCU);
>> + Name = ED->getName();
>> + }
>> + }
>> + break;
>> + }
>> +
>> + case Type::ObjCInterface: {
>> + TyDecl = cast<ObjCInterfaceType>(Ty)->getDecl();
>> + if (!TyDecl->isFromASTFile())
>> + return nullptr;
>> +
>> + if (auto *ID = dyn_cast<ObjCInterfaceDecl>(TyDecl)) {
>> + if (!ID->getDefinition())
>> + return nullptr;
>> + Tag = llvm::dwarf::DW_TAG_structure_type;
>> + Name = ID->getName();
>> + }
>> + break;
>> + }
>> +
>> + default:
>> + return nullptr;
>> + }
>> +
>> + if (Tag && !Name.empty()) {
>> + assert(TyDecl);
>> + auto *Ctx = getDeclContextDescriptor(TyDecl);
>> + return DBuilder.createForwardDecl(Tag, Name, Ctx, F, 0, 0, 0, 0, UID);
>> + } else
>> + return nullptr;
>> +}
>> +
>> llvm::DIType *CGDebugInfo::CreateTypeNode(QualType Ty, llvm::DIFile *Unit) {
>> // Handle qualifiers, which recursively handles what they refer to.
>> if (Ty.hasLocalQualifiers())
>> @@ -2325,8 +2454,10 @@ void CGDebugInfo::collectFunctionDeclPro
>> dyn_cast_or_null<NamespaceDecl>(FD->getDeclContext()))
>> FDContext = getOrCreateNameSpace(NSDecl);
>> else if (const RecordDecl *RDecl =
>> - dyn_cast_or_null<RecordDecl>(FD->getDeclContext()))
>> - FDContext = getContextDescriptor(RDecl, TheCU);
>> + dyn_cast_or_null<RecordDecl>(FD->getDeclContext())) {
>> + llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
>> + FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
>> + }
>> // Collect template parameters.
>> TParamsArray = CollectFunctionTemplateParams(FD, Unit);
>> }
>> @@ -2374,7 +2505,9 @@ void CGDebugInfo::collectVarDeclProps(co
>> // outside the class by putting it in the global scope.
>> if (DC->isRecord())
>> DC = CGM.getContext().getTranslationUnitDecl();
>> - VDContext = getContextDescriptor(cast<Decl>(DC), TheCU);
>> +
>> + llvm::DIScope *Mod = getParentModuleOrNull(VD);
>> + VDContext = getContextDescriptor(cast<Decl>(DC), Mod ? Mod : TheCU);
>> }
>>
>> llvm::DISubprogram *
>> @@ -3299,7 +3432,8 @@ void CGDebugInfo::EmitGlobalVariable(con
>> llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
>> if (!LexicalBlockStack.empty())
>> return LexicalBlockStack.back();
>> - return getContextDescriptor(D, TheCU);
>> + llvm::DIScope *Mod = getParentModuleOrNull(D);
>> + return getContextDescriptor(D, Mod ? Mod : TheCU);
>> }
>>
>> void CGDebugInfo::EmitUsingDirective(const UsingDirectiveDecl &UD) {
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=247369&r1=247368&r2=247369&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=247369&r1=247368&r2=247369&view=diff>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Sep 10 20:03:26 2015
>> @@ -397,6 +397,15 @@ private:
>> llvm::DIModule *
>> getOrCreateModuleRef(ExternalASTSource::ASTSourceDescriptor Mod);
>>
>> + /// DebugTypeExtRefs: If \p D originated in a clang module, return it.
>> + llvm::DIModule *getParentModuleOrNull(const Decl *D);
>> +
>> + /// Return a forward declaration of an external type, if this type
>> + /// came from a clang module. If \p Anchored is true, template
>> + /// types will be assumed to have been instantiated in the module.
>> + llvm::DIType *getTypeExtRefOrNull(QualType Ty, llvm::DIFile *F,
>> + bool Anchored = false);
>> +
>> /// Get the type from the cache or create a new partial type if
>> /// necessary.
>> llvm::DICompositeType *getOrCreateLimitedType(const RecordType *Ty,
>>
>> Added: cfe/trunk/test/Modules/ExtDebugInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.cpp?rev=247369&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.cpp?rev=247369&view=auto>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/ExtDebugInfo.cpp (added)
>> +++ cfe/trunk/test/Modules/ExtDebugInfo.cpp Thu Sep 10 20:03:26 2015
>> @@ -0,0 +1,74 @@
>> +// RUN: rm -rf %t
>> +// Test that only forward declarations are emitted for types dfined in modules.
>> +
>> +// Modules:
>> +// RUN: %clang_cc1 -x objective-c++ -std=c++11 -g -dwarf-ext-refs -fmodules \
>> +// RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \
>> +// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll
>> +// RUN: cat %t-mod.ll | FileCheck %s
>> +
>> +// PCH:
>> +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodule-format=obj -emit-pch -I%S/Inputs \
>> +// RUN: -o %t.pch %S/Inputs/DebugCXX.h
>> +// RUN: %clang_cc1 -std=c++11 -g -dwarf-ext-refs -fmodule-format=obj \
>> +// RUN: -include-pch %t.pch %s -emit-llvm -o %t-pch.ll %s
>> +// RUN: cat %t-pch.ll | FileCheck %s
>> +
>> +#ifdef MODULES
>> + at import DebugCXX;
>> +#endif
>> +
>> +using DebugCXX::Struct;
>> +
>> +Struct s;
>> +DebugCXX::Enum e;
>> +DebugCXX::Template<long> implicitTemplate;
>> +DebugCXX::Template<int> explicitTemplate;
>> +DebugCXX::FloatInstatiation typedefTemplate;
>> +int Struct::static_member = -1;
>> +enum {
>> + e3 = -1
>> +} conflicting_uid = e3;
>> +auto anon_enum = DebugCXX::e2;
>> +char _anchor = anon_enum + conflicting_uid;
>> +
>> +// CHECK: ![[ANON_ENUM:[0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type
>> +// CHECK-SAME: scope: ![[MOD:[0-9]+]],
>> +// CHECK-SAME: {{.*}}line: 16, {{.*}}, elements: ![[EE:[0-9]+]])
>> +
>> +// CHECK: ![[NS:.*]] = !DINamespace(name: "DebugCXX", scope: ![[MOD:[0-9]+]],
>> +// CHECK: ![[MOD]] = !DIModule(scope: null, name: {{.*}}DebugCXX
>> +
>> +// CHECK: ![[EE]] = !{![[E2:[0-9]+]]}
>> +// CHECK: ![[E2]] = !DIEnumerator(name: "e2", value: 50)
>> +
>> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct",
>> +// CHECK-SAME: scope: ![[NS]],
>> +// CHECK-SAME: flags: DIFlagFwdDecl,
>> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
>> +
>> +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Enum",
>> +// CHECK-SAME: scope: ![[NS]],
>> +// CHECK-SAME: flags: DIFlagFwdDecl,
>> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE")
>> +
>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
>> +
>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
>> +// CHECK-SAME: name: "Template<int, DebugCXX::traits<int> >",
>> +// CHECK-SAME: scope: ![[NS]],
>> +// CHECK-SAME: flags: DIFlagFwdDecl,
>> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")
>> +
>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
>> +// CHECK-SAME: name: "Template<float, DebugCXX::traits<float> >",
>> +// CHECK-SAME: scope: ![[NS]],
>> +// CHECK-SAME: flags: DIFlagFwdDecl,
>> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
>> +
>> +// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "static_member",
>> +// CHECK-SAME: scope: !"_ZTSN8DebugCXX6StructE"
>> +
>> +// CHECK: !DIGlobalVariable(name: "anon_enum", {{.*}}, type: ![[ANON_ENUM]]
>> +
>> +// CHECK: !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !0, entity: !"_ZTSN8DebugCXX6StructE", line: 21)
>>
>> Added: cfe/trunk/test/Modules/ExtDebugInfo.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.m?rev=247369&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.m?rev=247369&view=auto>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/ExtDebugInfo.m (added)
>> +++ cfe/trunk/test/Modules/ExtDebugInfo.m Thu Sep 10 20:03:26 2015
>> @@ -0,0 +1,29 @@
>> +// RUN: rm -rf %t
>> +// Test that only forward declarations are emitted for types dfined in modules.
>> +
>> +// Modules:
>> +// RUN: %clang_cc1 -x objective-c -g -dwarf-ext-refs -fmodules \
>> +// RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \
>> +// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll
>> +// RUN: cat %t-mod.ll | FileCheck %s
>> +
>> +// PCH:
>> +// RUN: %clang_cc1 -x objective-c -fmodule-format=obj -emit-pch -I%S/Inputs \
>> +// RUN: -o %t.pch %S/Inputs/DebugObjC.h
>> +// RUN: %clang_cc1 -x objective-c -g -dwarf-ext-refs -fmodule-format=obj \
>> +// RUN: -include-pch %t.pch %s -emit-llvm -o %t-pch.ll %s
>> +// RUN: cat %t-pch.ll | FileCheck %s
>> +
>> +#ifdef MODULES
>> + at import DebugObjC;
>> +#endif
>> +
>> +int foo(ObjCClass *c) {
>> + [c instanceMethodWithInt: 0];
>> + return [c property];
>> +}
>> +
>> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type,
>> +// CHECK-SAME: scope: ![[MOD:[0-9]+]],
>> +// CHECK-SAME: flags: DIFlagFwdDecl)
>> +// CHECK: ![[MOD]] = !DIModule(scope: null, name: {{.*}}DebugObjC
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <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/20150911/c72f2c9b/attachment-0001.html>
More information about the cfe-commits
mailing list