[llvm][DebugInfo] Backport DW_AT_default_value for template args

[Michael137]

Summary

Summary

Starting with DWARFv5, DW_AT_default_value can be used to indicate
that a template argument has a default value. With this patch LLVM
will emit the this attribute earlier versions of DWARF, unless
compiling with -gstrict-dwarf.

[dblaikie]

Looks good to me - maybe minor optional tweaks to the test.

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

37

Could consider using DWARF-DUMP-LABEL for the TAGs here (& maybe an extra DWARF-DUMP-LABEL: DW_TAG at the end) so that the attributes are constrained to be within the same tag? (could use DWARF-DUMP-DAG to test attributes in a way that's not order-specific, which would be nice - but I don't know that we do that much in the dwarfdump-driven tests, so not a huge deal)

41

NOT checks should be pretty general (so they don't accidentally pass despite minor differences-from-expectation) so maybe this should STRICT-NOT: DW_AT_default_value?

[Michael137]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

37

+1 for the DAG directive

Could you elaborate on the -LABEL though? IIUC the labels have to match a line in the file uniquely, which the DW_TAGs wouldn't

41

Makes sense!

[Michael137]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

37

Could label the DW_AT_name perhaps? Unless I'm misunderstanding

[Michael137]

• Use CHECK-DAG for attributes
• Also compile with non-strict DWARFv4
• Widen the STRICT-NOT checks

[probinson]

IIUC the labels have to match a line in the file uniquely, which the DW_TAGs wouldn't

CHECK-LABEL doesn't have to match a line in the file uniquely. What happens is that all the -LABEL directives are processed first, in order, subdividing the input text into regions. Then the non-LABEL directives are processed within their respective regions.

So you could have

CHECK-LABEL: DW_TAG
CHECK: DW_AT_name ("foo")
CHECK-LABEL: DW_TAG
CHECK: DW_AT_location
CHECK-LABEL: DW_TAG

which would search only the first tag for "foo" and only the second tag for the location attribute.

[probinson]

CHECK-LABEL doesn't have to match a line in the file uniquely.

I mean, it's good practice if they do match uniquely; that way you don't get excessively confusing results when the output changes, and things start matching where you didn't expect. But it's not a requirement.

[dblaikie]

I think this is good - with some check after the template list to constrain the dag/not checks to only be within the intended tag, not accidentally skip past into some other tag.

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

26

typo? (DUMPABEL)

37

Oh, yeah, unless the tests small enough to have unique/simple DW_TAGs you could -LABEL, this is probably all the reason why we don't DAG in dwarfdump tests usually.

41

Probably good to end this list of DAG/NOT with a DWARF-DUMP: {{DW_TAG|NULL}} to ensure these attribute matches don't happen to end up matching some other tag after the template parameters?

[Michael137]

• Fix label name
• Add check for end of block

[chapuni]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

7

llvm-dwarfdump fails if x86 is unconfigured.

[Michael137]

Ack, looking

[Michael137]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

7

I suppose we could just stop passing the triple explicitly

[aprantl]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

7

I think it would be better to check for the LLVM IR here, and add a second test that compiles that LLVM IR with llc in llvm/test/DebugInfo/X86

[aprantl]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

7

Generally, no clang fronted tests should depend on any target to be configured.

[Michael137]

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

7

https://reviews.llvm.org/D140084

clang/test/CodeGenCXX/debug-info-template-parameter.cpp

 // Test for DebugInfo for Defaulted parameters for C++ templates
 // Supported: -O0, standalone DI
 
 // RUN: %clang_cc1 -emit-llvm %std_cxx11-14 -dwarf-version=5 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,PRE17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=5 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,CXX17
 // RUN: %clang_cc1 -emit-llvm %std_cxx17- -dwarf-version=4 -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -o - | FileCheck %s --check-prefixes=CHECK,CXX17
+// RUN: %clang_cc1 %std_cxx17- -dwarf-version=4 -gstrict-dwarf -triple x86_64 %s -O0 -disable-llvm-passes -debug-info-kind=standalone -emit-obj -o - %s | llvm-dwarfdump --debug-info - | FileCheck %s --check-prefixes=DWARF-DUMP,STRICT
 
 // CHECK: DILocalVariable(name: "f1", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: ![[F1_TYPE:[0-9]+]]
 // CHECK: [[F1_TYPE]] = !{![[FIRST:[0-9]+]], ![[SECOND:[0-9]+]], ![[THIRD:[0-9]+]], ![[FORTH:[0-9]+]]}
 // CHECK: [[FIRST]] = !DITemplateTypeParameter(name: "T", type: !{{[0-9]*}})
 // CHECK: [[SECOND]] = !DITemplateValueParameter(name: "i", type: !{{[0-9]*}}, value: i32 6)
 // PRE17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, value: i8 0)
 // CXX17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, value: i1 false)
 
 // CHECK: DILocalVariable(name: "f2", {{.*}}, type: ![[TEMPLATE_TYPE:[0-9]+]]
 // CHECK: [[TEMPLATE_TYPE]] = {{.*}}!DICompositeType({{.*}}, templateParams: ![[F2_TYPE:[0-9]+]]
 // CHECK: [[F2_TYPE]] = !{![[FIRST:[0-9]+]], ![[SECOND:[0-9]+]], ![[THIRD:[0-9]+]], ![[FORTH:[0-9]+]]}
 // CHECK: [[FIRST]] = !DITemplateTypeParameter(name: "T", type: !{{[0-9]*}}, defaulted: true)
 // CHECK: [[SECOND]] = !DITemplateValueParameter(name: "i", type: !{{[0-9]*}}, defaulted: true, value: i32 3)
 // PRE17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, defaulted: true, value: i8 1)
 // CXX17: [[THIRD]] = !DITemplateValueParameter(name: "b", type: !{{[0-9]*}}, defaulted: true, value: i1 true)
 
+// DWARF-DUMP:     DW_TAG_class_type
+// DWARF-DUMP:       DW_AT_name      ("foo<char, 3, true, 1>")
+// DWARF-DUMP:       DW_TAG_template_type_parameter
+// DWARF-DUMP-NEXT:    DW_AT_type    ({{.*}} "char")
+// DWARF-DUMP-NEXT:    DW_AT_name    ("T")
+// DWARFv4-NEXT:       DW_AT_default_value   (true)
+// STRICT-NOT:         DW_AT_default_value   (true)
+// DWARF-DUMP:       DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:    DW_AT_type    ({{.*}} "int")
+// DWARF-DUMP-NEXT:    DW_AT_name    ("i")
+// DWARFv4-NEXT:       DW_AT_default_value   (true)
+// STRICT-NOT:         DW_AT_default_value   (true)
+// DWARF-DUMP:       DW_TAG_template_value_parameter
+// DWARF-DUMP-NEXT:    DW_AT_type    ({{.*}} "bool")
+// DWARF-DUMP-NEXT:    DW_AT_name    ("b")
+// DWARFv4-NEXT:       DW_AT_default_value   (true)
+// STRICT-NOT:         DW_AT_default_value   (true)
+
 template <typename T = char, int i = 3, bool b = true, int x = sizeof(T)>
 class foo {
 };
 
 int main() {
   foo<int, 6, false, 3> f1;
   foo<> f2;
   return 0;
 }

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h

private:
 
   /// If this is a named finished type then include it in the list of types for
   /// the accelerator tables.
   void updateAcceleratorTables(const DIScope *Context, const DIType *Ty,
                                const DIE &TyDIE);
 
   virtual bool isDwoUnit() const = 0;
   const MCSymbol *getCrossSectionRelativeBaseAddress() const override;
+
+  /// Returns 'true' if the current DwarfVersion is compatible
+  /// with the specified \p Version.
+  bool isCompatibleWithVersion(uint16_t Version) const;
 };
 
 class DwarfTypeUnit final : public DwarfUnit {
   uint64_t TypeSignature;
   const DIE *Ty;
   DwarfCompileUnit &CU;
   MCDwarfDwoLineTable *SplitLineTable;
   bool UsedLineTable = false;

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

void DwarfUnit::constructTemplateTypeParameterDIE(
     DIE &Buffer, const DITemplateTypeParameter *TP) {
   DIE &ParamDIE =
       createAndAddDIE(dwarf::DW_TAG_template_type_parameter, Buffer);
   // Add the type if it exists, it could be void and therefore no type.
   if (TP->getType())
     addType(ParamDIE, TP->getType());
   if (!TP->getName().empty())
     addString(ParamDIE, dwarf::DW_AT_name, TP->getName());
-  if (TP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (TP->isDefault() && isCompatibleWithVersion(5))
     addFlag(ParamDIE, dwarf::DW_AT_default_value);
 }
 
 void DwarfUnit::constructTemplateValueParameterDIE(
     DIE &Buffer, const DITemplateValueParameter *VP) {
   DIE &ParamDIE = createAndAddDIE(VP->getTag(), Buffer);
 
   // Add the type if there is one, template template and template parameter
   // packs will not have a type.
   if (VP->getTag() == dwarf::DW_TAG_template_value_parameter)
     addType(ParamDIE, VP->getType());
   if (!VP->getName().empty())
     addString(ParamDIE, dwarf::DW_AT_name, VP->getName());
-  if (VP->isDefault() && (DD->getDwarfVersion() >= 5))
+  if (VP->isDefault() && isCompatibleWithVersion(5))
     addFlag(ParamDIE, dwarf::DW_AT_default_value);
   if (Metadata *Val = VP->getValue()) {
     if (ConstantInt *CI = mdconst::dyn_extract<ConstantInt>(Val))
       addConstantValue(ParamDIE, CI, VP->getType());
     else if (GlobalValue *GV = mdconst::dyn_extract<GlobalValue>(Val)) {
       // We cannot describe the location of dllimport'd entities: the
       // computation of their address requires loads from the IAT.
       if (!GV->hasDLLImportStorageClass()) {
void DwarfUnit::addRnglistsBase() {
   addSectionLabel(getUnitDie(), dwarf::DW_AT_rnglists_base,
                   DU->getRnglistsTableBaseSym(),
                   TLOF.getDwarfRnglistsSection()->getBeginSymbol());
 }
 
 void DwarfTypeUnit::finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) {
   DD->getAddressPool().resetUsedFlag(true);
 }
+
+bool DwarfUnit::isCompatibleWithVersion(uint16_t Version) const {
+  return !Asm->TM.Options.DebugStrictDwarf || DD->getDwarfVersion() >= Version;
+}