[DebugInfo][InstrRef][NFC Update LDV to use generic DBG_VALUE* MI interface

[StephenTozer]

Summary

Currently, InstrRefLDV only handles DBG_VALUE instructions, not DBG_VALUE_LIST, and as a result of this it handles these instructions using functions that only work for that type of debug value, i.e. using getOperand(0) to get the debug operand. This patch changes this to use the generic debug value functions, such as getDebugOperand and isDebugOffsetImm, as well as adding an IsVariadic field to the DbgValueProperties class and a few other minor changes to acknowledge DBG_VALUE_LISTs. Note that this patch does not add support for DBG_VALUE_LIST here, but is a precursor to other patches that do add that support.

[Orlando]

LGTM

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

461

I would consider adding assert(!Prop.IsVariadic) above this line (or changing the emitMOLoc call to emitMOLoc(MO, Var, {NewExpr, Prop.Indirect, Prop.IsVariadic}) (or both), so that this line doesn't get out of sync with the if statement added at the start of this function, but YMMV - feel free to ignore this.

[StephenTozer]

Use IsVariadic when emitting a location for an entry value.

[StephenTozer]

Update unit tests to match the new interface.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h

bool operator!=(const SpillLocationNo &Other) const {
     return !(*this == Other);
   }
 };
 
 /// Meta qualifiers for a value. Pair of whatever expression is used to qualify
 /// the value, and Boolean of whether or not it's indirect.
 class DbgValueProperties {
 public:
-  DbgValueProperties(const DIExpression *DIExpr, bool Indirect)
-      : DIExpr(DIExpr), Indirect(Indirect) {}
+  DbgValueProperties(const DIExpression *DIExpr, bool Indirect, bool IsVariadic)
+      : DIExpr(DIExpr), Indirect(Indirect), IsVariadic(IsVariadic) {}
 
   /// Extract properties from an existing DBG_VALUE instruction.
   DbgValueProperties(const MachineInstr &MI) {
     assert(MI.isDebugValue());
+    assert(MI.getDebugExpression()->getNumLocationOperands() == 0 ||
+           MI.isDebugValueList() || MI.isUndefDebugValue());
+    IsVariadic = MI.isDebugValueList();
     DIExpr = MI.getDebugExpression();
-    Indirect = MI.getOperand(1).isImm();
+    Indirect = MI.isDebugOffsetImm();
   }
 
   bool operator==(const DbgValueProperties &Other) const {
-    return std::tie(DIExpr, Indirect) == std::tie(Other.DIExpr, Other.Indirect);
+    return std::tie(DIExpr, Indirect, IsVariadic) ==
+           std::tie(Other.DIExpr, Other.Indirect, Other.IsVariadic);
   }
 
   bool operator!=(const DbgValueProperties &Other) const {
     return !(*this == Other);
   }
 
+  unsigned getLocationOpCount() const {
+    return IsVariadic ? DIExpr->getNumLocationOperands() : 1;
+  }
+
   const DIExpression *DIExpr;
   bool Indirect;
+  bool IsVariadic;
 };
 
 /// Class recording the (high level) _value_ of a variable. Identifies either
 /// the value of the variable as a ValueIDNum, or a constant MachineOperand.
 /// This class also stores meta-information about how the value is qualified.
 /// Used to reason about variable values when performing the second
 /// (DebugVariable specific) dataflow analysis.
 class DbgValue {
public:
   MapVector<DebugVariable, DbgValue> Vars;
   SmallDenseMap<DebugVariable, const DILocation *, 8> Scopes;
   MachineBasicBlock *MBB = nullptr;
   const OverlapMap &OverlappingFragments;
   DbgValueProperties EmptyProperties;
 
 public:
   VLocTracker(const OverlapMap &O, const DIExpression *EmptyExpr)
-      : OverlappingFragments(O), EmptyProperties(EmptyExpr, false) {}
+      : OverlappingFragments(O), EmptyProperties(EmptyExpr, false, false) {}
 
   void defVar(const MachineInstr &MI, const DbgValueProperties &Properties,
               Optional<ValueIDNum> ID) {
     assert(MI.isDebugValue() || MI.isDebugRef());
     DebugVariable Var(MI.getDebugVariable(), MI.getDebugExpression(),
                       MI.getDebugLoc()->getInlinedAt());
     DbgValue Rec = (ID) ? DbgValue(*ID, Properties, DbgValue::Def)
                         : DbgValue(Properties, DbgValue::Undef);

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

public:
   bool recoverAsEntryValue(const DebugVariable &Var,
                            const DbgValueProperties &Prop,
                            const ValueIDNum &Num) {
     // Is this variable location a candidate to be an entry value. First,
     // should we be trying this at all?
     if (!ShouldEmitDebugEntryValues)
       return false;
 
+    // We don't currently emit entry values for DBG_VALUE_LISTs.
+    if (Prop.IsVariadic)
+      return false;
+
     // Is the variable appropriate for entry values (i.e., is a parameter).
     if (!isEntryValueVariable(Var, Prop.DIExpr))
       return false;
 
     // Is the value assigned to this variable still the entry value?
     if (!isEntryValueValue(Num))
       return false;
 
     // Emit a variable location using an entry value expression.
     DIExpression *NewExpr =
         DIExpression::prepend(Prop.DIExpr, DIExpression::EntryValue);
     Register Reg = MTracker->LocIdxToLocID[Num.getLoc()];
     MachineOperand MO = MachineOperand::CreateReg(Reg, false);
 
-    PendingDbgValues.push_back(emitMOLoc(MO, Var, {NewExpr, Prop.Indirect}));
+    PendingDbgValues.push_back(
+        emitMOLoc(MO, Var, {NewExpr, Prop.Indirect, false}));
     return true;
   }
 
   /// Change a variable value after encountering a DBG_VALUE inside a block.
   void redefVar(const MachineInstr &MI) {
     DebugVariable Var(MI.getDebugVariable(), MI.getDebugExpression(),
                       MI.getDebugLoc()->getInlinedAt());
     DbgValueProperties Properties(MI);
 
-    const MachineOperand &MO = MI.getOperand(0);
+    const MachineOperand &MO = MI.getDebugOperand(0);
 
     // Ignore non-register locations, we don't transfer those.
     if (!MO.isReg() || MO.getReg() == 0) {
       auto It = ActiveVLocs.find(Var);
       if (It != ActiveVLocs.end()) {
         ActiveMLocs[It->second.Loc].erase(Var);
         ActiveVLocs.erase(It);
      }

 #endif
 
 MachineInstrBuilder MLocTracker::emitLoc(Optional<LocIdx> MLoc,
                                          const DebugVariable &Var,
                                          const DbgValueProperties &Properties) {
   DebugLoc DL = DILocation::get(Var.getVariable()->getContext(), 0, 0,
                                 Var.getVariable()->getScope(),
                                 const_cast<DILocation *>(Var.getInlinedAt()));
-  auto MIB = BuildMI(MF, DL, TII.get(TargetOpcode::DBG_VALUE));
+
+  const MCInstrDesc &Desc = Properties.IsVariadic
+                                ? TII.get(TargetOpcode::DBG_VALUE_LIST)
+                                : TII.get(TargetOpcode::DBG_VALUE);
+
+  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)
+    return EmitUndef();
+  
+  bool Indirect = Properties.Indirect;
 
   const DIExpression *Expr = Properties.DIExpr;
   if (!MLoc) {
     // No location -> DBG_VALUE $noreg
-    MIB.addReg(0);
-    MIB.addReg(0);
+    return EmitUndef();
   } else if (LocIdxToLocID[*MLoc] >= NumRegs) {
     unsigned LocID = LocIdxToLocID[*MLoc];
     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;
-      MIB.addReg(Base);
+      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,
if (Offset == 0) {
         // 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.
         assert(!Expr->isImplicit());
         Expr = TRI.prependOffsetExpression(
             Expr, DIExpression::ApplyOffset | DIExpression::DerefAfter,
             Spill.SpillOffset);
-        MIB.addImm(0);
       } else if (UseDerefSize) {
         // 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);
-        MIB.addReg(0);
       } 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);
-        MIB.addReg(0);
       } 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);
-        MIB.addImm(0);
+        Indirect = true;
       }
     } else {
       // This is a stack location with a weird subregister offset: emit an undef
       // DBG_VALUE instead.
-      MIB.addReg(0);
-      MIB.addReg(0);
+      return EmitUndef();
     }
   } else {
     // Non-empty, non-stack slot, must be a plain register.
     unsigned LocID = LocIdxToLocID[*MLoc];
-    MIB.addReg(LocID);
-    if (Properties.Indirect)
-      MIB.addImm(0);
-    else
-      MIB.addReg(0);
+    MOs.push_back(GetRegOp(LocID));
   }
 
-  MIB.addMetadata(Var.getVariable());
-  MIB.addMetadata(Expr);
-  return MIB;
+  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 {
   unsigned Reg = MTracker->LocIdxToLocID[L];
   for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
bool InstrRefBasedLDV::transferDebugValue(const MachineInstr &MI) {
   if (MI.isDebugValueList()) {
     if (VTracker)
       VTracker->defVar(MI, Properties, None);
     if (TTracker)
       TTracker->redefVar(MI, Properties, None);
     return true;
   }
 
-  const MachineOperand &MO = MI.getOperand(0);
+  const MachineOperand &MO = MI.getDebugOperand(0);
 
   // MLocTracker needs to know that this register is read, even if it's only
   // read by a debug inst.
   if (MO.isReg() && MO.getReg() != 0)
     (void)MTracker->readReg(MO.getReg());
 
   // If we're preparing for the second analysis (variables), the machine value
   // locations are already solved, and we report this DBG_VALUE and the value
   // it refers to to VLocTracker.
   if (VTracker) {
     if (MO.isReg()) {
       // Feed defVar the new variable location, or if this is a
       // DBG_VALUE $noreg, feed defVar None.
       if (MO.getReg())
         VTracker->defVar(MI, Properties, MTracker->readReg(MO.getReg()));
       else
         VTracker->defVar(MI, Properties, None);
-    } else if (MI.getOperand(0).isImm() || MI.getOperand(0).isFPImm() ||
-               MI.getOperand(0).isCImm()) {
-      VTracker->defVar(MI, MI.getOperand(0));
+    } else if (MO.isImm() || MO.isFPImm() || MO.isCImm()) {
+      VTracker->defVar(MI, MO);
     }
   }
 
   // If performing final tracking of transfers, report this variable definition
   // to the TransferTracker too.
   if (TTracker)
     TTracker->redefVar(MI);
   return true;
if (NewID && !MTracker->isSpill(L)) {
       NewID = None;
     }
   }
 
   // We, we have a value number or None. Tell the variable value tracker about
   // it. The rest of this LiveDebugValues implementation acts exactly the same
   // for DBG_INSTR_REFs as DBG_VALUEs (just, the former can refer to values that
   // aren't immediately available).
-  DbgValueProperties Properties(Expr, false);
+  DbgValueProperties Properties(Expr, false, false);
   if (VTracker)
     VTracker->defVar(MI, Properties, NewID);
 
   // If we're on the final pass through the function, decompose this INSTR_REF
   // into a plain DBG_VALUE.
   if (!TTracker)
     return true;
 
bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
 
   // Tell transfer tracker that the variable value has changed.
   TTracker->redefVar(MI, Properties, FoundLoc);
 
   // If there was a value with no location; but the value is defined in a
   // later instruction in this block, this is a block-local use-before-def.
   if (!FoundLoc && NewID && NewID->getBlock() == CurBB &&
       NewID->getInst() > CurInst)
-    TTracker->addUseBeforeDef(V, {MI.getDebugExpression(), false}, *NewID);
+    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);
   TTracker->PendingDbgValues.push_back(DbgMI);
   TTracker->flushDbgValues(MI.getIterator(), nullptr);
void InstrRefBasedLDV::buildVLocValueMap(
 
   // Allocate some vectors for storing the live ins and live outs. Large.
   SmallVector<DbgValue, 32> LiveIns, LiveOuts;
   LiveIns.reserve(NumBlocks);
   LiveOuts.reserve(NumBlocks);
 
   // Initialize all values to start as NoVals. This signifies "it's live
   // through, but we don't know what it is".
-  DbgValueProperties EmptyProperties(EmptyExpr, false);
+  DbgValueProperties EmptyProperties(EmptyExpr, false, false);
   for (unsigned int I = 0; I < NumBlocks; ++I) {
     DbgValue EmptyDbgValue(I, EmptyProperties, DbgValue::NoVal);
     LiveIns.push_back(EmptyDbgValue);
     LiveOuts.push_back(EmptyDbgValue);
   }
 
   // Produce by-MBB indexes of live-in/live-outs, to ease lookup within
   // vlocJoin.