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 11:21:40 PDT 2015
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?)
> + 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?)
> + 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.
> + 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)
> 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.
> // 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?
> + 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?
> }
>
> 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?)
> +
> +// 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/24af4f32/attachment-0001.html>
More information about the cfe-commits
mailing list