On Fri, Sep 11, 2015 at 10:23 AM, Adrian Prantl via cfe-commits < cfe-commits@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 > +@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 > +@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
