[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG651128f55722: [DebugInfo] Add option to clang to limit debug 
info that is emitted for classes. (authored by akhuang).

Changed prior to commit:
  https://reviews.llvm.org/D72427?vs=237811=238077#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-limited-ctor.cpp


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | 
FileCheck %s
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct A {};
+void TestA() { A a; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct B {
+  B();
+};
+void TestB() { B b; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "C"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct C {
+  C() {}
+};
+void TestC() { C c; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "D"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct D {
+  D();
+};
+D::D() {}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1648,6 +1648,12 @@
   if (CGM.getLangOpts().Optimize)
 SPFlags |= llvm::DISubprogram::SPFlagOptimized;
 
+  // In this debug mode, emit type info for a class when its constructor type
+  // info is emitted.
+  if (DebugKind == codegenoptions::DebugInfoConstructor)
+if (const CXXConstructorDecl *CD = dyn_cast(Method))
+  completeClass(CD->getParent());
+
   llvm::DINodeArray TParamsArray = CollectFunctionTemplateParams(Method, Unit);
   llvm::DISubprogram *SP = DBuilder.createMethod(
   RecordTy, MethodName, MethodLinkageName, MethodDefUnit, MethodLine,
@@ -2211,6 +2217,17 @@
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
+  // In constructor debug mode, only emit debug info for a class when its
+  // constructor is emitted. Skip this optimization if the class or any of
+  // its methods are marked dllimport.
+  if (DebugKind == codegenoptions::DebugInfoConstructor &&
+  !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) {
+for (const auto *Ctor : CXXDecl->ctors()) {
+  if (Ctor->isUserProvided())
+return true;
+}
+  }
+
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast(RD))
 Spec = SD->getSpecializationKind();


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct A {};
+void TestA() { A a; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct B {
+  B();
+};
+void TestB() { B b; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "C"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct C {
+  C() {}
+};
+void TestC() { C c; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "D"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct D {
+  D();
+};
+D::D() {}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1648,6 +1648,12 @@
   if (CGM.getLangOpts().Optimize)
 SPFlags |= llvm::DISubprogram::SPFlagOptimized;
 
+  // In this debug mode, emit type info for a class when its constructor type
+  // info is emitted.
+  if (DebugKind == codegenoptions::DebugInfoConstructor)
+if (const CXXConstructorDecl *CD = dyn_cast(Method))
+  completeClass(CD->getParent());
+
   llvm::DINodeArray TParamsArray = CollectFunctionTemplateParams(Method, Unit);
   llvm::DISubprogram *SP = DBuilder.createMethod(
   RecordTy, MethodName, MethodLinkageName, MethodDefUnit, MethodLine,
@@ -2211,6 +2217,17 @@
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
+  // In constructor debug mode, only emit debug info for a class when its
+  // constructor is emitted. Skip this optimization if the 

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 237811.
akhuang added a comment.

-Remove redundant test case
-Committed refactoring part


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-limited-ctor.cpp


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | 
FileCheck %s
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct A {};
+void TestA() { A a; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct B {
+  B();
+};
+void TestB() { B b; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "C"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct C {
+  C() {}
+};
+void TestC() { C c; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "D"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct D {
+  D();
+};
+D::D() {}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1648,6 +1648,12 @@
   if (CGM.getLangOpts().Optimize)
 SPFlags |= llvm::DISubprogram::SPFlagOptimized;
 
+  // In this debug mode, emit type info for a class when its constructor type
+  // info is emitted.
+  if (DebugKind == codegenoptions::DebugInfoConstructor)
+if (const CXXConstructorDecl *CD = dyn_cast(Method))
+  completeClass(CD->getParent());
+
   llvm::DINodeArray TParamsArray = CollectFunctionTemplateParams(Method, Unit);
   llvm::DISubprogram *SP = DBuilder.createMethod(
   RecordTy, MethodName, MethodLinkageName, MethodDefUnit, MethodLine,
@@ -2211,6 +2217,17 @@
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
+  // In constructor debug mode, only emit debug info for a class when its
+  // constructor is emitted. Skip this optimization if the class or any of
+  // its methods are marked dllimport.
+  if (DebugKind == codegenoptions::DebugInfoConstructor &&
+  !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) {
+for (const auto *Ctor : CXXDecl->ctors()) {
+  if (ctor->isUserProvided())
+return true;
+}
+  }
+
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast(RD))
 Spec = SD->getSpecializationKind();


Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct A {};
+void TestA() { A a; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct B {
+  B();
+};
+void TestB() { B b; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "C"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct C {
+  C() {}
+};
+void TestC() { C c; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "D"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct D {
+  D();
+};
+D::D() {}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1648,6 +1648,12 @@
   if (CGM.getLangOpts().Optimize)
 SPFlags |= llvm::DISubprogram::SPFlagOptimized;
 
+  // In this debug mode, emit type info for a class when its constructor type
+  // info is emitted.
+  if (DebugKind == codegenoptions::DebugInfoConstructor)
+if (const CXXConstructorDecl *CD = dyn_cast(Method))
+  completeClass(CD->getParent());
+
   llvm::DINodeArray TParamsArray = CollectFunctionTemplateParams(Method, Unit);
   llvm::DISubprogram *SP = DBuilder.createMethod(
   RecordTy, MethodName, MethodLinkageName, MethodDefUnit, MethodLine,
@@ -2211,6 +2217,17 @@
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
+  // In constructor debug mode, only emit debug info for a class when its
+  // constructor is emitted. Skip this optimization if the class or any of
+  // its methods are marked dllimport.
+  if (DebugKind == codegenoptions::DebugInfoConstructor &&
+  !CXXDecl->isLambda() && 

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D72427#1818322 , @rnk wrote:

> > Total object file size on Windows, compiling with RelWithDebInfo:
> > 
> > before: 4,257,448 kb
> >  after:  2,104,963 kb
> > 
> > And on Linux
> > 
> > before: 9,225,140 kb
> >  after:  4,387,464 kb
>
> These numbers are amazing!
>
> I made a summary of Amy's list of types that become incomplete here: 
> https://reviews.llvm.org/P8184
>  It collapses template parameters and counts up instantiations.


Might be worth looking at it without collapsing the instantiations - but, yeah, 
I guess some templates get used in otehr templates in fairly regular ways, such 
that all/many of their instantiations are missing/present for the same reasons.

> I picked some types at random to analyze:
> 
> - llvm::MachineModuleAnalysis: Looks like this type is required to be 
> complete, but never actually constructed in LLVM. Amusing, seems 
> working-as-intended.

Yeah, guess maybe it was added for consistency with other types - maybe we 
should delete it with a comment on how to add it back in if needed? (or 
generalize other cases using a template, so there's no need for this to be 
explicitly written out)

> - llvm::IntrinsicLowering: ditto
> - llvm::ArrayRef: Used here: 
> http://llvm-cs.pcc.me.uk/include/llvm/Object/Wasm.h/relements This seems WAI, 
> it is only used in LLD, not clang, so we don't need it to debug clang.

Perhaps that API surface area could use a unit test in LLVM then - unfortunate 
to have code in LLVM that's only tested in one of the subprojects. But yeah, 
for the purposes of the debug info - that seems like it's working as intended. 
If you write a member function with a return type but never call that function 
- great, type isn't needed.

> In D72427#1812515 , @dblaikie wrote:
> 
>> What's the plan for this? Is it still in an experimental stage, with the 
>> intent to investigate the types that are no longer emitted unedr the flag & 
>> explain why they're missing (& either have a justification for why that's 
>> acceptable, or work on additional heuristics to address the gaps?)?
>>
>> If so, I'd probably rather this not be a full driver flag - if it's a 
>> reliable way to reduce debug info size (like the existing heuristics under 
>> -fstandalone-debug*) it should be rolled into -fno-standalone-debug 
>> behavior, and if it's not fully fleshed out yet, I think an -Xclang flag 
>> would be more suitable for the experimental phase.
>>
>> Do you have any sample of data on the sort of situations that lead to 
>> missing types under this heuristic?
> 
> 
> I'm glad you think we might eventually be able to roll this into 
> fno-standalone-debug. :)

The general design philosophy for this slice is consistent with that flag "did 
you compile the whole program with debug info? If yes, here's ways we can 
minimize the debug info to take advantage of that fact" & the fewer 
slices/variations (there are already many) in debug info emission we have, the 
better, I think.

> However, it is possible for the user to require a type to be complete, but 
> never construct it, as is typically the case for type traits. This patch 
> continues to emit most type trait classes, because they lack user-declared 
> constructors, but you could imagine giving such a class a constructor and 
> never calling it, and then later trying to look at it in the debugger. In 
> this new mode, it would be missing, probably just forward declared.

Most type traits get used to declare named variables, return types, etc (though 
admittedly only the outermost trait gets used/seen in the debug info - because 
the inner traits get canonicalized... ) - and their nested member typedef is 
the thing that you need in source and in any expression evaluation - so that 
would still have to be emitted.

If every trait template got emitted as a type declaration with a nested member 
type typedef emitted as well - I don't think that'd be any great loss in 
debuggability, you could still use the trait from your debugger in the usual 
way (by using that member typedef or constant).

> Do you think the user should have a mode between the constructor mode and the 
> standalone mode? Our current mode is also pretty close to what GCC does, so 
> we might want to keep the current mode for users who mix GCC and Clang debug 
> info. Alternatively, we could give users some kind of attribute escape hatch 
> to force-emit some particular type information.

If there are cases where GCC wouldn't emit the "homed" debug info, yeah, I'd be 
a bit concerned - but anywhere that GCC emits debug info for the ctor it'd emit 
the whole type too, so I think it's probably fine. A broad test to demonstrate 
that GCC never skips a type that clang emits in this way would be handy to have 
to back this up.

(the exsiting functionality in clang that does homing of explicit template 
specialization decls/defs /can/ have this same 

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

Committed debug info kind refactoring bit in fe7cda2e.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53539bb032d1: [DebugInfo] Add another level to DebugInfoKind 
called Constructor (authored by akhuang).

Changed prior to commit:
  https://reviews.llvm.org/D72427?vs=237411=237808#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DebugInfoOptions.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -731,6 +731,7 @@
 llvm::StringSwitch(A->getValue())
 .Case("line-tables-only", codegenoptions::DebugLineTablesOnly)
 .Case("line-directives-only", codegenoptions::DebugDirectivesOnly)
+.Case("constructor", codegenoptions::DebugInfoConstructor)
 .Case("limited", codegenoptions::LimitedDebugInfo)
 .Case("standalone", codegenoptions::FullDebugInfo)
 .Default(~0U);
@@ -787,8 +788,7 @@
   llvm::Triple::arm, llvm::Triple::armeb};
 
   llvm::Triple T(TargetOpts.Triple);
-  if (Opts.OptimizationLevel > 0 &&
-  Opts.getDebugInfo() >= codegenoptions::LimitedDebugInfo &&
+  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -999,6 +999,9 @@
   case codegenoptions::DebugLineTablesOnly:
 CmdArgs.push_back("-debug-info-kind=line-tables-only");
 break;
+  case codegenoptions::DebugInfoConstructor:
+CmdArgs.push_back("-debug-info-kind=constructor");
+break;
   case codegenoptions::LimitedDebugInfo:
 CmdArgs.push_back("-debug-info-kind=limited");
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4118,13 +4118,13 @@
 
   // Emit global variable debug information.
   if (CGDebugInfo *DI = getModuleDebugInfo())
-if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
+if (getCodeGenOpts().hasReducedDebugInfo())
   DI->EmitGlobalVariable(GV, D);
 }
 
 void CodeGenModule::EmitExternalVarDeclaration(const VarDecl *D) {
   if (CGDebugInfo *DI = getModuleDebugInfo())
-if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo) {
+if (getCodeGenOpts().hasReducedDebugInfo()) {
   QualType ASTTy = D->getType();
   llvm::Type *Ty = getTypes().ConvertTypeForMem(D->getType());
   llvm::PointerType *PTy =
@@ -5371,7 +5371,7 @@
 ObjCRuntime->GenerateClass(OMD);
 // Emit global variable debug information.
 if (CGDebugInfo *DI = getModuleDebugInfo())
-  if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
+  if (getCodeGenOpts().hasReducedDebugInfo())
 DI->getOrCreateInterfaceType(getContext().getObjCInterfaceType(
 OMD->getClassInterface()), OMD->getLocation());
 break;
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2110,7 +2110,7 @@
   const APValue ) {
   assert(Init.hasValue() && "Invalid DeclRefExpr initializer!");
   if (CGDebugInfo *Dbg = getDebugInfo())
-if (CGM.getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
+if (CGM.getCodeGenOpts().hasReducedDebugInfo())
   Dbg->EmitGlobalVariable(E->getDecl(), Init);
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -567,8 +567,7 @@
   const CapturedDecl *CD = S.getCapturedDecl();
   // Build the argument list.
   bool NeedWrapperFunction =
-  getDebugInfo() &&
-  CGM.getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo;
+  getDebugInfo() && CGM.getCodeGenOpts().hasReducedDebugInfo();
   FunctionArgList Args;
   llvm::MapVector> LocalAddrs;
   

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

> Total object file size on Windows, compiling with RelWithDebInfo:
> 
> before: 4,257,448 kb
>  after:  2,104,963 kb
> 
> And on Linux
> 
> before: 9,225,140 kb
>  after:  4,387,464 kb

These numbers are amazing!

I made a summary of Amy's list of types that become incomplete here: 
https://reviews.llvm.org/P8184
It collapses template parameters and counts up instantiations.

I picked some types at random to analyze:

- llvm::MachineModuleAnalysis: Looks like this type is required to be complete, 
but never actually constructed in LLVM. Amusing, seems working-as-intended.
- llvm::IntrinsicLowering: ditto
- llvm::ArrayRef: Used here: 
http://llvm-cs.pcc.me.uk/include/llvm/Object/Wasm.h/relements This seems WAI, 
it is only used in LLD, not clang, so we don't need it to debug clang.

In D72427#1812515 , @dblaikie wrote:

> What's the plan for this? Is it still in an experimental stage, with the 
> intent to investigate the types that are no longer emitted unedr the flag & 
> explain why they're missing (& either have a justification for why that's 
> acceptable, or work on additional heuristics to address the gaps?)?
>
> If so, I'd probably rather this not be a full driver flag - if it's a 
> reliable way to reduce debug info size (like the existing heuristics under 
> -fstandalone-debug*) it should be rolled into -fno-standalone-debug behavior, 
> and if it's not fully fleshed out yet, I think an -Xclang flag would be more 
> suitable for the experimental phase.
>
> Do you have any sample of data on the sort of situations that lead to missing 
> types under this heuristic?


I'm glad you think we might eventually be able to roll this into 
fno-standalone-debug. :)

However, it is possible for the user to require a type to be complete, but 
never construct it, as is typically the case for type traits. This patch 
continues to emit most type trait classes, because they lack user-declared 
constructors, but you could imagine giving such a class a constructor and never 
calling it, and then later trying to look at it in the debugger. In this new 
mode, it would be missing, probably just forward declared.

Do you think the user should have a mode between the constructor mode and the 
standalone mode? Our current mode is also pretty close to what GCC does, so we 
might want to keep the current mode for users who mix GCC and Clang debug info. 
Alternatively, we could give users some kind of attribute escape hatch to 
force-emit some particular type information.

In any case, we don't have to answer this question to land the -cc1 mode, as 
you have already discussed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Looks pretty good to me - if you could commit the debug info level refactoring 
separately/up-front, and maybe the test case could be simplified a bit. Looking 
forward to seeing what comes of this option, analysis of missing types, etc.




Comment at: clang/include/clang/Basic/CodeGenOptions.h:360-364
+
+  /// Check if full debug info should be emitted.
+  bool isFullDebug() const {
+return getDebugInfo() >= codegenoptions::DebugInfoConstructor;
+  }

dblaikie wrote:
> Could you commit (the pre-patch equivalent) of this change now/before your 
> patch - it'll simplify the diff/make it easier to review.
> 
> Though I think the name needs some massaging, since it doesn't return 
> "getDebugInfo() == codegenoptions::FullDebugInfo", so the name's a bit 
> misleading.
> 
> /maybe/ (but honestly I don't have any great names) 
> "hasVariableAndTypeDebugInfo" though that's a bit of a mouthful.
Yeah, name's still a bit awkward, but I've got nothing better - could you 
submit this function/usage now/ahead of the rest of this patch so it's easier 
to see what this patch is changing? (& easier to revert this patch if needed, 
etc)



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:9-15
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct B {};
+int bar(B *b);
+int TestB(B *b) {
+  return bar(b);
+}

This test case doesn't look interesting to me - I would expect that'd be 
covered by other tests already in-tree?

Is there a particular nuance this case is intended to capture? (I'm curious 
why/how 'bar's presence would make a difference in the presence of TestB 
already)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Orlando Cazalet-Hyams via Phabricator via cfe-commits]

Orlando added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2225
+  !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) {
+for (const auto *ctor : CXXDecl->ctors()) {
+  if (ctor->isUserProvided())

Nit: Should `ctor` be `Ctor` here?

I'm just passing by, please don't wait on any approval from me for this patch.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 237411.
akhuang added a comment.

Address comments

- Removed driver option
- Simplified test cases
- Changed name of limited debug info from isFullDebug to hasReducedDebugInfo, 
which is maybe

slightly less misleading


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DebugInfoOptions.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/debug-info-limited-ctor.cpp

Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct A {};
+void TestA() { A a; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct B {};
+int bar(B *b);
+int TestB(B *b) {
+  return bar(b);
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "C"
+// CHECK-SAME: flags: DIFlagFwdDecl
+struct C {
+  C();
+};
+void TestC() { C c; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "D"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct D {
+  D() {}
+};
+void TestD() { D d; }
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "E"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+struct E {
+  E();
+};
+E::E() {}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -731,6 +731,7 @@
 llvm::StringSwitch(A->getValue())
 .Case("line-tables-only", codegenoptions::DebugLineTablesOnly)
 .Case("line-directives-only", codegenoptions::DebugDirectivesOnly)
+.Case("constructor", codegenoptions::DebugInfoConstructor)
 .Case("limited", codegenoptions::LimitedDebugInfo)
 .Case("standalone", codegenoptions::FullDebugInfo)
 .Default(~0U);
@@ -787,8 +788,7 @@
   llvm::Triple::arm, llvm::Triple::armeb};
 
   llvm::Triple T(TargetOpts.Triple);
-  if (Opts.OptimizationLevel > 0 &&
-  Opts.getDebugInfo() >= codegenoptions::LimitedDebugInfo &&
+  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -999,6 +999,9 @@
   case codegenoptions::DebugLineTablesOnly:
 CmdArgs.push_back("-debug-info-kind=line-tables-only");
 break;
+  case codegenoptions::DebugInfoConstructor:
+CmdArgs.push_back("-debug-info-kind=constructor");
+break;
   case codegenoptions::LimitedDebugInfo:
 CmdArgs.push_back("-debug-info-kind=limited");
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4113,13 +4113,13 @@
 
   // Emit global variable debug information.
   if (CGDebugInfo *DI = getModuleDebugInfo())
-if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
+if (getCodeGenOpts().hasReducedDebugInfo())
   DI->EmitGlobalVariable(GV, D);
 }
 
 void CodeGenModule::EmitExternalVarDeclaration(const VarDecl *D) {
   if (CGDebugInfo *DI = getModuleDebugInfo())
-if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo) {
+if (getCodeGenOpts().hasReducedDebugInfo()) {
   QualType ASTTy = D->getType();
   llvm::Type *Ty = getTypes().ConvertTypeForMem(D->getType());
   llvm::PointerType *PTy =
@@ -5366,7 +5366,7 @@
 ObjCRuntime->GenerateClass(OMD);
 // Emit global variable debug information.
 if (CGDebugInfo *DI = getModuleDebugInfo())
-  if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
+  if (getCodeGenOpts().hasReducedDebugInfo())
 DI->getOrCreateInterfaceType(getContext().getObjCInterfaceType(
 OMD->getClassInterface()), 

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang marked 5 inline comments as done.
akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4516
 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue ) 
{
-  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(CGM.getCodeGenOpts().isFullDebug());
   if (VD->hasAttr())

aprantl wrote:
> akhuang wrote:
> > aprantl wrote:
> > > This change appears to be unnecessary?
> > Do you mean changing the DebugKind comparison?
> Yes, since the new kind is < Limited.
The new kind is supposed to do most of the same things as Limited, except 
slightly more limited, so I think the comparison needs to be changed? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3692
+  Args.getLastArg(options::OPT_flimit_debug_info_constructor), Args, 
D, TC) &&
+DebugInfoKind == codegenoptions::LimitedDebugInfo)
+  DebugInfoKind = codegenoptions::DebugInfoConstructor;

akhuang wrote:
> dblaikie wrote:
> > I'm surprised that this flag would only apply if you were already using 
> > -flimit-debug-info but that looks like what this code does? (I'd probably 
> > expect these to override each other -flimit-debug-info 
> > -flimit-debug-info-constructor gets you constructor limited, the other way 
> > around gets the classic/pre-patch behavior? but once this becomes not a 
> > driver option anymore, that gets a bit murkier I guess)
> I did this assuming that DebugInfoKind was Limited by default, but looking at 
> it now I think I just didn't put it in the right place in the code. 
> 
> If this wasn't a driver option, then I guess it would just be 
> -debug-info-kind=constructor? 
> 
Yeah, if that works, simple enough!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4516
 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue ) 
{
-  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(CGM.getCodeGenOpts().isFullDebug());
   if (VD->hasAttr())

akhuang wrote:
> aprantl wrote:
> > This change appears to be unnecessary?
> Do you mean changing the DebugKind comparison?
Yes, since the new kind is < Limited.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang marked 2 inline comments as done.
akhuang added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4516
 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue ) 
{
-  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(CGM.getCodeGenOpts().isFullDebug());
   if (VD->hasAttr())

aprantl wrote:
> This change appears to be unnecessary?
Do you mean changing the DebugKind comparison?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3692
+  Args.getLastArg(options::OPT_flimit_debug_info_constructor), Args, 
D, TC) &&
+DebugInfoKind == codegenoptions::LimitedDebugInfo)
+  DebugInfoKind = codegenoptions::DebugInfoConstructor;

dblaikie wrote:
> I'm surprised that this flag would only apply if you were already using 
> -flimit-debug-info but that looks like what this code does? (I'd probably 
> expect these to override each other -flimit-debug-info 
> -flimit-debug-info-constructor gets you constructor limited, the other way 
> around gets the classic/pre-patch behavior? but once this becomes not a 
> driver option anymore, that gets a bit murkier I guess)
I did this assuming that DebugInfoKind was Limited by default, but looking at 
it now I think I just didn't put it in the right place in the code. 

If this wasn't a driver option, then I guess it would just be 
-debug-info-kind=constructor? 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D72427#1812954 , @akhuang wrote:

> > What's the plan for this? Is it still in an experimental stage, with the 
> > intent to investigate the types that are no longer emitted unedr the flag & 
> > explain why they're missing (& either have a justification for why that's 
> > acceptable, or work on additional heuristics to address the gaps?)?
> > 
> > If so, I'd probably rather this not be a full driver flag - if it's a 
> > reliable way to reduce debug info size (like the existing heuristics under 
> > -fstandalone-debug*) it should be rolled into -fno-standalone-debug 
> > behavior, and if it's not fully fleshed out yet, I think an -Xclang flag 
> > would be more suitable for the experimental phase.
>
> Pretty much, I think the plan is to investigate further and maybe have more 
> people try it. The -Xclang flag seems reasonable. Do you have thoughts on 
> whether the added DebugInfoKind level makes sense?


Yeah, that seems reasonable - though once this mode has been 
vetted/stabilized/ironed out, hopefully that can be removed and the 
functionality will be rolled into LimitedDebugInfo.




Comment at: clang/include/clang/Basic/CodeGenOptions.h:360-364
+
+  /// Check if full debug info should be emitted.
+  bool isFullDebug() const {
+return getDebugInfo() >= codegenoptions::DebugInfoConstructor;
+  }

Could you commit (the pre-patch equivalent) of this change now/before your 
patch - it'll simplify the diff/make it easier to review.

Though I think the name needs some massaging, since it doesn't return 
"getDebugInfo() == codegenoptions::FullDebugInfo", so the name's a bit 
misleading.

/maybe/ (but honestly I don't have any great names) 
"hasVariableAndTypeDebugInfo" though that's a bit of a mouthful.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2225
+  !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) {
+for (auto ctor : CXXDecl->ctors()) {
+  if (ctor->isUserProvided())

Is the type here a pointer - if so, then "auto *ctor" would be preferred. (if 
it's not, then probably want to avoid copying it and use "auto ")



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3692
+  Args.getLastArg(options::OPT_flimit_debug_info_constructor), Args, 
D, TC) &&
+DebugInfoKind == codegenoptions::LimitedDebugInfo)
+  DebugInfoKind = codegenoptions::DebugInfoConstructor;

I'm surprised that this flag would only apply if you were already using 
-flimit-debug-info but that looks like what this code does? (I'd probably 
expect these to override each other -flimit-debug-info 
-flimit-debug-info-constructor gets you constructor limited, the other way 
around gets the classic/pre-patch behavior? but once this becomes not a driver 
option anymore, that gets a bit murkier I guess)



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:23
+
+extern int bar(B *b);
+int TestB(B *b) {

"extern" here is redundant - probably best to remove it.



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:35-36
+C *TestC() {
+  C *c = new C();
+  return c;
+}

Skip the local variable if it isn't needed & "return new C();" perhaps?

Though perhaps all these uses of "new" could use the direct type instead (as 
that should produce simpler IR, maybe make the test cases more obvious)?

"void TestC() { C c; }"

(or "C Test() { return C(); }"  but that seems more complicated.

Or maybe even use a straight global variable "C c;" ? (but I guess then you 
might need "extern C c; C c;" in some cases to make sure they're emitted at all 
- I'm not sure)

Or does that not capture what they're intended to test?



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:54-58
+class E {
+public:
+*d = new D();
+  return d;
+}

Did something get mangled in the patch upload, perhaps? - this doesn't look 
like valid code.



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:63-64
+// CHECK-SAME: ){{$}}
+class E {
+public:
+  E();

I'd probably write these all as structs, if the class/struct distinction isn't 
relevant - saves an extra line of "public:".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

> What's the plan for this? Is it still in an experimental stage, with the 
> intent to investigate the types that are no longer emitted unedr the flag & 
> explain why they're missing (& either have a justification for why that's 
> acceptable, or work on additional heuristics to address the gaps?)?
> 
> If so, I'd probably rather this not be a full driver flag - if it's a 
> reliable way to reduce debug info size (like the existing heuristics under 
> -fstandalone-debug*) it should be rolled into -fno-standalone-debug behavior, 
> and if it's not fully fleshed out yet, I think an -Xclang flag would be more 
> suitable for the experimental phase.

Pretty much, I think the plan is to investigate further and maybe have more 
people try it. The -Xclang flag seems reasonable. Do you have thoughts on 
whether the added DebugInfoKind level makes sense?

> Do you have any sample of data on the sort of situations that lead to missing 
> types under this heuristic?

I built clang with the heuristic and linked to a list of missing types in the 
description but haven't really looked into them more specifically.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

What's the plan for this? Is it still in an experimental stage, with the intent 
to investigate the types that are no longer emitted unedr the flag & explain 
why they're missing (& either have a justification for why that's acceptable, 
or work on additional heuristics to address the gaps?)?

If so, I'd probably rather this not be a full driver flag - if it's a reliable 
way to reduce debug info size (like the existing heuristics under 
-fstandalone-debug*) it should be rolled into -fno-standalone-debug behavior, 
and if it's not fully fleshed out yet, I think an -Xclang flag would be more 
suitable for the experimental phase.

Do you have any sample of data on the sort of situations that lead to missing 
types under this heuristic?

- that's also another point, the -flimit-debug-info is sort of the "old" name, 
there was a bit of a lengthy discussion about what to support and how to 
support it and eventually settled on the "-fstandalone-debug" flag name to 
represent the design goal here: debug info emitted under the assumption that no 
other files have debug info, or that all other files have debug info. So by 
that approach, this new behavior seems to fit under the "-fno-standalone-debug" 
behavior, without a new flag, if it's a reliable heuristic - but I understand 
it may be helpful to iterate in-tree on the specific edge cases, etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4516
 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue ) 
{
-  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(CGM.getCodeGenOpts().isFullDebug());
   if (VD->hasAttr())

This change appears to be unnecessary?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72427/new/

https://reviews.llvm.org/D72427



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: rnk, dblaikie.
Herald added subscribers: cfe-commits, aprantl.
Herald added a project: clang.

This patch adds an option to limit debug info by only emitting complete class
type information when its constructor is emitted. This applies to classes
that have nontrivial user defined constructors.

I implemented the option by adding another level to `DebugInfoKind`, and
a flag `-flimit-debug-info-constructor`.

Total object file size on Windows, compiling with RelWithDebInfo:

  before: 4,257,448 kb
  after:  2,104,963 kb

And on Linux

  before: 9,225,140 kb
  after:  4,387,464 kb

According to the Windows clang.pdb files, here is a list of types that are no
longer complete with this option enabled: https://reviews.llvm.org/P8182


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72427

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DebugInfoOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/debug-info-limited-ctor.cpp

Index: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-limited-ctor.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang -flimit-debug-info-constructor -emit-llvm -g -S %s -o - | FileCheck %s
+
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+class A {
+public:
+  int z;
+};
+
+A *TestA(A* x) {
+  A *a = new A(*x);
+  return a;
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "B"
+// CHECK-SAME: flags: DIFlagFwdDecl
+class B {
+public:
+  int y;
+};
+
+extern int bar(B *b);
+int TestB(B *b) {
+  return bar(b);
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "C"
+// CHECK-SAME: flags: DIFlagFwdDecl
+class C {
+public:
+  C();
+};
+C *TestC() {
+  C *c = new C();
+  return c;
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "D"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+class D {
+public:
+  D() {}
+};
+D *TestD() {
+  D *d = new D();
+  return d;
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "E"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+class E {
+public:
+*d = new D();
+  return d;
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "E"
+// CHECK-NOT:  flags: DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+class E {
+public:
+  E();
+};
+E::E() {}
+E *TestE() {
+  E *e = new E();
+  return e;
+}
+
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -731,6 +731,7 @@
 llvm::StringSwitch(A->getValue())
 .Case("line-tables-only", codegenoptions::DebugLineTablesOnly)
 .Case("line-directives-only", codegenoptions::DebugDirectivesOnly)
+.Case("constructor", codegenoptions::DebugInfoConstructor)
 .Case("limited", codegenoptions::LimitedDebugInfo)
 .Case("standalone", codegenoptions::FullDebugInfo)
 .Default(~0U);
@@ -787,8 +788,7 @@
   llvm::Triple::arm, llvm::Triple::armeb};
 
   llvm::Triple T(TargetOpts.Triple);
-  if (Opts.OptimizationLevel > 0 &&
-  Opts.getDebugInfo() >= codegenoptions::LimitedDebugInfo &&
+  if (Opts.OptimizationLevel > 0 && Opts.isFullDebug() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -999,6 +999,9 @@
   case codegenoptions::DebugLineTablesOnly:
 CmdArgs.push_back("-debug-info-kind=line-tables-only");
 break;
+  case codegenoptions::DebugInfoConstructor:
+CmdArgs.push_back("-debug-info-kind=constructor");
+break;
   case codegenoptions::LimitedDebugInfo:
 CmdArgs.push_back("-debug-info-kind=limited");
 break;
@@ -3682,6 +3685,13 @@
   if (DebugInfoKind == codegenoptions::LimitedDebugInfo && NeedFullDebug)
 DebugInfoKind = codegenoptions::FullDebugInfo;
 
+  if (Args.hasFlag(options::OPT_flimit_debug_info_constructor,
+   options::OPT_fno_limit_debug_info_constructor, false))
+if (checkDebugInfoOption(
+