[DebugInfo][InstrRef][NFC] Emit variadic debug values from InstrRefLDV

[StephenTozer]

Summary

In preparation for supporting DBG_VALUE_LIST in InstrRefLDV, this patch adds the logic for emitting DBG_VALUE_LIST instructions from InstrRefLDV. The logical changes here are fairly simple, with the main change being that instead of directly prepending offsets to the DIExpr, we use appendOpsToArg to modify the expression for individual debug operands in the expression. The function emitLoc is also changed to take a list of debug ops, with an empty list meaning an undef value.

[Orlando]

LGTM

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

929

tiny nit: this seems to have unnecessarily moved

[StephenTozer]

clang-format, address review comment.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h

public:
   std::string IDAsString(const ValueIDNum &Num) const;
 
 #ifndef NDEBUG
   LLVM_DUMP_METHOD void dump();
 
   LLVM_DUMP_METHOD void dump_mloc_map();
 #endif
 
-  /// Create a DBG_VALUE based on  machine location \p MLoc. Qualify it with the
+  /// Create a DBG_VALUE based on debug operands \p DbgOps. Qualify it with the
   /// information in \pProperties, for variable Var. Don't insert it anywhere,
   /// just return the builder for it.
-  MachineInstrBuilder emitLoc(Optional<LocIdx> MLoc, const DebugVariable &Var,
+  MachineInstrBuilder emitLoc(const SmallVectorImpl<ResolvedDbgOp> &DbgOps,
+                              const DebugVariable &Var,
                               const DbgValueProperties &Properties);
 };
 
 /// Types for recording sets of variable fragments that overlap. For a given
 /// local variable, we record all other fragments of that variable that could
 /// overlap it, to reduce search time.
 using FragmentOfVar =
     std::pair<const DILocalVariable *, DIExpression::FragmentInfo>;

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

for (const auto &Var : VLocs) {
       }
 
       LocIdx M = ValuesPreferredLoc->second;
       auto NewValue = LocAndProperties{M, Var.second.Properties};
       auto Result = ActiveVLocs.insert(std::make_pair(Var.first, NewValue));
       if (!Result.second)
         Result.first->second = NewValue;
       ActiveMLocs[M].insert(Var.first);
+      SmallVector<ResolvedDbgOp> ResolvedOps;
+      ResolvedOps.push_back(M);
       PendingDbgValues.push_back(
-          MTracker->emitLoc(M, Var.first, Var.second.Properties));
+          MTracker->emitLoc(ResolvedOps, Var.first, Var.second.Properties));
     }
     flushDbgValues(MBB.begin(), &MBB);
   }
 
   /// Record that \p Var has value \p ID, a value that becomes available
   /// later in the function.
   void addUseBeforeDef(const DebugVariable &Var,
                        const DbgValueProperties &Properties, ValueIDNum ID) {
for (auto &Use : MIt->second) {
       if (MTracker->readMLoc(L) != Use.ID)
         continue;
 
       // If a different debug instruction defined the variable value / location
       // since the start of the block, don't materialize this use-before-def.
       if (!UseBeforeDefVariables.count(Use.Var))
         continue;
 
-      PendingDbgValues.push_back(MTracker->emitLoc(L, Use.Var, Use.Properties));
+      SmallVector<ResolvedDbgOp> ResolvedOps;
+      ResolvedOps.push_back(L);
+      PendingDbgValues.push_back(
+          MTracker->emitLoc(ResolvedOps, Use.Var, Use.Properties));
     }
     flushDbgValues(pos, nullptr);
   }
 
   /// Helper to move created DBG_VALUEs into Transfers collection.
   void flushDbgValues(MachineBasicBlock::iterator Pos, MachineBasicBlock *MBB) {
     if (PendingDbgValues.size() == 0)
       return;
void clobberMloc(LocIdx MLoc, ValueIDNum OldValue,
     // Examine all the variables based on this location.
     DenseSet<DebugVariable> NewMLocs;
     for (auto &Var : ActiveMLocIt->second) {
       auto ActiveVLocIt = ActiveVLocs.find(Var);
       // Re-state the variable location: if there's no replacement then NewLoc
       // is None and a $noreg DBG_VALUE will be created. Otherwise, a DBG_VALUE
       // identifying the alternative location will be emitted.
       const DbgValueProperties &Properties = ActiveVLocIt->second.Properties;
-      PendingDbgValues.push_back(MTracker->emitLoc(NewLoc, Var, Properties));
+      SmallVector<ResolvedDbgOp> ResolvedOps;
+      if (NewLoc)
+        ResolvedOps.push_back(*NewLoc);
+      PendingDbgValues.push_back(
+          MTracker->emitLoc(ResolvedOps, Var, Properties));
 
       // Update machine locations <=> variable locations maps. Defer updating
       // ActiveMLocs to avoid invalidaing the ActiveMLocIt iterator.
       if (!NewLoc) {
         ActiveVLocs.erase(ActiveVLocIt);
       } else {
         ActiveVLocIt->second.Loc = *NewLoc;
         NewMLocs.insert(Var);
void transferMlocs(LocIdx Src, LocIdx Dst, MachineBasicBlock::iterator Pos) {
     VarLocs[Dst.asU64()] = VarLocs[Src.asU64()];
 
     // For each variable based on Src; create a location at Dst.
     for (auto &Var : MovingVars) {
       auto ActiveVLocIt = ActiveVLocs.find(Var);
       assert(ActiveVLocIt != ActiveVLocs.end());
       ActiveVLocIt->second.Loc = Dst;
 
+      SmallVector<ResolvedDbgOp> ResolvedOps;
+      ResolvedOps.push_back(Dst);
       MachineInstr *MI =
-          MTracker->emitLoc(Dst, Var, ActiveVLocIt->second.Properties);
+          MTracker->emitLoc(ResolvedOps, Var, ActiveVLocIt->second.Properties);
       PendingDbgValues.push_back(MI);
     }
     ActiveMLocs[Src].clear();
     flushDbgValues(Pos, nullptr);
 
     // XXX XXX XXX "pretend to be old LDV" means dropping all tracking data
     // about the old location.
     if (EmulateOldLDV)

 LLVM_DUMP_METHOD void MLocTracker::dump_mloc_map() {
   for (auto Location : locations()) {
     std::string foo = LocIdxToName(Location.Idx);
     dbgs() << "Idx " << Location.Idx.asU64() << " " << foo << "\n";
   }
 }
 #endif
 
-MachineInstrBuilder MLocTracker::emitLoc(Optional<LocIdx> MLoc,
-                                         const DebugVariable &Var,
-                                         const DbgValueProperties &Properties) {
+MachineInstrBuilder
+MLocTracker::emitLoc(const SmallVectorImpl<ResolvedDbgOp> &DbgOps,
   DebugLoc DL = DILocation::get(Var.getVariable()->getContext(), 0, 0,
                                 Var.getVariable()->getScope(),
                                 const_cast<DILocation *>(Var.getInlinedAt()));
 
   const MCInstrDesc &Desc = Properties.IsVariadic
                                 ? TII.get(TargetOpcode::DBG_VALUE_LIST)
                                 : TII.get(TargetOpcode::DBG_VALUE);
 
+#ifdef EXPENSIVE_CHECKS
+  assert(all_of(DbgOps,
+                [](const ResolvedDbgOp &Op) {
+                  return Op.IsConst || !Op.Loc.isIllegal();
+                }) &&
+         "Did not expect illegal ops in DbgOps.");
+  assert((DbgOps.size() == 0 ||
+          DbgOps.size() == Properties.getLocationOpCount()) &&
+         "Expected to have either one DbgOp per MI LocationOp, or none.");
+#endif
+
   auto GetRegOp = [](unsigned Reg) -> MachineOperand {
     return MachineOperand::CreateReg(
         /* Reg */ Reg, /* isDef */ false, /* isImp */ false,
         /* isKill */ false, /* isDead */ false,
         /* isUndef */ false, /* isEarlyClobber */ false,
         /* SubReg */ 0, /* isDebug */ true);
   };
 
   SmallVector<MachineOperand> MOs;
 
   auto EmitUndef = [&]() {
     MOs.clear();
     MOs.assign(Properties.getLocationOpCount(), GetRegOp(0));
     return BuildMI(MF, DL, Desc, false, MOs, Var.getVariable(),
                    Properties.DIExpr);
   };
 
-  // Only 1 location is currently supported.
-  if (Properties.IsVariadic && Properties.getLocationOpCount() != 1)
+  // Don't bother passing any real operands to BuildMI if any of them would be
+  // $noreg.
+  if (DbgOps.empty())
     return EmitUndef();
 
   bool Indirect = Properties.Indirect;
 
   const DIExpression *Expr = Properties.DIExpr;
-  if (!MLoc) {
-    // No location -> DBG_VALUE $noreg
-    return EmitUndef();
-  } else if (LocIdxToLocID[*MLoc] >= NumRegs) {
-    unsigned LocID = LocIdxToLocID[*MLoc];
+
+  assert(DbgOps.size() == Properties.getLocationOpCount());
+
+  // If all locations are valid, accumulate them into our list of
+  // MachineOperands. For any spilled locations, either update the indirectness
+  // register or apply the appropriate transformations in the DIExpression.
+  for (size_t Idx = 0; Idx < Properties.getLocationOpCount(); ++Idx) {
+    const ResolvedDbgOp &Op = DbgOps[Idx];
+
+    if (Op.IsConst) {
+      MOs.push_back(Op.MO);
+      continue;
+    }
+
+    LocIdx MLoc = Op.Loc;
+    unsigned LocID = LocIdxToLocID[MLoc];
+    if (LocID >= NumRegs) {
       SpillLocationNo SpillID = locIDToSpill(LocID);
       StackSlotPos StackIdx = locIDToSpillIdx(LocID);
       unsigned short Offset = StackIdx.second;
 
       // TODO: support variables that are located in spill slots, with non-zero
       // offsets from the start of the spill slot. It would require some more
       // complex DIExpression calculations. This doesn't seem to be produced by
       // LLVM right now, so don't try and support it.
       // Accept no-subregister slots and subregisters where the offset is zero.
       // The consumer should already have type information to work out how large
       // the variable is.
       if (Offset == 0) {
         const SpillLoc &Spill = SpillLocs[SpillID.id()];
         unsigned Base = Spill.SpillBase;
-      MOs.push_back(GetRegOp(Base));
 
         // There are several ways we can dereference things, and several inputs
         // to consider:
         // * NRVO variables will appear with IsIndirect set, but should have
         //   nothing else in their DIExpressions,
         // * Variables with DW_OP_stack_value in their expr already need an
         //   explicit dereference of the stack location,
         // * Values that don't match the variable size need DW_OP_deref_size,
         // * Everything else can just become a simple location expression.
 
         // We need to use deref_size whenever there's a mismatch between the
         // size of value and the size of variable portion being read.
         // Additionally, we should use it whenever dealing with stack_value
         // fragments, to avoid the consumer having to determine the deref size
         // from DW_OP_piece.
         bool UseDerefSize = false;
-      unsigned ValueSizeInBits = getLocSizeInBits(*MLoc);
+        unsigned ValueSizeInBits = getLocSizeInBits(MLoc);
         unsigned DerefSizeInBytes = ValueSizeInBits / 8;
         if (auto Fragment = Var.getFragment()) {
           unsigned VariableSizeInBits = Fragment->SizeInBits;
           if (VariableSizeInBits != ValueSizeInBits || Expr->isComplex())
             UseDerefSize = true;
         } else if (auto Size = Var.getVariable()->getSizeInBits()) {
           if (*Size != ValueSizeInBits) {
             UseDerefSize = true;
           }
         }
 
+        SmallVector<uint64_t, 5> OffsetOps;
+        TRI.getOffsetOpcodes(Spill.SpillOffset, OffsetOps);
+        bool StackValue = false;
+
         if (Properties.Indirect) {
           // This is something like an NRVO variable, where the pointer has been
-        // spilt to the stack, or a dbg.addr pointing at a coroutine frame
-        // field. It should end up being a memory location, with the pointer
-        // to the variable loaded off the stack with a deref. It can't be a
-        // DW_OP_stack_value expression.
+          // spilt to the stack. It should end up being a memory location, with
+          // the pointer to the variable loaded off the stack with a deref:
           assert(!Expr->isImplicit());
-        Expr = TRI.prependOffsetExpression(
-            Expr, DIExpression::ApplyOffset | DIExpression::DerefAfter,
-            Spill.SpillOffset);
-      } else if (UseDerefSize) {
+          OffsetOps.push_back(dwarf::DW_OP_deref);
+        } else if (UseDerefSize && !Properties.IsVariadic) {
+          // TODO: Figure out how to handle deref size issues for variadic
+          // values.
           // We're loading a value off the stack that's not the same size as the
-        // variable. Add / subtract stack offset, explicitly deref with a size,
-        // and add DW_OP_stack_value if not already present.
-        SmallVector<uint64_t, 2> Ops = {dwarf::DW_OP_deref_size,
-                                        DerefSizeInBytes};
-        Expr = DIExpression::prependOpcodes(Expr, Ops, true);
-        unsigned Flags = DIExpression::StackValue | DIExpression::ApplyOffset;
-        Expr = TRI.prependOffsetExpression(Expr, Flags, Spill.SpillOffset);
+          // variable. Add / subtract stack offset, explicitly deref with a
+          // size, and add DW_OP_stack_value if not already present.
+          OffsetOps.push_back(dwarf::DW_OP_deref_size);
+          OffsetOps.push_back(DerefSizeInBytes);
+          StackValue = true;
         } else if (Expr->isComplex()) {
           // A variable with no size ambiguity, but with extra elements in it's
           // expression. Manually dereference the stack location.
-        assert(Expr->isComplex());
-        Expr = TRI.prependOffsetExpression(
-            Expr, DIExpression::ApplyOffset | DIExpression::DerefAfter,
-            Spill.SpillOffset);
+          OffsetOps.push_back(dwarf::DW_OP_deref);
         } else {
           // A plain value that has been spilt to the stack, with no further
           // context. Request a location expression, marking the DBG_VALUE as
           // IsIndirect.
-        Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset,
-                                           Spill.SpillOffset);
           Indirect = true;
         }
+
+        Expr = DIExpression::appendOpsToArg(Expr, OffsetOps, Idx, StackValue);
+        MOs.push_back(GetRegOp(Base));
       } else {
-      // This is a stack location with a weird subregister offset: emit an undef
-      // DBG_VALUE instead.
+        // This is a stack location with a weird subregister offset: emit an
+        // undef DBG_VALUE instead.
         return EmitUndef();
       }
     } else {
       // Non-empty, non-stack slot, must be a plain register.
-    unsigned LocID = LocIdxToLocID[*MLoc];
       MOs.push_back(GetRegOp(LocID));
     }
+  }
 
   return BuildMI(MF, DL, Desc, Indirect, MOs, Var.getVariable(), Expr);
 }
 
 /// Default construct and initialize the pass.
 InstrRefBasedLDV::InstrRefBasedLDV() = default;
 
 bool InstrRefBasedLDV::isCalleeSaved(LocIdx L) const {
if (!FoundLoc && NewID && NewID->getBlock() == CurBB &&
       NewID->getInst() > CurInst)
     TTracker->addUseBeforeDef(V, {MI.getDebugExpression(), false, false},
                               *NewID);
 
   // Produce a DBG_VALUE representing what this DBG_INSTR_REF meant.
   // This DBG_VALUE is potentially a $noreg / undefined location, if
   // FoundLoc is None.
   // (XXX -- could morph the DBG_INSTR_REF in the future).
-  MachineInstr *DbgMI = MTracker->emitLoc(FoundLoc, V, Properties);
+  SmallVector<ResolvedDbgOp> ResolvedOps;
+  if (FoundLoc)
+    ResolvedOps.push_back(*FoundLoc);
+  MachineInstr *DbgMI = MTracker->emitLoc(ResolvedOps, V, Properties);
 
   TTracker->PendingDbgValues.push_back(DbgMI);
   TTracker->flushDbgValues(MI.getIterator(), nullptr);
   return true;
 }
 
 bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) {
   if (!MI.isDebugPHI())