r247049 - Module Debugging: Emit debug type information into clang modules.
Adrian Prantl via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 9 09:26:10 PDT 2015
> On Sep 8, 2015, at 8:05 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Sep 8, 2015 at 12:20 PM, Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> Author: adrian
> Date: Tue Sep 8 14:20:27 2015
> New Revision: 247049
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247049&view=rev <http://llvm.org/viewvc/llvm-project?rev=247049&view=rev>
> Log:
> Module Debugging: Emit debug type information into clang modules.
>
> When -fmodule-format is set to "obj", emit debug info for all types
> declared in a module or referenced by a declaration into the module's
> object file container.
>
> This patch adds support for C and C++ types.
>
> Added:
> cfe/trunk/test/Modules/Inputs/DebugCXX.h
> cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> Modified:
> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> cfe/trunk/test/Modules/Inputs/module.map
>
> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247049&r1=247048&r2=247049&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247049&r1=247048&r2=247049&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Tue Sep 8 14:20:27 2015
> @@ -50,6 +50,34 @@ class PCHContainerGenerator : public AST
> raw_pwrite_stream *OS;
> std::shared_ptr<PCHBuffer> Buffer;
>
> + /// Visit every type and emit debug info for it.
> + struct DebugTypeVisitor : public RecursiveASTVisitor<DebugTypeVisitor> {
> + clang::CodeGen::CGDebugInfo &DI;
> + ASTContext &Ctx;
> + DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx)
> + : DI(DI), Ctx(Ctx) {}
> +
> + /// Determine whether this type can be represented in DWARF.
> + static bool CanRepresent(const Type *Ty) {
> + return !Ty->isDependentType() && !Ty->isUndeducedType();
> + }
> +
> + bool VisitTypeDecl(TypeDecl *D) {
>
> Does this only visit types defined in this module? Or also types otherwise referenced by this module? (does it only visit definitions, or also declarations?)
Currently this does the wrong thing and emits all types referenced by this module because the patch that emits fwddecls for external types is not yet committed (though expected to land soon).
>
> + QualType QualTy = Ctx.getTypeDeclType(D);
> + if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
> + DI.getOrCreateStandaloneType(QualTy, D->getLocation());
> + return true;
> + }
> +
> + bool VisitValueDecl(ValueDecl *D) {
>
> This seems strange - why would we put this type into the module debug info? (the type may be defined elsewhere, in another module, etc - just because there's a variable of that type defined in the module wouldn't imply that the module should contain a definition of the type, would it? That would seem to result in a lot of duplicate info)
CGDebugInfo eventually will only emit type definitions for types defined in the current module, but that said, I think your argument is correct and this can safely be removed.
>
> + QualType QualTy = D->getType();
> + if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
> + DI.getOrCreateStandaloneType(QualTy, D->getLocation());
> + return true;
> + }
> +
> + };
> +
> public:
> PCHContainerGenerator(DiagnosticsEngine &diags,
> const HeaderSearchOptions &HSO,
> @@ -82,6 +110,36 @@ public:
> *Ctx, HeaderSearchOpts, PreprocessorOpts, CodeGenOpts, *M, Diags));
> }
>
> + bool HandleTopLevelDecl(DeclGroupRef D) override {
> + if (Diags.hasErrorOccurred() ||
> + (CodeGenOpts.getDebugInfo() == CodeGenOptions::NoDebugInfo))
> + return true;
> +
> + // Collect debug info for all decls in this group.
> + for (auto *I : D)
> + if (!I->isFromASTFile()) {
> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx);
> + DTV.TraverseDecl(I);
> + }
> + return true;
> + }
> +
> + void HandleTagDeclDefinition(TagDecl *D) override {
> + if (Diags.hasErrorOccurred())
>
> Is it particularly useful to check for errors here (& in HandleTagDeclRequiredDefinition below)? That seems like a speed optimization for the slow case (we're emitting diagnostics anyway, the speed isn't too important - and we'll still have to be able to discard this work later in case we haven't seen errors yet, but we do see them later/by the end of the module?)
This is not a speed optimization but crash prevention. When compiling source with errors clang will still build an AST and continue as far as possible and the type system will contain unresolved types which are not handled by CGDebugInfo so we need to bail out here.
>
> + return;
> +
> + Builder->UpdateCompletedType(D);
> + }
> +
> + void HandleTagDeclRequiredDefinition(const TagDecl *D) override {
> + if (Diags.hasErrorOccurred())
> + return;
> +
> + if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo())
> + if (const RecordDecl *RD = dyn_cast<RecordDecl>(D))
> + DI->completeRequiredType(RD);
> + }
> +
> /// Emit a container holding the serialized AST.
> void HandleTranslationUnit(ASTContext &Ctx) override {
> assert(M && VMContext && Builder);
>
> Added: cfe/trunk/test/Modules/Inputs/DebugCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=247049&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=247049&view=auto>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/DebugCXX.h (added)
> +++ cfe/trunk/test/Modules/Inputs/DebugCXX.h Tue Sep 8 14:20:27 2015
>
> A test case involving two modules (where one module depends on the other and uses its types) would be useful to demonstrate that we aren't redundantly emitting types into modules that didn't define them, perhaps?
Yes, such a test will come in the aforementioned next patch that introduces forward declarations for AST-imported types in CGDebugInfo.
>
> @@ -0,0 +1,52 @@
> +/* -*- C++ -*- */
> +namespace DebugCXX {
> + // Records.
> + struct Struct {
> + int i;
> + static int static_member;
> + };
> +
> + // Enums.
> + enum Enum {
> + Enumerator
> + };
> + enum {
> + e1 = '1'
> + };
> + enum {
> + e2 = '2'
> + };
> +
> + // Templates (instatiations).
> + template<typename T> struct traits {};
> + template<typename T,
> + typename Traits = traits<T>
> + > class Template {
> + T member;
> + };
> + extern template class Template<int>;
> +
> + extern template struct traits<float>;
> + typedef class Template<float> FloatInstatiation;
> +
> + inline void fn() {
> + Template<long> invisible;
> + }
> +
> + // Non-template inside a template.
> + template <class> struct Outer {
> + Outer();
> + struct Inner {
> + Inner(Outer) {}
> + };
> + };
> + template <class T> Outer<T>::Outer() {
> + Inner a(*this);
> + };
> +
> + // Partial template specialization.
> + template <typename...> class A;
> + template <typename T> class A<T> {};
> + typedef A<void> B;
> + void foo(B) {}
> +}
>
> Modified: cfe/trunk/test/Modules/Inputs/module.map
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=247049&r1=247048&r2=247049&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=247049&r1=247048&r2=247049&view=diff>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/module.map (original)
> +++ cfe/trunk/test/Modules/Inputs/module.map Tue Sep 8 14:20:27 2015
> @@ -332,6 +332,10 @@ module DebugModule {
> header "DebugModule.h"
> }
>
> +module DebugCXX {
> + header "DebugCXX.h"
> +}
> +
> module ImportNameInDir {
> header "ImportNameInDir.h"
> export *
>
> Added: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247049&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247049&view=auto>
> ==============================================================================
> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (added)
> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Tue Sep 8 14:20:27 2015
> @@ -0,0 +1,41 @@
> +// Test that (the same) debug info is emitted for an Objective-C++
> +// module and a C++ precompiled header.
> +
> +// REQUIRES: asserts, shell
> +
> +// Modules:
> +// RUN: rm -rf %t
> +// RUN: %clang_cc1 -x objective-c++ -std=c++11 -g -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll
> +// RUN: cat %t-mod.ll | FileCheck %s
> +
> +// PCH:
> +// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h -mllvm -debug-only=pchcontainer &>%t-pch.ll
> +// RUN: cat %t-pch.ll | FileCheck %s
> +
> +#ifdef MODULES
> + at import DebugCXX;
> +#endif
> +
>
> If practical, it'd be nice to have these checks near the things tehy're checking for - it'd make the code easier to read/understand. (haven't looked at related tests in the same directory to see what the convention's like)
The other tests all have their imported modules in Inputs/. The module is supposed to be shared among several testcases (one for testiung the debug info inside the module, one testing the debug info referring to the module) so separating this out may even be somewhat more readable than having everything inside a single file. I debated this with myself, too :-)
>
> +// CHECK: distinct !DICompileUnit(language: DW_LANG_{{.*}}C_plus_plus,
> +// CHECK-SAME: isOptimized: false,
> +// CHECK-SAME: splitDebugFilename:
> +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Enum"
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE")
> +// CHECK: !DINamespace(name: "DebugCXX"
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<int, DebugCXX::traits<int> >"
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<float, DebugCXX::traits<float> >"
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<long, DebugCXX::traits<long> >"
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE")
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
> +// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
> +// no mangled name here yet.
>
> Is this a 'fixme'? (might be good to phrase it that way) - any idea why they're missing?
DIDerivedType doesn’t have a UID field. My uneducated guess is that the C++ name mangling schemes don’t represent typedef sugar?
-- adrian
>
> +// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
> +// no mangled name here yet.
>
>
> _______________________________________________
> 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/20150909/35ceb86f/attachment-0001.html>
More information about the cfe-commits
mailing list