[Clang][DebugInfo] Clang generates an extra spurious unnamed 'dbg.declare' #69681
Conversation
This file contains hidden or 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
CarlosAlbertoEnciso
added
clang
CarlosAlbertoEnciso
requested review from
nikic,
jryans,
adrian-prantl,
felipepiovezan,
dwblaikie and
djtodoro
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Carlos Alberto Enciso (CarlosAlbertoEnciso) ChangesDo not emit call to llvm.dbg.declare when the variable declaration The emitted debug declare looks like: Full diff: https://github.com/llvm/llvm-project/pull/69681.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index c430713b0d77d79..b2646cd1d2a0f53 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4879,11 +4879,15 @@ CGDebugInfo::EmitDeclareOfAutoVariable(const VarDecl *VD, llvm::Value *Storage,
const bool UsePointerValue) {
assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
- if (auto *DD = dyn_cast<DecompositionDecl>(VD))
+ if (auto *DD = dyn_cast<DecompositionDecl>(VD)) {
for (auto *B : DD->bindings()) {
EmitDeclare(B, Storage, std::nullopt, Builder,
VD->getType()->isReferenceType());
}
+ // Don't emit an llvm.dbg.declare for the composite storage as it doesn't
+ // correspond to a user variable.
+ return nullptr;
+ }
return EmitDeclare(VD, Storage, std::nullopt, Builder, UsePointerValue);
}
diff --git a/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp b/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
index b5ee96224565d3a..0234e41009f6225 100644
--- a/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
+++ b/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
@@ -189,7 +189,6 @@ struct S11 {
// CHECK-LABEL: define dso_local void @_Z4fS11v
// CHECK: alloca %struct.S11, align 4
// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S11, align 4
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]]
// CHECK-NOT: call void @llvm.dbg.declare(metadata ptr [[TMP0]]
//
void fS11() {
@@ -206,7 +205,6 @@ struct S12 {
// CHECK: alloca %struct.S12, align 4
// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S12, align 4
// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata [[S12_A:![0-9]+]], metadata !DIExpression())
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]]
// CHECK-NOT: call void @llvm.dbg.declare(metadata ptr [[TMP0]]
//
void fS12() {
@@ -222,7 +220,6 @@ struct __attribute__((packed)) S13 {
// CHECK-LABEL: define dso_local void @_Z4fS13v
// CHECK: alloca %struct.S13, align 1
// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S13, align 1
-// CHECK: call void @llvm.dbg.declare(metadata ptr [[TMP0]], metadata {{.*}}, metadata !DIExpression())
// CHECK-NOT: call void @llvm.dbg.declare(metadata ptr [[TMP0]]
//
void fS13() {
diff --git a/clang/test/CodeGenCXX/debug-info-structured-binding-field.cpp b/clang/test/CodeGenCXX/debug-info-structured-binding-field.cpp
new file mode 100644
index 000000000000000..928d22776d21093
--- /dev/null
+++ b/clang/test/CodeGenCXX/debug-info-structured-binding-field.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s
+
+struct Tuple {
+ int Fld_1;
+ int Fld_2;
+};
+__attribute__((optnone)) Tuple get() { return {10, 20}; }
+
+// CHECK-LABEL: define dso_local noundef i32 @main
+// CHECK: %retval = alloca i32, align 4
+// CHECK-NEXT: [[T0:%.*]] = alloca %struct.Tuple, align 4
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[T0]], metadata {{.*}}, metadata !DIExpression())
+// CHECK: call void @llvm.dbg.declare(metadata ptr [[T0]], metadata {{.*}}, metadata !DIExpression(DW_OP_plus_uconst, {{[0-9]+}}))
+// CHECK-NOT: call void @llvm.dbg.declare(metadata ptr [[T0]], metadata {{.*}}, metadata !DIExpression())
+//
+int main() {
+ auto [Var_1, Var_2] = get();
+
+ return Var_1 + Var_2;
+}
diff --git a/clang/test/CodeGenCXX/debug-info-structured-binding.cpp b/clang/test/CodeGenCXX/debug-info-structured-binding.cpp
index 8d4ae0aaf32636b..163c152efa19d0b 100644
--- a/clang/test/CodeGenCXX/debug-info-structured-binding.cpp
+++ b/clang/test/CodeGenCXX/debug-info-structured-binding.cpp
@@ -2,10 +2,8 @@
// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression())
// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_plus_uconst, {{[0-9]+}}))
-// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression())
// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_deref))
// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, {{[0-9]+}}))
-// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[0-9]+}}, metadata !{{[0-9]+}}, metadata !DIExpression())
struct A {
int x;
int y;
|
OCHyams
reviewed
Oct 20, 2023
There was a problem hiding this comment.
This SGTM.
I don't think the unnamed variable serves any useful purpose but may not being imaginative enough. I can't see any discussion on the topic when the code was added in D119178 . CC the author, @shafik (or @adrian-prantl) - is there a use-case for the anonymous variable that covers the whole of the structured binding storage?
Comment on lines
12
to
14
| // CHECK: call void @llvm.dbg.declare(metadata ptr [[T0]], metadata {{.*}}, metadata !DIExpression()) | ||
| // CHECK: call void @llvm.dbg.declare(metadata ptr [[T0]], metadata {{.*}}, metadata !DIExpression(DW_OP_plus_uconst, {{[0-9]+}})) | ||
| // CHECK-NOT: call void @llvm.dbg.declare(metadata ptr [[T0]], metadata {{.*}}, metadata !DIExpression()) |
There was a problem hiding this comment.
It doesn't look like this test covers anything that clang/test/CodeGenCXX/debug-info-structured-binding.cpp doesn't already?
IMO no need to add this one. Please can you modify the other test to check that the DILocalVariables in the dbg.declares have the expected names (i.e., confirm there's no unnamed variable).
There was a problem hiding this comment.
Removed the test case.
Added checks to check the variable names.
| @@ -2,10 +2,8 @@ | |||
|
|
|||
There was a problem hiding this comment.
Please can you add --implicit-check-not="call void @llvm.dbg.declare" to the FileCheck command?
There was a problem hiding this comment.
Added the --implicit-check-not option.
adrian-prantl
approved these changes
Oct 23, 2023
CarlosAlbertoEnciso
force-pushed
the
unnamed-debug-declare
branch
from
4e0131d to
9dc38cc
Compare
|
Uploaded new patch to address comments from @OCHyams |
…lare'. Do not emit call to llvm.dbg.declare when the variable declaration is a DecompositionDecl as its instance class is always unnamed. The emitted debug declare looks like: call void @llvm.dbg.declare(metadata ..., metadata !xx, metadata ...) !xx = !DILocalVariable(scope: !..., file: !..., line: ..., type: !...)
…lare'. Do not emit call to llvm.dbg.declare when the variable declaration is a DecompositionDecl as its instance class is always unnamed. The emitted debug declare looks like: call void @llvm.dbg.declare(metadata ..., metadata !xx, metadata ...) !xx = !DILocalVariable(scope: !..., file: !..., line: ..., type: !...) - Remove the added test case. - Added the '--implicit-check-not' option - Added checks for variables names.
…lare'. Do not emit call to llvm.dbg.declare when the variable declaration is a DecompositionDecl as its instance class is always unnamed. The emitted debug declare looks like: call void @llvm.dbg.declare(metadata ..., metadata !xx, metadata ...) !xx = !DILocalVariable(scope: !..., file: !..., line: ..., type: !...) - Added check for missing captured 'a' variable. - Check for the specific values in DW_OP_plus_uconst.
CarlosAlbertoEnciso
force-pushed
the
unnamed-debug-declare
branch
from
9dc38cc to
d9fe4ff
Compare
|
Uploaded new patch to address minor details with the test case:
|
OCHyams
approved these changes
Oct 25, 2023
|
@adrian-prantl, @OCHyams Thanks for your reviews. |
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.
Do not emit call to llvm.dbg.declare when the variable declaration
is a DecompositionDecl as its instance class is always unnamed.
The emitted debug declare looks like: