[clang][DebugInfo] Improve heuristic to determine whether to evaluate a static variable's initializer #72974
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-clang-codegen Author: Michael Buch (Michael137) ChangesThis patch extracts the logic to evaluate a C++ static data-member's constant initializer such that it can be used by an upcoming patch. It also makes the check for whether we are dealing with a constant initializer more robust/idiomatic, which revealed a bug in the Full diff: https://github.com/llvm/llvm-project/pull/72974.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 0b52d99ad07f164..4c7c7d68b4271e3 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -69,6 +69,29 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) {
return D->hasAttr<AlignedAttr>() ? D->getMaxAlignment() : 0;
}
+/// Given a VarDecl corresponding to either the definition or
+/// declaration of a C++ static data member, if it has a constant
+/// initializer and is evaluatable, return the evaluated value.
+/// Returns std::nullopt otherwise.
+static std::optional<APValue>
+evaluateConstantInitializer(const clang::VarDecl *VD,
+ const clang::ASTContext &Ctx) {
+ assert(VD != nullptr);
+
+ if (!VD->isStaticDataMember())
+ return std::nullopt;
+
+ if (!VD->isUsableInConstantExpressions(Ctx))
+ return std::nullopt;
+
+ auto const *InitExpr = VD->getAnyInitializer();
+ Expr::EvalResult Result;
+ if (!InitExpr->EvaluateAsConstantExpr(Result, Ctx))
+ return std::nullopt;
+
+ return Result.Val;
+}
+
CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
: CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
@@ -5596,14 +5619,11 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
if (VD->hasAttr<NoDebugAttr>())
return;
- if (!VD->hasInit())
- return;
-
const auto CacheIt = DeclCache.find(VD);
if (CacheIt != DeclCache.end())
return;
- auto const *InitVal = VD->evaluateValue();
+ const auto InitVal = evaluateConstantInitializer(VD, CGM.getContext());
if (!InitVal)
return;
diff --git a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
index f2d4d9408a8297a..6ba98373d33ada8 100644
--- a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
+++ b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
@@ -67,10 +67,6 @@ int main() {
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:
-// CHECK: ![[IENUM_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "inline_enum",
-// CHECK-SAME: flags: DIFlagStaticMember
-// CHECK-NOT: extraData:
-
// CHECK: ![[EMPTY_TEMPLATED_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "empty_templated",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:
@@ -98,11 +94,6 @@ int main() {
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[ENUM_DECL]])
-// CHECK: !DIGlobalVariableExpression(var: ![[IENUM_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
-// CHECK: ![[IENUM_VAR]] = distinct !DIGlobalVariable(name: "inline_enum"
-// CHECK-NOT: linkageName:
-// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[IENUM_DECL]])
-
// CHECK: !DIGlobalVariableExpression(var: ![[EMPTY_TEMPLATED_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
// CHECK: ![[EMPTY_TEMPLATED_VAR]] = distinct !DIGlobalVariable(name: "empty_templated"
// CHECK-NOT: linkageName:
|
|
@llvm/pr-subscribers-clang Author: Michael Buch (Michael137) ChangesThis patch extracts the logic to evaluate a C++ static data-member's constant initializer such that it can be used by an upcoming patch. It also makes the check for whether we are dealing with a constant initializer more robust/idiomatic, which revealed a bug in the Full diff: https://github.com/llvm/llvm-project/pull/72974.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 0b52d99ad07f164..4c7c7d68b4271e3 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -69,6 +69,29 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) {
return D->hasAttr<AlignedAttr>() ? D->getMaxAlignment() : 0;
}
+/// Given a VarDecl corresponding to either the definition or
+/// declaration of a C++ static data member, if it has a constant
+/// initializer and is evaluatable, return the evaluated value.
+/// Returns std::nullopt otherwise.
+static std::optional<APValue>
+evaluateConstantInitializer(const clang::VarDecl *VD,
+ const clang::ASTContext &Ctx) {
+ assert(VD != nullptr);
+
+ if (!VD->isStaticDataMember())
+ return std::nullopt;
+
+ if (!VD->isUsableInConstantExpressions(Ctx))
+ return std::nullopt;
+
+ auto const *InitExpr = VD->getAnyInitializer();
+ Expr::EvalResult Result;
+ if (!InitExpr->EvaluateAsConstantExpr(Result, Ctx))
+ return std::nullopt;
+
+ return Result.Val;
+}
+
CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
: CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
@@ -5596,14 +5619,11 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
if (VD->hasAttr<NoDebugAttr>())
return;
- if (!VD->hasInit())
- return;
-
const auto CacheIt = DeclCache.find(VD);
if (CacheIt != DeclCache.end())
return;
- auto const *InitVal = VD->evaluateValue();
+ const auto InitVal = evaluateConstantInitializer(VD, CGM.getContext());
if (!InitVal)
return;
diff --git a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
index f2d4d9408a8297a..6ba98373d33ada8 100644
--- a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
+++ b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
@@ -67,10 +67,6 @@ int main() {
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:
-// CHECK: ![[IENUM_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "inline_enum",
-// CHECK-SAME: flags: DIFlagStaticMember
-// CHECK-NOT: extraData:
-
// CHECK: ![[EMPTY_TEMPLATED_DECL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_member, name: "empty_templated",
// CHECK-SAME: flags: DIFlagStaticMember
// CHECK-NOT: extraData:
@@ -98,11 +94,6 @@ int main() {
// CHECK-NOT: linkageName:
// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[ENUM_DECL]])
-// CHECK: !DIGlobalVariableExpression(var: ![[IENUM_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, {{.*}}, DW_OP_stack_value))
-// CHECK: ![[IENUM_VAR]] = distinct !DIGlobalVariable(name: "inline_enum"
-// CHECK-NOT: linkageName:
-// CHECK-SAME: isLocal: true, isDefinition: true, declaration: ![[IENUM_DECL]])
-
// CHECK: !DIGlobalVariableExpression(var: ![[EMPTY_TEMPLATED_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
// CHECK: ![[EMPTY_TEMPLATED_VAR]] = distinct !DIGlobalVariable(name: "empty_templated"
// CHECK-NOT: linkageName:
|
This patch extracts the logic to evaluate a C++ static data-member's constant initializer such that it can be used by an upcoming patch. It also makes the check for whether we are dealing with a constant initializer more robust/idiomatic, which revealed a bug in the `debug-info-static-inline-member` test.
Michael137
force-pushed
the
inline-statics-support/clang/improve-initializer-check
branch
from
4bb7f6b
to
0120962
Compare
adrian-prantl
approved these changes
Nov 29, 2023
Michael137
deleted the
inline-statics-support/clang/improve-initializer-check
branch
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Nov 30, 2023
…fo-static-inline-member The check for this was removed in llvm#72974 This patch removes the member from the source itself since it was confusing FileCheck
|
Test fix in flight... |
Michael137
added a commit
that referenced
this pull request
Nov 30, 2023
…fo-static-inline-member The check for this was removed in #72974 This patch removes the member from the source itself since it was confusing FileCheck
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch extracts the logic to evaluate a C++ static data-member's constant initializer. This logic will be re-used in an upcoming patch.
It also makes the check for whether we are dealing with a constant initializer more robust/idiomatic, which revealed a bug in the
debug-info-static-inline-membertest (which existed since its introduction in #71780)Test changes
debug-info-static-member.cpp:const_bas part of thepatch series in
638a8393615e911b729d5662096f60ef49f1c65e.The check for
isUsableAsConstantExpressionadded in the current patchdoesn't support constant inline floats (since they are neither constexpr nor
integrals). This isn't a regression since before said patch series
we wouldn't ever emit the definition for
const_banyway. Nowwe just don't do it for
inline const floats. This is consistent withGCC's behaviour starting with C++11.
debug-info-static-inline-member:a
DW_AT_const_valuefor a non-const static.