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

[StephenTozer]

Summary

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 DIExpressions for variadic and non-variadic dbg.values are disjoint. This may result in errors further along the pipeline.

[StephenTozer]

Added test that was missing from the previous diff.

[djtodoro]

Do we expect cases where this feature is useful when doing -O0? I guess no, right? So the FastISel is OK, since it is being used at -O0 case only.

[StephenTozer]

In D102500#2759700, @djtodoro wrote:

Do we expect cases where this feature is useful when doing -O0? I guess no, right? So the FastISel is OK, since it is being used at -O0 case only.

You're right for the most part, but there are some edge cases: a function which is "always inline" called from a function marked "optnone" can result in FastIsel being run on a variadic dbg.value.

[jmorse]

LGTM given that I've seen a reproducer where this causes an assertion failure -- it's clearly best to avoid the assertion failure. We should re-visit this in the future as there are quite legitimate scenarios where this would be important, for example using optimised LLVM-IR with llc -O0.

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
   }
   case Intrinsic::dbg_value: {
     // This form of DBG_VALUE is target-independent.
     const DbgValueInst &DI = cast<DbgValueInst>(CI);
     const Value *V = DI.getValue();
     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());
     } else {
       for (Register Reg : getOrCreateVRegs(*V)) {
         // FIXME: This does not handle register-indirect values at offset 0. The
         // direct/indirect thing shouldn't really be handled by something as
         // implicit as reg+noreg vs reg+imm in the first place, but it seems

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
   }
   case Intrinsic::dbg_value: {
     // This form of DBG_VALUE is target-independent.
     const DbgValueInst *DI = cast<DbgValueInst>(II);
     const MCInstrDesc &II = TII.get(TargetOpcode::DBG_VALUE);
     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)) {
       if (CI->getBitWidth() > 64)
         BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)
             .addCImm(CI)
             .addImm(0U)
             .addMetadata(DI->getVariable())

llvm/test/DebugInfo/X86/debug_value_list_selectiondag.ll

-; RUN: llc %s -start-after=codegenprepare -stop-before=finalize-isel -o - | FileCheck %s
+; RUN: llc %s -start-after=codegenprepare -stop-before=finalize-isel -o - | FileCheck --check-prefixes=CHECK,DAG %s
+; RUN: llc %s -fast-isel=true -start-after=codegenprepare -stop-before=finalize-isel -o - | FileCheck --check-prefixes=CHECK,FAST %s
+; RUN: llc %s -global-isel=true -start-after=codegenprepare -stop-before=finalize-isel -o - | FileCheck --check-prefixes=CHECK,GLOBAL %s
 
 ;; Test that a dbg.value that uses a DIArgList is correctly converted to a
 ;; DBG_VALUE_LIST that uses the registers corresponding to its operands.
 
 ; CHECK-DAG: [[A_VAR:![0-9]+]] = !DILocalVariable(name: "a"
 ; CHECK-DAG: [[B_VAR:![0-9]+]] = !DILocalVariable(name: "b"
 ; CHECK-DAG: [[C_VAR:![0-9]+]] = !DILocalVariable(name: "c"
-; CHECK-LABEL: bb.0.entry
-; CHECK-DAG: DBG_VALUE_LIST [[A_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), %0, debug-location
-; CHECK-DAG: DBG_VALUE_LIST [[B_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), %1, debug-location
-; CHECK: DBG_VALUE_LIST [[C_VAR]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus), %0, %1, debug-location
+; CHECK-LABEL: bb.{{(0|1)}}.entry
+; DAG-DAG: DBG_VALUE_LIST [[A_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), %0, debug-location
+; DAG-DAG: DBG_VALUE_LIST [[B_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), %1, debug-location
+; DAG: DBG_VALUE_LIST [[C_VAR]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus), %0, %1, debug-location
+; FAST-DAG: DBG_VALUE $noreg, $noreg, [[A_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location
+; FAST-DAG: DBG_VALUE $noreg, $noreg, [[B_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location
+; FAST: DBG_VALUE $noreg, $noreg, [[C_VAR]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus), debug-location
+; GLOBAL-DAG: DBG_VALUE $noreg, 0, [[A_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location
+; GLOBAL-DAG: DBG_VALUE $noreg, 0, [[B_VAR]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location
+; GLOBAL: DBG_VALUE $noreg, 0, [[C_VAR]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus), debug-location
 
 target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc19.16.27034"
 
 define dso_local i32 @"?foo@@YAHHH@Z"(i32 %a, i32 %b) local_unnamed_addr !dbg !8 {
 entry:
   call void @llvm.dbg.value(metadata !DIArgList(i32 %b), metadata !14, metadata !DIExpression(DW_OP_LLVM_arg, 0)), !dbg !17
   call void @llvm.dbg.value(metadata !DIArgList(i32 %a), metadata !15, metadata !DIExpression(DW_OP_LLVM_arg, 0)), !dbg !17