r247432 - Module Debugging: Emit forward declarations for types that are defined in
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 11 12:58:28 PDT 2015
On Fri, Sep 11, 2015 at 11:55 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Sep 11, 2015, at 11:21 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Sep 11, 2015 at 10:23 AM, Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: adrian
>> Date: Fri Sep 11 12:23:08 2015
>> New Revision: 247432
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=247432&view=rev
>> Log:
>> Module Debugging: Emit forward declarations for types that are defined in
>> clang modules, if -dwarf-ext-refs (DebugTypesExtRefs) is specified.
>>
>> This reimplements r247369 in about a third of the amount of code.
>> Thanks to David Blaikie pointing this out in post-commit review!
>>
>> 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=247432&r1=247431&r2=247432&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Sep 11 12:23:08 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);
>>
>
> Might've been nice to separate the module-parenting part of the change,
> smaller patches (easier to revert, review, etc), the usual stuff.
>
>
>> }
>>
>> 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())
>> @@ -1478,8 +1483,19 @@ static bool hasExplicitMemberDefinition(
>> }
>>
>> static bool shouldOmitDefinition(CodeGenOptions::DebugInfoKind DebugKind,
>> + bool DebugTypeExtRefs,
>> const RecordDecl *RD,
>> const LangOptions &LangOpts) {
>> + // Does the type exist in an imported clang module?
>> + if (DebugTypeExtRefs && RD->isFromASTFile()) {
>> + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
>> + if (CTSD->isExplicitInstantiationOrSpecialization())
>> + // We may not assume that this type made it into the module.
>>
>
> I guess you mean "we may assume this type made it into the module"? (the
> "not" seems erroneous - but perhaps I'm misparsing the sentence?)
>
>
> Yes, I inverted the condition without inverting the comment.
>
>
>
>> + return true;
>>
>
> Does this case ^ actually do anything? (aren't all these instantiations
> going to be definitions anyway? so if you removed that code the below would
> return true anyway, wouldn't it?)
>
>
> Good catch! I removed the condition.
>
>
>
>> + if (RD->getDefinition())
>> + return true;
>> + }
>> +
>> if (DebugKind > CodeGenOptions::LimitedDebugInfo)
>> return false;
>>
>> @@ -1513,7 +1529,8 @@ static bool shouldOmitDefinition(CodeGen
>> llvm::DIType *CGDebugInfo::CreateType(const RecordType *Ty) {
>> RecordDecl *RD = Ty->getDecl();
>> llvm::DIType *T =
>> cast_or_null<llvm::DIType>(getTypeOrNull(QualType(Ty, 0)));
>> - if (T || shouldOmitDefinition(DebugKind, RD, CGM.getLangOpts())) {
>> + if (T || shouldOmitDefinition(DebugKind, DebugTypeExtRefs, RD,
>> + CGM.getLangOpts())) {
>> if (!T)
>> T = getOrCreateRecordFwdDecl(Ty, getDeclContextDescriptor(RD));
>> return T;
>> @@ -1616,6 +1633,12 @@ llvm::DIType *CGDebugInfo::CreateType(co
>> if (!ID)
>> return nullptr;
>>
>> + // Return a forward declaration if this type was imported from a clang
>> module.
>>
>
> What's this extra code needed for? Isn't this what shouldOmitDefinition
> already does? It causes calls to getOrCreateRecordFwdDecl and so we should
> never get into the CreateTypeDefinition function for the type.
>
>
> An ObjCInterfaceType is not a RecordDecl, so we have to replicate this
> here.
>
>
>
>> + if (DebugTypeExtRefs && ID->isFromASTFile() && ID->getDefinition())
>> + return DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
>> + ID->getName(),
>> + getDeclContextDescriptor(ID),
>> Unit, 0);
>> +
>> // Get overall information about the record type for the debug info.
>> llvm::DIFile *DefUnit = getOrCreateFile(ID->getLocation());
>> unsigned Line = getLineNumber(ID->getLocation());
>> @@ -1669,9 +1692,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));
>>
>
> This is just reformatting, yes? (stared at it for a while & couldn't spot
> the difference)
>
>
> Sorry, yes.
>
>
>
>> DIB.finalize();
>> ModRef.reset(M);
>> return M;
>> @@ -1930,6 +1953,7 @@ llvm::DIType *CGDebugInfo::CreateType(co
>>
>> llvm::DIType *CGDebugInfo::CreateEnumType(const EnumType *Ty) {
>> const EnumDecl *ED = Ty->getDecl();
>> +
>> uint64_t Size = 0;
>> uint64_t Align = 0;
>> if (!ED->getTypeForDecl()->isIncompleteType()) {
>> @@ -1939,9 +1963,12 @@ llvm::DIType *CGDebugInfo::CreateEnumTyp
>>
>> SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);
>>
>> + bool isImportedFromModule =
>> + DebugTypeExtRefs && ED->isFromASTFile() && ED->getDefinition();
>> +
>>
>
> Again, seems like code that shouldn't be here - shouldOmitDefinition
> returning true should mean that getOrCreateRecordFwdDecl is called and we
> never reach into the concrete CreateXXX functions.
>
>
> Again, an EnumDecl is not a RecordDecl (
> http://clang.llvm.org/doxygen/classclang_1_1TagDecl.html). Am I missing
> something obvious here?
>
Nope, don't think you're missing anything - thanks for the
pointer/explanation.
Reckon there's anything we can do to unify these bits of code (perhaps over
all TagTypes)? (eg: there's a fair bit of overlap between the declaration
portions of getOrCreateRecordFwdDecl and CreateEnumType. CreateType(const
ObjCInterfaceType*, DIFile*) has some overlap, but it's a bit trickier -
checking for implementation as well as definition, etc - dunno.
>
>
>
>> // If this is just a forward declaration, construct an appropriately
>> // marked node and just return it.
>> - if (!ED->getDefinition()) {
>> + if (isImportedFromModule || !ED->getDefinition()) {
>> llvm::DIScope *EDContext = getDeclContextDescriptor(ED);
>> llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation());
>> unsigned Line = getLineNumber(ED->getLocation());
>> @@ -2081,9 +2108,8 @@ llvm::DIType *CGDebugInfo::getOrCreateTy
>> if (auto *T = getTypeOrNull(Ty))
>> return T;
>>
>> - // Otherwise create the type.
>> llvm::DIType *Res = CreateTypeNode(Ty, Unit);
>> - void *TyPtr = Ty.getAsOpaquePtr();
>> + void* TyPtr = Ty.getAsOpaquePtr();
>>
>> // And update the type cache.
>> TypeCache[TyPtr].reset(Res);
>> @@ -2115,6 +2141,19 @@ ObjCInterfaceDecl *CGDebugInfo::getObjCI
>> }
>> }
>>
>> +llvm::DIModule *CGDebugInfo::getParentModuleOrNull(const Decl *D) {
>> + if (!DebugTypeExtRefs || !D || !D->isFromASTFile())
>>
>
> Is a null Decl valid here? What's that for?
>
>
> Removed.
>
>
>
>> + 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::CreateTypeNode(QualType Ty, llvm::DIFile
>> *Unit) {
>> // Handle qualifiers, which recursively handles what they refer to.
>> if (Ty.hasLocalQualifiers())
>> @@ -2325,8 +2364,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 +2415,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);
>>
>
> I'm not quite seeing why this (& the other two/three cases) aren't using
> the getDeclContextDescriptor helper you added? Could you explain it to me?
>
>
> They are either manually picking a DeclContext different than the one
> returned by Decl->GetDeclContext(), or they are using the Decl as its own
> DeclContext.
>
>
>
>> }
>>
>> llvm::DISubprogram *
>> @@ -3299,7 +3342,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=247432&r1=247431&r2=247432&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Fri Sep 11 12:23:08 2015
>> @@ -397,6 +397,9 @@ private:
>> llvm::DIModule *
>> getOrCreateModuleRef(ExternalASTSource::ASTSourceDescriptor Mod);
>>
>> + /// DebugTypeExtRefs: If \p D originated in a clang module, return it.
>> + llvm::DIModule *getParentModuleOrNull(const Decl *D);
>> +
>> /// 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=247432&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/ExtDebugInfo.cpp (added)
>> +++ cfe/trunk/test/Modules/ExtDebugInfo.cpp Fri Sep 11 12:23:08 2015
>> @@ -0,0 +1,69 @@
>> +// 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: ![[NS:.*]] = !DINamespace(name: "DebugCXX", scope:
>> ![[MOD:[0-9]+]],
>> +// CHECK: ![[MOD]] = !DIModule(scope: null, name: {{.*}}DebugCXX
>> +
>> +// 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:[0-9]+]]
>> +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, scope: ![[NS]],
>> +// CHECK-SAME: line: 16
>> +
>> +// 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=247432&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/ExtDebugInfo.m (added)
>> +++ cfe/trunk/test/Modules/ExtDebugInfo.m Fri Sep 11 12:23:08 2015
>> @@ -0,0 +1,29 @@
>> +// RUN: rm -rf %t
>> +// Test that only forward declarations are emitted for types dfined in
>> modules.
>>
>
> Typo "dfined".
>
> I don't see where/how this test is testing for forward declarations - it
> doesn't seem to test for any declarations or definitions. (they could be
> emitted as either, or not emitted at all?)
>
>
> I made it stricter now.
>
> thanks!
> adrian
>
>
>
>> +
>> +// 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
>> 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/356c711f/attachment-0001.html>
More information about the cfe-commits
mailing list