[Clang] Emit DW_TAG_template_alias for template aliases #87623
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang-codegen Author: Orlando Cazalet-Hyams (OCHyams) ChangesFix issue #86529 Add front end flags -gtemplate-alias (also a cc1 flag) and -gno-template-alias to enable/disable usage of the feature. It's enabled by default in the front end for SCE debugger tuning only. GCC emits DW_TAG_typedef for template aliases, and GDB and LLDB appear not to support DW_TAG_template_alias. The -Xclang option -gsimple-template-names=mangled is treated the same as =full, which is not a regression from current behaviour for type aliases. Use DICompositeType to represnt the template alias, using its extraData field as a tuple of DITemplateParameter to describe the template parameters. New tests: Modified tests: Patch is 79.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87623.diff 22 Files Affected:
diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def
index 7cd3edf08a17ea..b94f6aef9ac60b 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -129,6 +129,9 @@ DEBUGOPT(CodeViewCommandLine, 1, 0)
/// Whether emit extra debug info for sample pgo profile collection.
DEBUGOPT(DebugInfoForProfiling, 1, 0)
+/// Whether to emit DW_TAG_template_alias for template aliases.
+DEBUGOPT(DebugTemplateAlias, 1, 0)
+
/// Whether to emit .debug_gnu_pubnames section instead of .debug_pubnames.
DEBUGOPT(DebugNameTable, 2, 0)
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index e33a1f4c45b949..862a6c72a769b2 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -393,6 +393,9 @@ def warn_ignored_clang_option : Warning<"the flag '%0' has been deprecated and w
def warn_drv_unsupported_opt_for_target : Warning<
"optimization flag '%0' is not supported for target '%1'">,
InGroup<IgnoredOptimizationArgument>;
+def warn_drv_dwarf_feature_requires_version : Warning<
+ "debug information option '%0' is not supported and will be ignored; requires at least DWARF-%1 but "
+ "-gdwarf-%2 has been specified">, InGroup<OptionIgnored>;
def warn_drv_unsupported_debug_info_opt_for_target : Warning<
"debug information option '%0' is not supported for target '%1'">,
InGroup<UnsupportedTargetOpt>;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 29066ea14280c2..ec95e3077dc15c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4273,6 +4273,8 @@ def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group<g_flags_Group>,
Values<"split,single">;
def gno_split_dwarf : Flag<["-"], "gno-split-dwarf">, Group<g_flags_Group>,
Visibility<[ClangOption, CLOption, DXCOption]>;
+def gtemplate_alias : Flag<["-"], "gtemplate-alias">, Group<g_flags_Group>, Visibility<[ClangOption, CC1Option]>;
+def gno_template_alias : Flag<["-"], "gno-template-alias">, Group<g_flags_Group>, Visibility<[ClangOption]>;
def gsimple_template_names : Flag<["-"], "gsimple-template-names">, Group<g_flags_Group>;
def gsimple_template_names_EQ
: Joined<["-"], "gsimple-template-names=">,
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 7453ed14aef414..1667f948db61d7 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1332,6 +1332,30 @@ llvm::DIType *CGDebugInfo::CreateType(const TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+ TemplateArgs Args = {TD->getTemplateParameters(), Ty->template_arguments()};
+
+ // FIXME: Respect DebugTemplateNameKind::Mangled, e.g. by using GetName.
+ // Note we can't use GetName without additional work: TypeAliasTemplateDecl
+ // doesn't have instantiation information, so
+ // TypeAliasTemplateDecl::getNameForDiagnostic wouldn't have access to the
+ // template args.
+ std::string Name;
+ llvm::raw_string_ostream OS(Name);
+ TD->getNameForDiagnostic(OS, PP, /*Qualified=*/false);
+ if (CGM.getCodeGenOpts().getDebugSimpleTemplateNames() !=
+ llvm::codegenoptions::DebugTemplateNamesKind::Simple ||
+ !HasReconstitutableArgs(Args.Args))
+ printTemplateArgumentList(OS, Args.Args, PP);
+
+ llvm::DIDerivedType *AliasTy = DBuilder.createTemplateAlias(
+ Src, Name, getOrCreateFile(Loc), getLineNumber(Loc),
+ getDeclContextDescriptor(AliasDecl), CollectTemplateParams(Args, Unit));
+ return AliasTy;
+ }
+
// Disable PrintCanonicalTypes here because we want
// the DW_AT_name to benefit from the TypePrinter's ability
// to skip defaulted template arguments.
@@ -1343,8 +1367,6 @@ llvm::DIType *CGDebugInfo::CreateType(const TemplateSpecializationType *Ty,
PP.PrintCanonicalTypes = false;
printTemplateArgumentList(OS, Ty->template_arguments(), PP,
TD->getTemplateParameters());
-
- SourceLocation Loc = AliasDecl->getLocation();
return DBuilder.createTypedef(Src, OS.str(), getOrCreateFile(Loc),
getLineNumber(Loc),
getDeclContextDescriptor(AliasDecl));
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ ArrayRef<TemplateArgument> Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument &TA) {
+ switch (TA.getKind()) {
+ case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
+ return true;
+ case TemplateArgument::Declaration:
+ // Reference and pointer non-type template parameters point to
+ // variables, functions, etc and their value is, at best (for
+ // variables) represented as an address - not a reference to the
+ // DWARF describing the variable/function/etc. This makes it hard,
+ // possibly impossible to rebuild the original name - looking up
+ // the address in the executable file's symbol table would be
+ // needed.
+ return false;
+ case TemplateArgument::NullPtr:
+ // These could be rebuilt, but figured they're close enough to the
+ // declaration case, and not worth rebuilding.
+ return false;
+ case TemplateArgument::Pack:
+ // A pack is invalid if any of the elements of the pack are
+ // invalid.
+ return HasReconstitutableArgs(TA.getPackAsArray());
+ case TemplateArgument::Integral:
+ // Larger integers get encoded as DWARF blocks which are a bit
+ // harder to parse back into a large integer, etc - so punting on
+ // this for now. Re-parsing the integers back into APInt is
+ // probably feasible some day.
+ return TA.getAsIntegral().getBitWidth() <= 64 &&
+ IsReconstitutableType(TA.getIntegralType());
+ case TemplateArgument::StructuralValue:
+ return false;
+ case TemplateArgument::Type:
+ return IsReconstitutableType(TA.getAsType());
+ case TemplateArgument::Expression:
+ return IsReconstitutableType(TA.getAsExpr()->getType());
+ default:
+ llvm_unreachable("Other, unresolved, template arguments should "
+ "not be seen here");
+ }
+ });
+}
+
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified,
+ const Type *Ty) const {
std::string Name;
llvm::raw_string_ostream OS(Name);
const NamedDecl *ND = dyn_cast<NamedDecl>(D);
@@ -5387,49 +5458,7 @@ std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
} else if (auto *VD = dyn_cast<VarDecl>(ND)) {
Args = GetTemplateArgs(VD);
}
- std::function<bool(ArrayRef<TemplateArgument>)> HasReconstitutableArgs =
- [&](ArrayRef<TemplateArgument> Args) {
- return llvm::all_of(Args, [&](const TemplateArgument &TA) {
- switch (TA.getKind()) {
- case TemplateArgument::Template:
- // Easy to reconstitute - the value of the parameter in the debug
- // info is the string name of the template. (so the template name
- // itself won't benefit from any name rebuilding, but that's a
- // representational limitation - maybe DWARF could be
- // changed/improved to use some more structural representation)
- return true;
- case TemplateArgument::Declaration:
- // Reference and pointer non-type template parameters point to
- // variables, functions, etc and their value is, at best (for
- // variables) represented as an address - not a reference to the
- // DWARF describing the variable/function/etc. This makes it hard,
- // possibly impossible to rebuild the original name - looking up the
- // address in the executable file's symbol table would be needed.
- return false;
- case TemplateArgument::NullPtr:
- // These could be rebuilt, but figured they're close enough to the
- // declaration case, and not worth rebuilding.
- return false;
- case TemplateArgument::Pack:
- // A pack is invalid if any of the elements of the pack are invalid.
- return HasReconstitutableArgs(TA.getPackAsArray());
- case TemplateArgument::Integral:
- // Larger integers get encoded as DWARF blocks which are a bit
- // harder to parse back into a large integer, etc - so punting on
- // this for now. Re-parsing the integers back into APInt is probably
- // feasible some day.
- return TA.getAsIntegral().getBitWidth() <= 64 &&
- IsReconstitutableType(TA.getIntegralType());
- case TemplateArgument::StructuralValue:
- return false;
- case TemplateArgument::Type:
- return IsReconstitutableType(TA.getAsType());
- default:
- llvm_unreachable("Other, unresolved, template arguments should "
- "not be seen here");
- }
- });
- };
+
// A conversion operator presents complications/ambiguity if there's a
// conversion to class template that is itself a template, eg:
// template<typename T>
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d060..56aef02f6288f8 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -626,7 +626,9 @@ class CGDebugInfo {
llvm::DIType *WrappedType;
};
- std::string GetName(const Decl*, bool Qualified = false) const;
+ bool HasReconstitutableArgs(ArrayRef<TemplateArgument> Args) const;
+ std::string GetName(const Decl *, bool Qualified = false,
+ const Type *Ty = nullptr) const;
/// Build up structure info for the byref. See \a BuildByRefType.
BlockByRefType EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 3bcacff7724c7d..fdc7ba241dd14d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4584,6 +4584,32 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
}
}
+ // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
+ const auto *DebugTemplateAlias = Args.getLastArg(
+ options::OPT_gtemplate_alias, options::OPT_gno_template_alias);
+ bool UseDebugTemplateAlias =
+ DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+ if (DebugTemplateAlias &&
+ checkDebugInfoOption(DebugTemplateAlias, Args, D, TC)) {
+ const auto &Opt = DebugTemplateAlias->getOption();
+ UseDebugTemplateAlias = Opt.matches(options::OPT_gtemplate_alias);
+ }
+ if (UseDebugTemplateAlias) {
+ // DW_TAG_template_alias is a DWARFv5 feature. Warn if we can't use it.
+ if (DebugTemplateAlias && RequestedDWARFVersion < 5)
+ D.Diag(diag::warn_drv_dwarf_feature_requires_version)
+ << DebugTemplateAlias->getAsString(Args) << 5
+ << RequestedDWARFVersion;
+ else if (DebugTemplateAlias && EffectiveDWARFVersion < 5)
+ // The toolchain has reduced allowed dwarf version, so we can't enable
+ // -gtemplate-alias.
+ D.Diag(diag::warn_drv_dwarf_version_limited_by_target)
+ << DebugTemplateAlias->getAsString(Args) << TC.getTripleString() << 5
+ << EffectiveDWARFVersion;
+ else
+ CmdArgs.push_back("-gtemplate-alias");
+ }
+
if (const Arg *A = Args.getLastArg(options::OPT_gsrc_hash_EQ)) {
StringRef v = A->getValue();
CmdArgs.push_back(Args.MakeArgString("-gsrc-hash=" + v));
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 7bd91d4791ecf0..e9b171f945bdb4 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1556,6 +1556,9 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
llvm::DICompileUnit::DebugNameTableKind::Default))
GenerateArg(Consumer, OPT_gpubnames);
+ if (Opts.DebugTemplateAlias)
+ GenerateArg(Consumer, OPT_gtemplate_alias);
+
auto TNK = Opts.getDebugSimpleTemplateNames();
if (TNK != llvm::codegenoptions::DebugTemplateNamesKind::Full) {
if (TNK == llvm::codegenoptions::DebugTemplateNamesKind::Simple)
@@ -1827,6 +1830,8 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
Opts.BinutilsVersion =
std::string(Args.getLastArgValue(OPT_fbinutils_version_EQ));
+ Opts.DebugTemplateAlias = Args.hasArg(OPT_gtemplate_alias);
+
Opts.DebugNameTable = static_cast<unsigned>(
Args.hasArg(OPT_ggnu_pubnames)
? llvm::DICompileUnit::DebugNameTableKind::GNU
diff --git a/clang/test/CodeGenCXX/debug-info-alias.cpp b/clang/test/CodeGenCXX/debug-info-alias.cpp
index 3d3f87ed1f6fa8..d87063e651e83d 100644
--- a/clang/test/CodeGenCXX/debug-info-alias.cpp
+++ b/clang/test/CodeGenCXX/debug-info-alias.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang -ggdb -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
template<typename T>
struct foo {
diff --git a/clang/test/CodeGenCXX/template-alias.cpp b/clang/test/CodeGenCXX/template-alias.cpp
new file mode 100644
index 00000000000000..a671f9820b408b
--- /dev/null
+++ b/clang/test/CodeGenCXX/template-alias.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s --check-prefixes=ALIAS-SIMPLE,ALIAS-ALL
+
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=mangled \
+// RUN: | FileCheck %s --check-prefixes=ALIAS-MANGLED,ALIAS-ALL
+
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone -gtemplate-alias %s \
+// RUN: | FileCheck %s --check-prefixes=ALIAS-FULL,ALIAS-ALL
+
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone %s \
+// RUN: | FileCheck %s --check-prefixes=TYPEDEF
+
+
+//// Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+//// template aliases, and that respects gsimple-template-names.
+////
+//// Test type and value template parameters.
+
+template<typename Y, int Z>
+struct X {
+ Y m1 = Z;
+};
+
+template<typename B, int C>
+using A = X<B, C>;
+
+A<int, 5> a;
+
+
+// ALIAS-SIMPLE: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// ALIAS-SIMPLE: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X",
+
+// FIXME: Mangled name is wrong (not a regression).
+// ALIAS-MANGLED: !DIDerivedType(tag: DW_TAG_template_alias, name: "A<int, 5>", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// ALIAS-MANGLED: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "_STN|X|<int, 5>",
+
+// ALIAS-FULL: !DIDerivedType(tag: DW_TAG_template_alias, name: "A<int, 5>", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// ALIAS-FULL: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X<int, 5>",
+
+// ALIAS-ALL: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// ALIAS-ALL: ![[extraData]] = !{![[B:[0-9]+]], ![[C:[0-9]+]]}
+// ALIAS-ALL: ![[B]] = !DITemplateTypeParameter(name: "B", type: ![[int]])
+// ALIAS-ALL: ![[C]] = !DITemplateValueParameter(name: "C", type: ![[int]], value: i32 5)
+
+// TYPEDEF: !DIDerivedType(tag: DW_TAG_typedef, name: "A<int, 5>", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]])
+// TYPEDEF: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X<int, 5>",
+// TYPEDEF: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
\ No newline at end of file
diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c
index e4809511ac91a0..1058a6f215aa97 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -465,3 +465,15 @@
// MANGLED_TEMP_NAMES: error: unknown argument '-gsimple-template-names=mangled'; did you mean '-Xclang -gsimple-template-names=mangled'
// RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck --check-prefix=FULL_TEMP_NAMES --implicit-check-not=debug-forward-template-params %s
// FULL_TEMP_NAMES-NOT: -gsimple-template-names
+
+//// Test -g[no-]template-alias (enabled by default with SCE debugger tuning). Requires DWARFv5.
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce -gtemplate-alias %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce -gno-template-alias %s 2>&1 | FileCheck %s --check-prefixes=NO-TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gtemplate-alias %s 2>&1 | FileCheck %s --check-prefixes="TEMPLATE-ALIAS,NO-TEMPLATE-ALIAS-WARN"
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gtemplate-alias -gno-template-alias %s 2>&1 | FileCheck %s --check-prefixes=NO-TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-4 -gtemplate-alias %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS-WARN
+// TEMPLATE-ALIAS: "-gtemplate-alias"
+// NO-TEMPLATE-ALIAS-NOT: "-gtemplate-alias"
+// TEMPLATE-ALIAS-WARN: warning: debug information option '-gtemplate-alias' is not supported and will be ignored; requires at least DWARF-5 but -gdwarf-4 has been specified
+// NO-TEMPLATE-ALIAS-WARN-NOT: warning:
diff --git a/core b/core
new file mode 100644
index 00000000000000..0605621c4e4483
Binary files /dev/null and b/core differ
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 002f2db6da5447..309f6b5fb7a49c 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -310,6 +310,23 @@ namespace llvm {
DINode::DIFlags Flags = DINode::FlagZero,
DINodeArray Annotations = nullptr);
+ /// Create debugging information entry for a typedef.
+ /// \param Ty Original type.
+ /// \param Name Typedef name.
+ /// \param File File where this type is defined.
+ /// \param LineNo Line number.
+ /// \param Context The surrounding context for the typedef.
+ /// \param TParams The template arguments.
+ /// \param AlignInBits Alignment. (optional)
+ /// \param Flags Flags to describe inheritance attribute, e.g. private
+ /// \param Annotations Annotations. (optional)
+ DIDerivedType *createTemplateAlias(DIType *Ty, StringRef Name, DIFile *File,
+ unsigned LineNo, DIScope *Context,
+ DINodeArray TParams,
+ uint32_t AlignInBits = 0,
+ DINode::DIFlags Flags = DINode::FlagZero,
+ DINodeArray Annotations = nullptr);
+
/// Create debugging information entry for a 'friend'.
DIDerivedType *createFriend(DIType *Ty, DIType *FriendTy);
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 2805f6c4780578..0470ed633274bd 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Orlando Cazalet-Hyams (OCHyams) ChangesFix issue #86529 Add front end flags -gtemplate-alias (also a cc1 flag) and -gno-template-alias to enable/disable usage of the feature. It's enabled by default in the front end for SCE debugger tuning only. GCC emits DW_TAG_typedef for template aliases, and GDB and LLDB appear not to support DW_TAG_template_alias. The -Xclang option -gsimple-template-names=mangled is treated the same as =full, which is not a regression from current behaviour for type aliases. Use DICompositeType to represnt the template alias, using its extraData field as a tuple of DITemplateParameter to describe the template parameters. New tests: Modified tests: Patch is 79.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87623.diff 22 Files Affected:
diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def
index 7cd3edf08a17ea..b94f6aef9ac60b 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -129,6 +129,9 @@ DEBUGOPT(CodeViewCommandLine, 1, 0)
/// Whether emit extra debug info for sample pgo profile collection.
DEBUGOPT(DebugInfoForProfiling, 1, 0)
+/// Whether to emit DW_TAG_template_alias for template aliases.
+DEBUGOPT(DebugTemplateAlias, 1, 0)
+
/// Whether to emit .debug_gnu_pubnames section instead of .debug_pubnames.
DEBUGOPT(DebugNameTable, 2, 0)
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index e33a1f4c45b949..862a6c72a769b2 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -393,6 +393,9 @@ def warn_ignored_clang_option : Warning<"the flag '%0' has been deprecated and w
def warn_drv_unsupported_opt_for_target : Warning<
"optimization flag '%0' is not supported for target '%1'">,
InGroup<IgnoredOptimizationArgument>;
+def warn_drv_dwarf_feature_requires_version : Warning<
+ "debug information option '%0' is not supported and will be ignored; requires at least DWARF-%1 but "
+ "-gdwarf-%2 has been specified">, InGroup<OptionIgnored>;
def warn_drv_unsupported_debug_info_opt_for_target : Warning<
"debug information option '%0' is not supported for target '%1'">,
InGroup<UnsupportedTargetOpt>;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 29066ea14280c2..ec95e3077dc15c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4273,6 +4273,8 @@ def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group<g_flags_Group>,
Values<"split,single">;
def gno_split_dwarf : Flag<["-"], "gno-split-dwarf">, Group<g_flags_Group>,
Visibility<[ClangOption, CLOption, DXCOption]>;
+def gtemplate_alias : Flag<["-"], "gtemplate-alias">, Group<g_flags_Group>, Visibility<[ClangOption, CC1Option]>;
+def gno_template_alias : Flag<["-"], "gno-template-alias">, Group<g_flags_Group>, Visibility<[ClangOption]>;
def gsimple_template_names : Flag<["-"], "gsimple-template-names">, Group<g_flags_Group>;
def gsimple_template_names_EQ
: Joined<["-"], "gsimple-template-names=">,
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 7453ed14aef414..1667f948db61d7 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1332,6 +1332,30 @@ llvm::DIType *CGDebugInfo::CreateType(const TemplateSpecializationType *Ty,
auto PP = getPrintingPolicy();
Ty->getTemplateName().print(OS, PP, TemplateName::Qualified::None);
+ SourceLocation Loc = AliasDecl->getLocation();
+
+ if (CGM.getCodeGenOpts().DebugTemplateAlias) {
+ TemplateArgs Args = {TD->getTemplateParameters(), Ty->template_arguments()};
+
+ // FIXME: Respect DebugTemplateNameKind::Mangled, e.g. by using GetName.
+ // Note we can't use GetName without additional work: TypeAliasTemplateDecl
+ // doesn't have instantiation information, so
+ // TypeAliasTemplateDecl::getNameForDiagnostic wouldn't have access to the
+ // template args.
+ std::string Name;
+ llvm::raw_string_ostream OS(Name);
+ TD->getNameForDiagnostic(OS, PP, /*Qualified=*/false);
+ if (CGM.getCodeGenOpts().getDebugSimpleTemplateNames() !=
+ llvm::codegenoptions::DebugTemplateNamesKind::Simple ||
+ !HasReconstitutableArgs(Args.Args))
+ printTemplateArgumentList(OS, Args.Args, PP);
+
+ llvm::DIDerivedType *AliasTy = DBuilder.createTemplateAlias(
+ Src, Name, getOrCreateFile(Loc), getLineNumber(Loc),
+ getDeclContextDescriptor(AliasDecl), CollectTemplateParams(Args, Unit));
+ return AliasTy;
+ }
+
// Disable PrintCanonicalTypes here because we want
// the DW_AT_name to benefit from the TypePrinter's ability
// to skip defaulted template arguments.
@@ -1343,8 +1367,6 @@ llvm::DIType *CGDebugInfo::CreateType(const TemplateSpecializationType *Ty,
PP.PrintCanonicalTypes = false;
printTemplateArgumentList(OS, Ty->template_arguments(), PP,
TD->getTemplateParameters());
-
- SourceLocation Loc = AliasDecl->getLocation();
return DBuilder.createTypedef(Src, OS.str(), getOrCreateFile(Loc),
getLineNumber(Loc),
getDeclContextDescriptor(AliasDecl));
@@ -5361,7 +5383,56 @@ static bool IsReconstitutableType(QualType QT) {
return T.Reconstitutable;
}
-std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+bool CGDebugInfo::HasReconstitutableArgs(
+ ArrayRef<TemplateArgument> Args) const {
+ return llvm::all_of(Args, [&](const TemplateArgument &TA) {
+ switch (TA.getKind()) {
+ case TemplateArgument::Template:
+ // Easy to reconstitute - the value of the parameter in the debug
+ // info is the string name of the template. (so the template name
+ // itself won't benefit from any name rebuilding, but that's a
+ // representational limitation - maybe DWARF could be
+ // changed/improved to use some more structural representation)
+ return true;
+ case TemplateArgument::Declaration:
+ // Reference and pointer non-type template parameters point to
+ // variables, functions, etc and their value is, at best (for
+ // variables) represented as an address - not a reference to the
+ // DWARF describing the variable/function/etc. This makes it hard,
+ // possibly impossible to rebuild the original name - looking up
+ // the address in the executable file's symbol table would be
+ // needed.
+ return false;
+ case TemplateArgument::NullPtr:
+ // These could be rebuilt, but figured they're close enough to the
+ // declaration case, and not worth rebuilding.
+ return false;
+ case TemplateArgument::Pack:
+ // A pack is invalid if any of the elements of the pack are
+ // invalid.
+ return HasReconstitutableArgs(TA.getPackAsArray());
+ case TemplateArgument::Integral:
+ // Larger integers get encoded as DWARF blocks which are a bit
+ // harder to parse back into a large integer, etc - so punting on
+ // this for now. Re-parsing the integers back into APInt is
+ // probably feasible some day.
+ return TA.getAsIntegral().getBitWidth() <= 64 &&
+ IsReconstitutableType(TA.getIntegralType());
+ case TemplateArgument::StructuralValue:
+ return false;
+ case TemplateArgument::Type:
+ return IsReconstitutableType(TA.getAsType());
+ case TemplateArgument::Expression:
+ return IsReconstitutableType(TA.getAsExpr()->getType());
+ default:
+ llvm_unreachable("Other, unresolved, template arguments should "
+ "not be seen here");
+ }
+ });
+}
+
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified,
+ const Type *Ty) const {
std::string Name;
llvm::raw_string_ostream OS(Name);
const NamedDecl *ND = dyn_cast<NamedDecl>(D);
@@ -5387,49 +5458,7 @@ std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
} else if (auto *VD = dyn_cast<VarDecl>(ND)) {
Args = GetTemplateArgs(VD);
}
- std::function<bool(ArrayRef<TemplateArgument>)> HasReconstitutableArgs =
- [&](ArrayRef<TemplateArgument> Args) {
- return llvm::all_of(Args, [&](const TemplateArgument &TA) {
- switch (TA.getKind()) {
- case TemplateArgument::Template:
- // Easy to reconstitute - the value of the parameter in the debug
- // info is the string name of the template. (so the template name
- // itself won't benefit from any name rebuilding, but that's a
- // representational limitation - maybe DWARF could be
- // changed/improved to use some more structural representation)
- return true;
- case TemplateArgument::Declaration:
- // Reference and pointer non-type template parameters point to
- // variables, functions, etc and their value is, at best (for
- // variables) represented as an address - not a reference to the
- // DWARF describing the variable/function/etc. This makes it hard,
- // possibly impossible to rebuild the original name - looking up the
- // address in the executable file's symbol table would be needed.
- return false;
- case TemplateArgument::NullPtr:
- // These could be rebuilt, but figured they're close enough to the
- // declaration case, and not worth rebuilding.
- return false;
- case TemplateArgument::Pack:
- // A pack is invalid if any of the elements of the pack are invalid.
- return HasReconstitutableArgs(TA.getPackAsArray());
- case TemplateArgument::Integral:
- // Larger integers get encoded as DWARF blocks which are a bit
- // harder to parse back into a large integer, etc - so punting on
- // this for now. Re-parsing the integers back into APInt is probably
- // feasible some day.
- return TA.getAsIntegral().getBitWidth() <= 64 &&
- IsReconstitutableType(TA.getIntegralType());
- case TemplateArgument::StructuralValue:
- return false;
- case TemplateArgument::Type:
- return IsReconstitutableType(TA.getAsType());
- default:
- llvm_unreachable("Other, unresolved, template arguments should "
- "not be seen here");
- }
- });
- };
+
// A conversion operator presents complications/ambiguity if there's a
// conversion to class template that is itself a template, eg:
// template<typename T>
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d060..56aef02f6288f8 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -626,7 +626,9 @@ class CGDebugInfo {
llvm::DIType *WrappedType;
};
- std::string GetName(const Decl*, bool Qualified = false) const;
+ bool HasReconstitutableArgs(ArrayRef<TemplateArgument> Args) const;
+ std::string GetName(const Decl *, bool Qualified = false,
+ const Type *Ty = nullptr) const;
/// Build up structure info for the byref. See \a BuildByRefType.
BlockByRefType EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 3bcacff7724c7d..fdc7ba241dd14d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4584,6 +4584,32 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
}
}
+ // Emit DW_TAG_template_alias for template aliases? True by default for SCE.
+ const auto *DebugTemplateAlias = Args.getLastArg(
+ options::OPT_gtemplate_alias, options::OPT_gno_template_alias);
+ bool UseDebugTemplateAlias =
+ DebuggerTuning == llvm::DebuggerKind::SCE && RequestedDWARFVersion >= 5;
+ if (DebugTemplateAlias &&
+ checkDebugInfoOption(DebugTemplateAlias, Args, D, TC)) {
+ const auto &Opt = DebugTemplateAlias->getOption();
+ UseDebugTemplateAlias = Opt.matches(options::OPT_gtemplate_alias);
+ }
+ if (UseDebugTemplateAlias) {
+ // DW_TAG_template_alias is a DWARFv5 feature. Warn if we can't use it.
+ if (DebugTemplateAlias && RequestedDWARFVersion < 5)
+ D.Diag(diag::warn_drv_dwarf_feature_requires_version)
+ << DebugTemplateAlias->getAsString(Args) << 5
+ << RequestedDWARFVersion;
+ else if (DebugTemplateAlias && EffectiveDWARFVersion < 5)
+ // The toolchain has reduced allowed dwarf version, so we can't enable
+ // -gtemplate-alias.
+ D.Diag(diag::warn_drv_dwarf_version_limited_by_target)
+ << DebugTemplateAlias->getAsString(Args) << TC.getTripleString() << 5
+ << EffectiveDWARFVersion;
+ else
+ CmdArgs.push_back("-gtemplate-alias");
+ }
+
if (const Arg *A = Args.getLastArg(options::OPT_gsrc_hash_EQ)) {
StringRef v = A->getValue();
CmdArgs.push_back(Args.MakeArgString("-gsrc-hash=" + v));
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 7bd91d4791ecf0..e9b171f945bdb4 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1556,6 +1556,9 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
llvm::DICompileUnit::DebugNameTableKind::Default))
GenerateArg(Consumer, OPT_gpubnames);
+ if (Opts.DebugTemplateAlias)
+ GenerateArg(Consumer, OPT_gtemplate_alias);
+
auto TNK = Opts.getDebugSimpleTemplateNames();
if (TNK != llvm::codegenoptions::DebugTemplateNamesKind::Full) {
if (TNK == llvm::codegenoptions::DebugTemplateNamesKind::Simple)
@@ -1827,6 +1830,8 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
Opts.BinutilsVersion =
std::string(Args.getLastArgValue(OPT_fbinutils_version_EQ));
+ Opts.DebugTemplateAlias = Args.hasArg(OPT_gtemplate_alias);
+
Opts.DebugNameTable = static_cast<unsigned>(
Args.hasArg(OPT_ggnu_pubnames)
? llvm::DICompileUnit::DebugNameTableKind::GNU
diff --git a/clang/test/CodeGenCXX/debug-info-alias.cpp b/clang/test/CodeGenCXX/debug-info-alias.cpp
index 3d3f87ed1f6fa8..d87063e651e83d 100644
--- a/clang/test/CodeGenCXX/debug-info-alias.cpp
+++ b/clang/test/CodeGenCXX/debug-info-alias.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang -ggdb -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
template<typename T>
struct foo {
diff --git a/clang/test/CodeGenCXX/template-alias.cpp b/clang/test/CodeGenCXX/template-alias.cpp
new file mode 100644
index 00000000000000..a671f9820b408b
--- /dev/null
+++ b/clang/test/CodeGenCXX/template-alias.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=simple \
+// RUN: | FileCheck %s --check-prefixes=ALIAS-SIMPLE,ALIAS-ALL
+
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone -gtemplate-alias %s -gsimple-template-names=mangled \
+// RUN: | FileCheck %s --check-prefixes=ALIAS-MANGLED,ALIAS-ALL
+
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone -gtemplate-alias %s \
+// RUN: | FileCheck %s --check-prefixes=ALIAS-FULL,ALIAS-ALL
+
+// RUN: %clang_cc1 -triple x86_64-unk-unk -o - -emit-llvm -debug-info-kind=standalone %s \
+// RUN: | FileCheck %s --check-prefixes=TYPEDEF
+
+
+//// Check that -gtemplate-alias causes DW_TAG_template_alias emission for
+//// template aliases, and that respects gsimple-template-names.
+////
+//// Test type and value template parameters.
+
+template<typename Y, int Z>
+struct X {
+ Y m1 = Z;
+};
+
+template<typename B, int C>
+using A = X<B, C>;
+
+A<int, 5> a;
+
+
+// ALIAS-SIMPLE: !DIDerivedType(tag: DW_TAG_template_alias, name: "A", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// ALIAS-SIMPLE: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X",
+
+// FIXME: Mangled name is wrong (not a regression).
+// ALIAS-MANGLED: !DIDerivedType(tag: DW_TAG_template_alias, name: "A<int, 5>", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// ALIAS-MANGLED: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "_STN|X|<int, 5>",
+
+// ALIAS-FULL: !DIDerivedType(tag: DW_TAG_template_alias, name: "A<int, 5>", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]], extraData: ![[extraData:[0-9]+]])
+// ALIAS-FULL: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X<int, 5>",
+
+// ALIAS-ALL: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// ALIAS-ALL: ![[extraData]] = !{![[B:[0-9]+]], ![[C:[0-9]+]]}
+// ALIAS-ALL: ![[B]] = !DITemplateTypeParameter(name: "B", type: ![[int]])
+// ALIAS-ALL: ![[C]] = !DITemplateValueParameter(name: "C", type: ![[int]], value: i32 5)
+
+// TYPEDEF: !DIDerivedType(tag: DW_TAG_typedef, name: "A<int, 5>", file: ![[#]], line: [[#]], baseType: ![[baseType:[0-9]+]])
+// TYPEDEF: ![[baseType]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X<int, 5>",
+// TYPEDEF: ![[int:[0-9]+]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
\ No newline at end of file
diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c
index e4809511ac91a0..1058a6f215aa97 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -465,3 +465,15 @@
// MANGLED_TEMP_NAMES: error: unknown argument '-gsimple-template-names=mangled'; did you mean '-Xclang -gsimple-template-names=mangled'
// RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck --check-prefix=FULL_TEMP_NAMES --implicit-check-not=debug-forward-template-params %s
// FULL_TEMP_NAMES-NOT: -gsimple-template-names
+
+//// Test -g[no-]template-alias (enabled by default with SCE debugger tuning). Requires DWARFv5.
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce -gtemplate-alias %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce -gno-template-alias %s 2>&1 | FileCheck %s --check-prefixes=NO-TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gtemplate-alias %s 2>&1 | FileCheck %s --check-prefixes="TEMPLATE-ALIAS,NO-TEMPLATE-ALIAS-WARN"
+// RUN: %clang -### -target x86_64 -c -gdwarf-5 -gtemplate-alias -gno-template-alias %s 2>&1 | FileCheck %s --check-prefixes=NO-TEMPLATE-ALIAS
+// RUN: %clang -### -target x86_64 -c -gdwarf-4 -gtemplate-alias %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS-WARN
+// TEMPLATE-ALIAS: "-gtemplate-alias"
+// NO-TEMPLATE-ALIAS-NOT: "-gtemplate-alias"
+// TEMPLATE-ALIAS-WARN: warning: debug information option '-gtemplate-alias' is not supported and will be ignored; requires at least DWARF-5 but -gdwarf-4 has been specified
+// NO-TEMPLATE-ALIAS-WARN-NOT: warning:
diff --git a/core b/core
new file mode 100644
index 00000000000000..0605621c4e4483
Binary files /dev/null and b/core differ
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 002f2db6da5447..309f6b5fb7a49c 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -310,6 +310,23 @@ namespace llvm {
DINode::DIFlags Flags = DINode::FlagZero,
DINodeArray Annotations = nullptr);
+ /// Create debugging information entry for a typedef.
+ /// \param Ty Original type.
+ /// \param Name Typedef name.
+ /// \param File File where this type is defined.
+ /// \param LineNo Line number.
+ /// \param Context The surrounding context for the typedef.
+ /// \param TParams The template arguments.
+ /// \param AlignInBits Alignment. (optional)
+ /// \param Flags Flags to describe inheritance attribute, e.g. private
+ /// \param Annotations Annotations. (optional)
+ DIDerivedType *createTemplateAlias(DIType *Ty, StringRef Name, DIFile *File,
+ unsigned LineNo, DIScope *Context,
+ DINodeArray TParams,
+ uint32_t AlignInBits = 0,
+ DINode::DIFlags Flags = DINode::FlagZero,
+ DINodeArray Annotations = nullptr);
+
/// Create debugging information entry for a 'friend'.
DIDerivedType *createFriend(DIType *Ty, DIType *FriendTy);
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 2805f6c4780578..0470ed633274bd 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Quick initial pass.
Converting frame-types.s from v4 to v5 should be done as a separate commit. Then adding the alias would be a simpler change to review. (Is symbolizer really the right place to be testing this, anyway?)
I'm a little uncomfortable with adding a new user-facing option for template aliases. Even with that in place, we should not warn and refuse to do what the user asked for, based on DWARF version. -gdwarf-2 -gsplit-dwarf generates a .dwo file claiming to be v2, for example.
core
Outdated
llvm/include/llvm/IR/DIBuilder.h
Outdated
| /// \param Context The surrounding context for the typedef. | ||
| /// \param TParams The template arguments. | ||
| /// \param AlignInBits Alignment. (optional) | ||
| /// \param Flags Flags to describe inheritance attribute, e.g. private |
| Tag != dwarf::DW_TAG_restrict_type && Tag != dwarf::DW_TAG_atomic_type && | ||
| Tag != dwarf::DW_TAG_immutable_type) | ||
| Tag != dwarf::DW_TAG_immutable_type && | ||
| Tag != dwarf::DW_TAG_template_alias) |
There was a problem hiding this comment.
Looks like this got added twice?
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
| // info is the string name of the template. (so the template name | ||
| // itself won't benefit from any name rebuilding, but that's a | ||
| // representational limitation - maybe DWARF could be | ||
| // changed/improved to use some more structural representation) |
There was a problem hiding this comment.
The sentence inside the parens should be a proper sentence.
There was a problem hiding this comment.
These are existing comments that I've just translocated - this block has been copied from line 5390, below. I'm happy to make your suggested change, I just wanted to add this context first in case it changes your view?
There was a problem hiding this comment.
Ah, didn't notice it was a copy-paste. But it would still be nice to have the comment follow our style guide. So, up to you.
There was a problem hiding this comment.
👍 Promoted the sentence out of parens.
Much appreciated!
Makes sense, I can do that.
I modified frame-types.s here because
I don't have strong feelings about the flag setup and am happy to change it. @dwblaikie mentioned that we might not want to have the debugger tuning be the only way of controlling whether or not we emit this DIE, so adding |
| ; PUB-TYPES: "A" | ||
|
|
||
|
|
||
| target triple = "x86_64-unk-unk" |
There was a problem hiding this comment.
x86_64-unknown-unkown
Thanks! @pogo59 said:
I've split that into two commits in this PR: upgrade to v5: 18446f2, add template alias: 49d6642. It does improve readability a bit but its still quite noisy. I am happy to commit the former separately then rebase this PR, if you'd like? |
| @@ -1,4 +1,4 @@ | |||
| // RUN: %clang -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s | |||
| // RUN: %clang -ggdb -std=c++11 -S -emit-llvm %s -o - | FileCheck %s | |||
There was a problem hiding this comment.
Do we now lose a tiny bit of coverage for -glldb? I assume this change is to make sure the test passes on platforms that are tuned for sce? Maybe we should just add -gno-template-alias instead?
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
| } | ||
|
|
||
| std::string CGDebugInfo::GetName(const Decl *D, bool Qualified, | ||
| const Type *Ty) const { |
There was a problem hiding this comment.
Are we using this extra Ty parameter anywhere? I assume your original intent was to pass the TemplateSpecializationType into GetName and get the template arguments for the alias decl that way? Any reason you didn't end up doing that?
There was a problem hiding this comment.
Aha, good catch. You're correct on both counts - it's not used in this patch and that was one of the approaches I tried out.
This is discussed a bit on the issue starting here #54624 (comment).
IIRC, we could pass in the TemplateSpecializationType to get the template arguments for this case, but I think we'd need to introduce a special case for building the name where we can't use D's getNameForDiagnostic on line 5461 and 5481 of the original file.
My memory is slightly blurred by the conference - I think I was erring on the side of caution given my unfamiliarity with the code, preferring to have a special case localised at the usage site rather than inside a utility function. I'd be happy to change the patch to use the TemplateSpecializationType parameter if you'd like to see what it looks like?
There was a problem hiding this comment.
Ah thanks for the additional context. Keeping it as is seems fine to me then. I don't see a better way to get to template arguments from the alias decl. My initial thought of special-casing this in GetName isn't necessarily great either
@pogo59 Hmm - I thought you held a strong preference that debugger tuning never be the only way to access a certain feature, and that we should always have direct control of these features via flags (but with default (or explicit) debugger tuning setting some of these flag defaults based on the preferred content for that debugger). Perhaps i'm misremembering? |
|
Re: code: Looks about right. Bit unfortunate to store template parameters in different ways (in the |
You are not misremembering. Ideally tuning is not gating (although we probably never expressed it that way). We probably have not fully held to that ideal, but if we're going to have knobs to tweak we should implement the knobs properly. I will be making another pass over this PR later today and pay attention to that. |
|
Thanks @dwblaikie
I hadn't actually tried using DICompsiteType. I didn't choose DIDerivedType for any particularly principled reasons: typedefs use derived types which made it very easy to find places that needed updating, and an alias "felt" more like a derived type than composite type. Having looked only briefly, I don't think it'd be any more invasive to use composite types instead (not 100% sure without diving in). I'm happy to give that a go if that is your preference? I've just found an input that causes an assertion failure in There's a mismatch between the template parameter list and template argument list sizes, created here: I notice that we emit |
Eh, I guess if we've already got DIGlobalVariable having template params, we aren't going to unify all 3 cases - not sure if unifying 2 out of three is particularly valuable, especially if it ends up with a different conflict (that alias templates look less like aliases & so need special casing in that direction) But perhaps at least you could add named accessors to DIDerivedType for the template params, same as DIGlobalVariable/DICompositeType have? (oh, and in case anyone hasn't mentioned it already - this would generally be committed in smaller pieces upstream - adding the LLVM functionality first, then adding clang patches that use that functionality)
yeah, probably worth checking the APIs that we use for building the template args for classes and variable templates, they probably have some handling for this - would be good to share/reuse that code, rather than duplicating it, once you find it |
| << EffectiveDWARFVersion; | ||
| else | ||
| CmdArgs.push_back("-gtemplate-alias"); | ||
| } |
There was a problem hiding this comment.
Please take out the diags here, if someone explicitly asks for something in debug info we do it.
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
| // info is the string name of the template. (so the template name | ||
| // itself won't benefit from any name rebuilding, but that's a | ||
| // representational limitation - maybe DWARF could be | ||
| // changed/improved to use some more structural representation) |
There was a problem hiding this comment.
Ah, didn't notice it was a copy-paste. But it would still be nice to have the comment follow our style guide. So, up to you.
| @@ -465,3 +465,15 @@ | |||
| // MANGLED_TEMP_NAMES: error: unknown argument '-gsimple-template-names=mangled'; did you mean '-Xclang -gsimple-template-names=mangled' | |||
| // RUN: %clang -### -target x86_64 -c -g %s 2>&1 | FileCheck --check-prefix=FULL_TEMP_NAMES --implicit-check-not=debug-forward-template-params %s | |||
| // FULL_TEMP_NAMES-NOT: -gsimple-template-names | |||
|
|
|||
| //// Test -g[no-]template-alias (enabled by default with SCE debugger tuning). Requires DWARFv5. | |||
| // RUN: %clang -### -target x86_64 -c -gdwarf-5 -gsce %s 2>&1 | FileCheck %s --check-prefixes=TEMPLATE-ALIAS | |||
There was a problem hiding this comment.
This gave clang: error: unknown argument '--target'; did you mean '-target'?
There was a problem hiding this comment.
Huh Coulda sworn that was changed... okay.
Done. I couldn't add an assert (
Sure, no problem, I'll open separate pull requests once this has been accepted then. I think I've addressed all the concerns raised now except for the variadic issue I mentioned, which I'll look at tomorrow. |
I didn't find anything that does what (I think) we need here (but I'm not familiar with the FE, so might've just missed it if it exists. Not for lack of trying). The code I've added looks a bit ugly, but I'm not sure what else to do. See https://godbolt.org/z/nTK4bW1Yn - there doesn't seem to be a @dwblaikie are you happy with the latest addition mentioned above (plus test) - that's the commits dc5a611 and c9c000f? @pogo59 are you happy with the flag situation now? I've separated the LLVM IR side into #88943. I can rebase this to repurpose it for the Clang side once that lands. |
…ault parameter values
OCHyams
force-pushed
the
upstream-template-alias
branch
from
85c8d37 to
0952cf9
Compare
|
I've rebased this so now this pull request contains only the Clang changes (LLVM side is here #88943). There's a discussion starting here that discusses whether or not we should include defaulted arguments in the list. The very short summary is that I tried this by scraping the defaults from the template parameter list, but it turns out this is difficult in the face of parameter defaults that are dependent types and values. So the current implementation ignores defaulted arguments, i.e., doesn't include them in the argument list (and as a consequence also omits empty parameter packs that come after defaulted arguments). This is not ideal, but not a regression from the DW_TAG_typedef names for template aliases which also do not include defaulted arg values. |
| // CHECK: ![[NonDefault]] = !DITemplateTypeParameter(name: "NonDefault", type: ![[int]]) | ||
|
|
||
| //// FIXME: Ideally, we would describe the deafulted args, like this: | ||
| // : ![[extraData]] = !{![[NonDefault:[0-9]+]], ![[T:[0-9]+]], ![[I:[0-9]+]], ![[Ts:[0-9]+]]} |
There was a problem hiding this comment.
Minor: Is it worth turning these into CHECK-NOT? So when this is fixed in the future we remember that we have these tests ready
There was a problem hiding this comment.
I think the test will already start failing if this gets fixed because the current extraData metadata tuple contents check will fail (it'll have more than one element). I'm still happy to add CHECK-NOTs if you'd like, just thought I'd raise this in case it changes your stance.
There was a problem hiding this comment.
ah right, that makes sense, happy keeping it as is
There was a problem hiding this comment.
LGTM, if @pogo59 and @dwblaikie are happy with the driver changes
|
Yes, driver part LGTM. |
|
Thanks everyone. I'll land this now then to see what the bots have to say about it. |
|
Dependent expressions strike again - https://godbolt.org/z/W381837vr
The issue is Looking into this now (will revert if I can't find a solution). |
|
@Michael137 said:
There's a couple of switches in llvm/lib/DWARFLinker that look like they want a DW_TAG_template_alias: |
Yup, dsymutil looks good now, thanks
Good question, I'm not familiar with these either, but would make sense to address eventually. Though I don't think it's urgent atm since |
Fix issue #54624
Add front end flags -gtemplate-alias (also a cc1 flag) and -gno-template-alias to enable/disable usage of the feature. It's enabled by default in the front end for SCE debugger tuning only. GCC emits DW_TAG_typedef for template aliases, and GDB and LLDB appear not to support DW_TAG_template_alias.
The -Xclang option -gsimple-template-names=mangled is treated the same as =full, which is not a regression from current behaviour for type aliases.
Use DICompositeType to represnt the template alias, using its extraData field as a tuple of DITemplateParameter to describe the template parameters.
New tests:
Modified tests: