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

[akhuang]

Summary

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

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

4516

This change appears to be unnecessary?

[dblaikie]

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.

[akhuang]

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.

[dblaikie]

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.

clang/include/clang/Basic/CodeGenOptions.h

360–364 ↗

(On Diff #236933)

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.

clang/lib/CodeGen/CGDebugInfo.cpp

2225

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 &ctor")

clang/lib/Driver/ToolChains/Clang.cpp

3692 ↗

(On Diff #236933)

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)

clang/test/CodeGenCXX/debug-info-limited-ctor.cpp

24

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

36–37

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?

55–59

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

64–65

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

[akhuang]

clang/lib/CodeGen/CGDebugInfo.cpp

4516

Do you mean changing the DebugKind comparison?

clang/lib/Driver/ToolChains/Clang.cpp

3692 ↗

(On Diff #236933)

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?

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

4516

Yes, since the new kind is < Limited.

[dblaikie]

clang/lib/Driver/ToolChains/Clang.cpp

3692 ↗

(On Diff #236933)

Yeah, if that works, simple enough!

[akhuang]

clang/lib/CodeGen/CGDebugInfo.cpp

4516

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?

[akhuang]

Address comments

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

slightly less misleading

[Orlando]

clang/lib/CodeGen/CGDebugInfo.cpp

2225

Nit: Should ctor be Ctor here?

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

[dblaikie]

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.

clang/include/clang/Basic/CodeGenOptions.h

360–364 ↗

(On Diff #236933)

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)

clang/test/CodeGenCXX/debug-info-limited-ctor.cpp

10–16

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)

[rnk]

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<llvm::wasm::WasmElemSegment>: 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.

[akhuang]

Committed debug info kind refactoring bit in fe7cda2e.

[akhuang]

-Remove redundant test case
-Committed refactoring part

[dblaikie]

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<llvm::wasm::WasmElemSegment>: 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 problem - if you have a template with no member functions/static variables to instantiate is homed, but that seemed sufficiently narrow/uninteresting to me not to bother (or maybe I worked around/fixed that by not triggering the optimization if the explicitly instantiated type doesn't have anything to home)

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

[dblaikie]

Looks good - thanks!

clang/lib/CodeGen/CGDebugInfo.cpp

if (Method->hasPrototype())
     Flags |= llvm::DINode::FlagPrototyped;
   if (Method->getRefQualifier() == RQ_LValue)
     Flags |= llvm::DINode::FlagLValueReference;
   if (Method->getRefQualifier() == RQ_RValue)
     Flags |= llvm::DINode::FlagRValueReference;
   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<CXXConstructorDecl>(Method))
+      completeClass(CD->getParent());
+
   llvm::DINodeArray TParamsArray = CollectFunctionTemplateParams(Method, Unit);
   llvm::DISubprogram *SP = DBuilder.createMethod(
       RecordTy, MethodName, MethodLinkageName, MethodDefUnit, MethodLine,
       MethodTy, VIndex, ThisAdjustment, ContainingType, Flags, SPFlags,
       TParamsArray.get());
 
   SPCache[Method->getCanonicalDecl()].reset(SP);
 
static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
   // across DLL boundaries, so skip this optimization if the class or any of its
   // methods are marked dllimport. This isn't a complete solution, since objects
   // without any dllimport methods can be used in one DLL and constructed in
   // another, but it is the current behavior of LimitedDebugInfo.
   if (CXXDecl->hasDefinition() && CXXDecl->isDynamicClass() &&
       !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<ClassTemplateSpecializationDecl>(RD))
     Spec = SD->getSpecializationKind();
 
   if (Spec == TSK_ExplicitInstantiationDeclaration &&
       hasExplicitMemberDefinition(CXXDecl->method_begin(),
                                   CXXDecl->method_end()))
     return true;
GVE = DBuilder.createGlobalVariableExpression(
         getOrCreateStaticDataMemberDeclarationOrNull(D), TemplateParameters,
         Align);
     Var->addDebugInfo(GVE);
   }
   DeclCache[D->getCanonicalDecl()].reset(GVE);
 }
 
 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
   if (VD->hasAttr<NoDebugAttr>())
     return;
   llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
     std::string Name;
     llvm::raw_string_ostream OS(Name);
     VD->getNameForDiagnostic(OS, getPrintingPolicy(),
                              /*Qualified=*/true);
     return Name;

clang/test/CodeGenCXX/debug-info-limited-ctor.cpp

+// 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() {}