[DebugInfo] Drop stale entry value-limitation for call site values #172340
+8
−10
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
Entry value operations could previously not be combined with other operations in debug expressions, meaning that we had to skip emitting call site values in such cases. This DIExpression limitation was removed in 57a371d, so we should be free to emit call site values for such cases now, for example: extern void call(int, int); void entry_value (int param) { call(param + 222, param - 444); } This change exposed a call site parameter entry order issue in the dbgcall-site-expr-entry-value.mir test case. That ordering issue is tracked in llvm#43998, and I don't think there is anything inherent in this patch that caused that.
dstenb
requested review from
OCHyams,
SLTozer,
adrian-prantl,
djtodoro and
jmorse
Member
|
@llvm/pr-subscribers-debuginfo Author: David Stenberg (dstenb) ChangesEntry value operations could previously not be combined with other operations in debug expressions, meaning that we had to skip emitting call site values in such cases. This DIExpression limitation was removed in 57a371d, so we should be free to emit call site values for such cases now, for example: This change exposed a call site parameter entry order issue in the dbgcall-site-expr-entry-value.mir test case. That ordering issue is tracked in #43998, and I don't think there is anything inherent in this patch that caused that. Full diff: https://github.com/llvm/llvm-project/pull/172340.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 08444fc680df6..a5a84eaf81be6 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -614,11 +614,6 @@ static void finishCallSiteParams(ValT Val, const DIExpression *Expr,
for (auto Param : DescribedParams) {
bool ShouldCombineExpressions = Expr && Param.Expr->getNumElements() > 0;
- // TODO: Entry value operations can currently not be combined with any
- // other expressions, so we can't emit call site entries in those cases.
- if (ShouldCombineExpressions && Expr->isEntryValue())
- continue;
-
// If a parameter's call site value is produced by a chain of
// instructions we may have already created an expression for the
// parameter when walking through the instructions. Append that to the
diff --git a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-entry-value.mir b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-entry-value.mir
index d7740e0e59db1..7cff4486cf30f 100644
--- a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-entry-value.mir
+++ b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-entry-value.mir
@@ -81,12 +81,15 @@ body: |
...
-# Verify that a call site parameter is emitted for the third parameter. There
-# should also be entries for the first and second parameter, but
-# DW_OP_LLVM_entry_value operations can currently not be emitted together with
-# any other expressions. Verify that nothing is emitted rather than an assert
-# being triggered, or broken expressions being emitted.
+# Verify that call site parameters are emitted for the three parameters.
+# FIXME: The parameters are ordered incorrectly (#43998).
+# CHECK: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location (DW_OP_reg0 W0)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_GNU_entry_value(DW_OP_reg0 W0), DW_OP_plus_uconst 0xde)
# CHECK: DW_TAG_GNU_call_site_parameter
# CHECK-NEXT: DW_AT_location (DW_OP_reg2 W2)
# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_GNU_entry_value(DW_OP_reg0 W0))
+# CHECK: DW_TAG_GNU_call_site_parameter
+# CHECK-NEXT: DW_AT_location (DW_OP_reg1 W1)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_GNU_entry_value(DW_OP_reg0 W0), DW_OP_constu 0x1bc, DW_OP_minus)
|
Collaborator
Author
|
I had missed that this affects test cases that #172339 affects as well. I'll address those failures. |
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) LLVMLLVM.DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mirLLVM.DebugInfo/MIR/ARM/dbgcall-site-interpretation.mirIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
🪟 Windows x64 Test Results
Failed Tests(click on a test name to see its output) LLVMLLVM.DebugInfo/MIR/AArch64/dbgcall-site-interpretation.mirLLVM.DebugInfo/MIR/ARM/dbgcall-site-interpretation.mirIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Entry value operations could previously not be combined with other operations in debug expressions, meaning that we had to skip emitting call site values in such cases. This DIExpression limitation was removed in 57a371d, so we should be free to emit call site values for such cases now, for example:
This change exposed a call site parameter entry order issue in the dbgcall-site-expr-entry-value.mir test case. That ordering issue is tracked in #43998, and I don't think there is anything inherent in this patch that caused that.