> On Sep 11, 2015, at 8:27 AM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Fri, Sep 11, 2015 at 8:18 AM, Adrian Prantl <apra...@apple.com > <mailto:apra...@apple.com>> wrote: > >> On Sep 10, 2015, at 6:56 PM, David Blaikie <dblai...@gmail.com >> <mailto:dblai...@gmail.com>> wrote: >> >> >> >> On Thu, Sep 10, 2015 at 6:40 PM, David Blaikie <dblai...@gmail.com >> <mailto:dblai...@gmail.com>> wrote: >> >> >> On Thu, Sep 10, 2015 at 6:03 PM, Adrian Prantl via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@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 >> +@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 >> +@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 <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <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
