r186262 - PR16214, PR14467: DebugInfo: use "RequireCompleteType" to decide when to emit the full definition of a type in -flimit-debug-info
David Blaikie
dblaikie at gmail.com
Sat Jul 13 18:16:06 PDT 2013
On Sat, Jul 13, 2013 at 3:21 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Sat, Jul 13, 2013 at 2:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > Author: dblaikie
> > Date: Sat Jul 13 16:08:14 2013
> > New Revision: 186262
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=186262&view=rev
> > Log:
> > PR16214, PR14467: DebugInfo: use "RequireCompleteType" to decide when to emit the full definition of a type in -flimit-debug-info
> >
> > This simplifies the core benefit of -flimit-debug-info by taking a more
> > systematic approach to avoid emitting debug info definitions for types
> > that only require declarations. The previous ad-hoc approach (3 cases
> > removed in this patch) had many holes.
> >
> > The general approach (adding a bit to TagDecl and callback through
> > ASTConsumer) has been discussed with Richard Smith - though always open
> > to revision.
> >
> > Modified:
> > cfe/trunk/include/clang/AST/ASTConsumer.h
> > cfe/trunk/include/clang/AST/Decl.h
> > cfe/trunk/include/clang/Sema/Sema.h
> > cfe/trunk/lib/CodeGen/CGClass.cpp
> > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> > cfe/trunk/lib/CodeGen/CGExprCXX.cpp
> > cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> > cfe/trunk/lib/CodeGen/CodeGenAction.cpp
> > cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
> > cfe/trunk/lib/Sema/SemaType.cpp
> > cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp
> >
> > Modified: cfe/trunk/include/clang/AST/ASTConsumer.h
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/AST/ASTConsumer.h (original)
> > +++ cfe/trunk/include/clang/AST/ASTConsumer.h Sat Jul 13 16:08:14 2013
> > @@ -72,6 +72,10 @@ public:
> > /// can be defined in declspecs).
> > virtual void HandleTagDeclDefinition(TagDecl *D) {}
> >
> > + /// \brief This callback is invoked the first time each TagDecl is required to
> > + /// be complete.
> > + virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
> > +
> > /// \brief Invoked when a function is implicitly instantiated.
> > /// Note that at this point point it does not have a body, its body is
> > /// instantiated at the end of the translation unit and passed to
> >
> > Modified: cfe/trunk/include/clang/AST/Decl.h
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/AST/Decl.h (original)
> > +++ cfe/trunk/include/clang/AST/Decl.h Sat Jul 13 16:08:14 2013
> > @@ -2445,6 +2445,9 @@ protected:
> > /// This option is only enabled when modules are enabled.
> > bool MayHaveOutOfDateDef : 1;
> >
> > + /// Has the full definition of this type been required by a use somewhere in
> > + /// the TU.
> > + bool IsCompleteDefinitionRequired : 1;
>
> It looks like you're missing serialization/deserialization support for
> this flag.
Added in r186266 - not sure if it needs separate tests, but when I got
it wrong in a few ways the tests certainly failed, so I guess it's
somewhat covered.
>
> > private:
> > SourceLocation RBraceLoc;
> >
> > @@ -2531,6 +2534,12 @@ public:
> > return IsCompleteDefinition;
> > }
> >
> > + /// \brief Return true if this complete decl is
> > + /// required to be complete for some existing use.
> > + bool isCompleteDefinitionRequired() const {
> > + return IsCompleteDefinitionRequired;
> > + }
> > +
> > /// isBeingDefined - Return true if this decl is currently being defined.
> > bool isBeingDefined() const {
> > return IsBeingDefined;
> > @@ -2572,6 +2581,8 @@ public:
> >
> > void setCompleteDefinition(bool V) { IsCompleteDefinition = V; }
> >
> > + void setCompleteDefinitionRequired() { IsCompleteDefinitionRequired = true; }
> > +
> > // FIXME: Return StringRef;
> > const char *getKindName() const {
> > return TypeWithKeyword::getTagTypeKindName(getTagKind());
> >
> > Modified: cfe/trunk/include/clang/Sema/Sema.h
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Sema/Sema.h (original)
> > +++ cfe/trunk/include/clang/Sema/Sema.h Sat Jul 13 16:08:14 2013
> > @@ -1182,6 +1182,10 @@ public:
> > virtual ~BoundTypeDiagnoser3() { }
> > };
> >
> > +private:
> > + bool RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
> > + TypeDiagnoser &Diagnoser);
> > +public:
> > bool RequireCompleteType(SourceLocation Loc, QualType T,
> > TypeDiagnoser &Diagnoser);
> > bool RequireCompleteType(SourceLocation Loc, QualType T,
> >
> > Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGClass.cpp Sat Jul 13 16:08:14 2013
> > @@ -1630,17 +1630,6 @@ CodeGenFunction::EmitCXXConstructorCall(
> > llvm::Value *This,
> > CallExpr::const_arg_iterator ArgBeg,
> > CallExpr::const_arg_iterator ArgEnd) {
> > -
> > - CGDebugInfo *DI = getDebugInfo();
> > - if (DI &&
> > - CGM.getCodeGenOpts().getDebugInfo() == CodeGenOptions::LimitedDebugInfo) {
> > - // If debug info for this class has not been emitted then this is the
> > - // right time to do so.
> > - const CXXRecordDecl *Parent = D->getParent();
> > - DI->getOrCreateRecordType(CGM.getContext().getTypeDeclType(Parent),
> > - Parent->getLocation());
> > - }
> > -
> > // If this is a trivial constructor, just emit what's needed.
> > if (D->isTrivial()) {
> > if (ArgBeg == ArgEnd) {
> >
> > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Sat Jul 13 16:08:14 2013
> > @@ -1379,7 +1379,8 @@ llvm::DIType CGDebugInfo::CreateType(con
> > RecordDecl *RD = Ty->getDecl();
> > // Limited debug info should only remove struct definitions that can
> > // safely be replaced by a forward declaration in the source code.
> > - if (DebugKind <= CodeGenOptions::LimitedDebugInfo && Declaration) {
> > + if (DebugKind <= CodeGenOptions::LimitedDebugInfo && Declaration &&
> > + !RD->isCompleteDefinitionRequired()) {
> > // FIXME: This implementation is problematic; there are some test
> > // cases where we violate the above principle, such as
> > // test/CodeGen/debug-info-records.c .
> >
> > Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Sat Jul 13 16:08:14 2013
> > @@ -175,17 +175,6 @@ RValue CodeGenFunction::EmitCXXMemberCal
> > const MemberExpr *ME = cast<MemberExpr>(callee);
> > const CXXMethodDecl *MD = cast<CXXMethodDecl>(ME->getMemberDecl());
> >
> > - CGDebugInfo *DI = getDebugInfo();
> > - if (DI &&
> > - CGM.getCodeGenOpts().getDebugInfo() == CodeGenOptions::LimitedDebugInfo &&
> > - !isa<CallExpr>(ME->getBase())) {
> > - QualType PQTy = ME->getBase()->IgnoreParenImpCasts()->getType();
> > - if (const PointerType * PTy = dyn_cast<PointerType>(PQTy)) {
> > - DI->getOrCreateRecordType(PTy->getPointeeType(),
> > - MD->getParent()->getLocation());
> > - }
> > - }
> > -
> > if (MD->isStatic()) {
> > // The method is static, emit it as we would a regular call.
> > llvm::Value *Callee = CGM.GetAddrOfFunction(MD);
> >
> > Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Sat Jul 13 16:08:14 2013
> > @@ -992,18 +992,6 @@ Value *ScalarExprEmitter::VisitMemberExp
> > return Builder.getInt(Value);
> > }
> >
> > - // Emit debug info for aggregate now, if it was delayed to reduce
> > - // debug info size.
> > - CGDebugInfo *DI = CGF.getDebugInfo();
> > - if (DI &&
> > - CGF.CGM.getCodeGenOpts().getDebugInfo()
> > - == CodeGenOptions::LimitedDebugInfo) {
> > - QualType PQTy = E->getBase()->IgnoreParenImpCasts()->getType();
> > - if (const PointerType * PTy = dyn_cast<PointerType>(PQTy))
> > - if (FieldDecl *M = dyn_cast<FieldDecl>(E->getMemberDecl()))
> > - DI->getOrCreateRecordType(PTy->getPointeeType(),
> > - M->getParent()->getLocation());
> > - }
> > return EmitLoadOfLValue(E);
> > }
> >
> >
> > Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Sat Jul 13 16:08:14 2013
> > @@ -171,6 +171,10 @@ namespace clang {
> > Gen->HandleTagDeclDefinition(D);
> > }
> >
> > + virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {
> > + Gen->HandleTagDeclRequiredDefinition(D);
> > + }
> > +
> > virtual void CompleteTentativeDefinition(VarDecl *D) {
> > Gen->CompleteTentativeDefinition(D);
> > }
> >
> > Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Sat Jul 13 16:08:14 2013
> > @@ -13,6 +13,7 @@
> >
> > #include "clang/CodeGen/ModuleBuilder.h"
> > #include "CodeGenModule.h"
> > +#include "CGDebugInfo.h"
> > #include "clang/AST/ASTContext.h"
> > #include "clang/AST/DeclObjC.h"
> > #include "clang/AST/Expr.h"
> > @@ -93,6 +94,12 @@ namespace {
> > }
> > }
> >
> > + virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) LLVM_OVERRIDE {
> > + if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo())
> > + if (const RecordDecl *RD = dyn_cast<RecordDecl>(D))
> > + DI->completeFwdDecl(*RD);
> > + }
> > +
> > virtual void HandleTranslationUnit(ASTContext &Ctx) {
> > if (Diags.hasErrorOccurred()) {
> > M.reset();
> >
> > Modified: cfe/trunk/lib/Sema/SemaType.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaType.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaType.cpp Sat Jul 13 16:08:14 2013
> > @@ -12,6 +12,7 @@
> > //===----------------------------------------------------------------------===//
> >
> > #include "clang/Sema/SemaInternal.h"
> > +#include "clang/AST/ASTConsumer.h"
> > #include "clang/AST/ASTContext.h"
> > #include "clang/AST/ASTMutationListener.h"
> > #include "clang/AST/CXXInheritance.h"
> > @@ -4844,6 +4845,20 @@ bool Sema::RequireCompleteExprType(Expr
> > /// @c false otherwise.
> > bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
> > TypeDiagnoser &Diagnoser) {
> > + if (RequireCompleteTypeImpl(Loc, T, Diagnoser))
> > + return true;
> > + if (const TagType *Tag = T->getAs<TagType>()) {
> > + if (!Tag->getDecl()->isCompleteDefinitionRequired()) {
> > + Tag->getDecl()->setCompleteDefinitionRequired();
> > + Consumer.HandleTagDeclRequiredDefinition(Tag->getDecl());
> > + }
> > + }
> > + return false;
> > +}
> > +
> > +/// \brief The implementation of RequireCompleteType
> > +bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
> > + TypeDiagnoser &Diagnoser) {
> > // FIXME: Add this assertion to make sure we always get instantiation points.
> > // assert(!Loc.isInvalid() && "Invalid location in RequireCompleteType");
> > // FIXME: Add this assertion to help us flush out problems with
> >
> > Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp?rev=186262&r1=186261&r2=186262&view=diff
> > ==============================================================================
> > --- cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp (original)
> > +++ cfe/trunk/test/CodeGenCXX/debug-info-class-limited.cpp Sat Jul 13 16:08:14 2013
> > @@ -1,8 +1,7 @@
> > // RUN: %clang -emit-llvm -g -S %s -o - | FileCheck %s
> >
> > namespace PR16214_1 {
> > -// CHECK: [[PR16214_1:![0-9]*]] = {{.*}} [ DW_TAG_namespace ] [PR16214_1]
> > -// CHECK: = metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata [[PR16214_1]], {{.*}} ; [ DW_TAG_structure_type ] [foo] {{.*}} [def]
> > +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def]
> > struct foo {
> > int i;
> > };
> > @@ -13,9 +12,9 @@ bar *a;
> > bar b;
> > }
> >
> > -namespace test1 {
> > +namespace PR14467 {
> > +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def]
> > struct foo {
> > - int i;
> > };
> >
> > foo *bar(foo *a) {
> > @@ -24,9 +23,9 @@ foo *bar(foo *a) {
> > }
> > }
> >
> > -namespace test2 {
> > +namespace test1 {
> > +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [decl]
> > struct foo {
> > - int i;
> > };
> >
> > extern int bar(foo *a);
> > @@ -34,3 +33,20 @@ int baz(foo *a) {
> > return bar(a);
> > }
> > }
> > +
> > +namespace test2 {
> > +// FIXME: if we were a bit fancier, we could realize that the 'foo' type is only
> > +// required because of the 'bar' type which is not required at all (or might
> > +// only be required to be declared)
> > +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def]
> > +struct foo {
> > +};
> > +
> > +struct bar {
> > + foo f;
> > +};
> > +
> > +void func() {
> > + foo *f;
> > +}
> > +}
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list