[PATCH] D102500: [DebugInfo] Produce undef DBG_VALUE instructions for variadic dbg.values when using FastISel or GlobalIsel

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer created this revision.
StephenTozer added reviewers: aprantl, vsk, t.p.northover, dexonsmith, jmorse, dblaikie.
StephenTozer added a project: debug-info.
Herald added subscribers: hiraditya, rovka.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Currently, variadic dbg.values (i.e. those using a `DIArgList` as part of their location) are not handled properly by FastIsel or GlobalIsel. A full implementation may take some time to complete, but these pipelines must not produce invalid debug info in the absence of a full implementation. Maintaining current behaviour in this case means producing undef `DBG_VALUE` instructions for each variadic dbg.value; currently these passes instead produce `DBG_VALUE` instructions as normal, but only using the first value in the `DIArgList` - this is not only incorrect, but also invalid as the set of valid `DIExpression`s for variadic and non-variadic dbg.values are disjoint. This may result in errors further along the pipeline.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102500

Files:
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp


Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1275,9 +1275,9 @@
     const Value *V = DI->getValue();
     assert(DI->getVariable()->isValidLocationForIntrinsic(DbgLoc) &&
            "Expected inlined-at fields to agree");
-    if (!V || isa<UndefValue>(V)) {
-      // Currently the optimizer can produce this; insert an undef to
-      // help debugging.
+    if (!V || isa<UndefValue>(V) || DI->hasArgList()) {
+      // DI is either undef or cannot produce a valid DBG_VALUE, so produce an
+      // undef DBG_VALUE to terminate any prior location.
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II, false, 0U,
               DI->getVariable(), DI->getExpression());
     } else if (const auto *CI = dyn_cast<ConstantInt>(V)) {
Index: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -1913,9 +1913,9 @@
     assert(DI.getVariable()->isValidLocationForIntrinsic(
                MIRBuilder.getDebugLoc()) &&
            "Expected inlined-at fields to agree");
-    if (!V) {
-      // Currently the optimizer can produce this; insert an undef to
-      // help debugging.  Probably the optimizer should not do this.
+    if (!V || DI.hasArgList()) {
+      // DI cannot produce a valid DBG_VALUE, so produce an undef DBG_VALUE to
+      // terminate any prior location.
       MIRBuilder.buildIndirectDbgValue(0, DI.getVariable(), DI.getExpression());
     } else if (const auto *CI = dyn_cast<Constant>(V)) {
       MIRBuilder.buildConstDbgValue(*CI, DI.getVariable(), DI.getExpression());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D102500.345437.patch
Type: text/x-patch
Size: 1840 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210514/38c030d3/attachment.bin>