r247432 - 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 11:55:25 PDT 2015
> 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 <mailto: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 <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 <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?
>
> // 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 <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 <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 <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 <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/fce5dde3/attachment-0001.html>
More information about the cfe-commits
mailing list