[MachineInstr] Move MIParser's DBG_VALUE RegState::Debug invariant into MachineInstr::addOperand

[jackoalan]

Summary

Based on the reasoning of D53903, register operands of DBG_VALUE are invariably treated as RegState::Debug operands. This change enforces this invariant as part of MachineInstr::addOperand so that all passes emit this flag consistently.

RegState::Debug is inconsistently set on DBG_VALUE registers throughout LLVM. This runs the risk of a filtering iterator like MachineRegisterInfo::reg_nodbg_iterator to process these operands erroneously when not parsed from MIR sources.

This issue was observed in the development of the llvm-mos fork which adds a backend that relies on physical register operands much more than existing targets. Physical RegUnit 0 has the same numeric encoding as $noreg (indicating an undef for DBG_VALUE). Allowing debug operands into the machine scheduler correlates $noreg with RegUnit 0 (i.e. a collision of register numbers with different zero semantics). Eventually, this causes an assert where DBG_VALUE instructions are prohibited from participating in live register ranges.

[StephenTozer]

This looks like a strict improvement, since the new location for calling setIsDebug will catch all the old cases and more. There is one gap though - MachineOperand->ChangeToRegister(0, false) can be called on an operand for a DBG_VALUE (and is in multiple places), which sounds like it could cause the same problem you've observed; plugging this gap may require adding a similar block to that function.

This could probably do with a test - isDebug isn't printed to MIR, and it sounds like the issue only appears on a backend that isn't in LLVM, so a lit test probably isn't feasible. A simple unit test could work though: possibly a test in MachineInstrTest that creates a MachineInstr, adds MachineOperands, and confirms that they're debug registers.

[jackoalan]

Cover MachineOperand::ChangeToRegister with invariant. Add unit test to MachineInstrTest.cpp

[MatzeB]

Seems to me that the most important thing would be to add a check to MachineVerifier so we consistently catch problems. Adding automation to addOperand is fine, but there are enough ways to change instructions and operands with other APIs.

If you could find the time to add a check to MachineVerifier that would be fantastic! Though we can also leave that for another time, up to you.

Changes so far look fine, but if you add the automation, could you go through the sourcecode and remove manual usage of setIsDebug() then?

[jackoalan]

Shouldn't be too difficult to add the MachineVerifier check and remove the setIsDebug calls. Shall I also remove RegState::Debug in the context of DBG_VALUE builders?

[jackoalan]

Also curious if invariant debug register operands are applicable to other debug instructions (DBG_INSTR_REF, DBG_PHI, DBG_LABEL)

[jackoalan]

Add check to MachineVerifier. Remove explicit debug operand flag uses.

[StephenTozer]

Very nice cleanup!

Also curious if invariant debug register operands are applicable to other debug instructions (DBG_INSTR_REF, DBG_PHI, DBG_LABEL)

I think that this change should probably include DBG_PHIs as well - you could probably just replace isDebugValue() with isDebugInstr(), as any current or future debug instructions that use registers should have them set isDebug.

[jackoalan]

Apply to all debug instructions according to MachineInstr::isDebugInstr

[MatzeB]

Thanks, great work cleaning all up all the use sites! I have some comments on the verifier changes, then this will be good to go.

llvm/lib/CodeGen/MachineVerifier.cpp

1894–1895

• Use isDebugInstr() here as well and tweak message.
• Also check the reverse: non-debug instructions must not use the debug flag.
• Also test for isUse() for good measure (even though it should not be possible for debug instruction to ever define regs).

1963–1966

It seems a similar check already existed but was incorrectly only put into the isPhysicalRegister() branch... We can drop it now I guess.

[jackoalan]

Remove now-redundant MachineVerifier check. Also check debug instructions on uses only and non-debug instruction case.

[MatzeB]

LGTM

[jackoalan]

Thank you for the feedback! Would you mind committing this? --author="Jack Andersen <[e-mail]>"

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

MachineInstrBuilder emitLoc(Optional<LocIdx> MLoc, const DebugVariable &Var,
     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 DIExpression *Expr = Properties.DIExpr;
     if (!MLoc) {
       // No location -> DBG_VALUE $noreg
-      MIB.addReg(0, RegState::Debug);
-      MIB.addReg(0, RegState::Debug);
+      MIB.addReg(0);
+      MIB.addReg(0);
     } else if (LocIdxToLocID[*MLoc] >= NumRegs) {
       unsigned LocID = LocIdxToLocID[*MLoc];
       const SpillLoc &Spill = SpillLocs[LocID - NumRegs + 1];
 
       auto *TRI = MF.getSubtarget().getRegisterInfo();
       Expr = TRI->prependOffsetExpression(Expr, DIExpression::ApplyOffset,
                                           Spill.SpillOffset);
       unsigned Base = Spill.SpillBase;
-      MIB.addReg(Base, RegState::Debug);
+      MIB.addReg(Base);
       MIB.addImm(0);
     } else {
       unsigned LocID = LocIdxToLocID[*MLoc];
-      MIB.addReg(LocID, RegState::Debug);
+      MIB.addReg(LocID);
       if (Properties.Indirect)
         MIB.addImm(0);
       else
-        MIB.addReg(0, RegState::Debug);
+        MIB.addReg(0);
     }
 
     MIB.addMetadata(Var.getVariable());
     MIB.addMetadata(Expr);
     return MIB;
   }
 };
 
bool recoverAsEntryValue(const DebugVariable &Var, DbgValueProperties &Prop,
     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);
-    MO.setIsDebug(true);
 
     PendingDbgValues.push_back(emitMOLoc(MO, Var, {NewExpr, Prop.Indirect}));
     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(),

llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp

MachineInstr *BuildDbgValue(MachineFunction &MF) const {
           // one from the entry DBG_VALUE, it does not matter if the entry value
           // was copied in to another register due to some optimizations.
           // Non-entry value register locations are like the source
           // DBG_VALUE, but with the register number from this VarLoc.
           MOs.push_back(MachineOperand::CreateReg(
               EVKind == EntryValueLocKind::EntryValueKind ? Orig.getReg()
                                                           : Register(Loc.RegNo),
               false));
-          MOs.back().setIsDebug();
           break;
         case MachineLocKind::SpillLocKind: {
           // Spills are indirect DBG_VALUEs, with a base register and offset.
           // Use the original DBG_VALUEs expression to build the spilt location
           // on top of. FIXME: spill locations created before this pass runs
           // are not recognized, and not handled here.
           unsigned Base = Loc.SpillLocation.SpillBase;
           auto *TRI = MF.getSubtarget().getRegisterInfo();
           if (MI.isNonListDebugValue()) {
             auto Deref = Indirect ? DIExpression::DerefAfter : 0;
             DIExpr = TRI->prependOffsetExpression(
                 DIExpr, DIExpression::ApplyOffset | Deref,
                 Loc.SpillLocation.SpillOffset);
             Indirect = true;
           } else {
             SmallVector<uint64_t, 4> Ops;
             TRI->getOffsetOpcodes(Loc.SpillLocation.SpillOffset, Ops);
             Ops.push_back(dwarf::DW_OP_deref);
             DIExpr = DIExpression::appendOpsToArg(DIExpr, Ops, I);
           }
           MOs.push_back(MachineOperand::CreateReg(Base, false));
-          MOs.back().setIsDebug();
           break;
         }
         case MachineLocKind::ImmediateKind: {
           MOs.push_back(Orig);
           break;
         }
         case MachineLocKind::InvalidKind:
           llvm_unreachable("Tried to produce DBG_VALUE for invalid VarLoc");

llvm/lib/CodeGen/MIRParser/MIParser.cpp

while (!Token.isNewlineOrEOF() && Token.isNot(MIToken::kw_pre_instr_symbol) &&
          Token.isNot(MIToken::kw_heap_alloc_marker) &&
          Token.isNot(MIToken::kw_debug_location) &&
          Token.isNot(MIToken::kw_debug_instr_number) &&
          Token.isNot(MIToken::coloncolon) && Token.isNot(MIToken::lbrace)) {
     auto Loc = Token.location();
     Optional<unsigned> TiedDefIdx;
     if (parseMachineOperandAndTargetFlags(OpCode, Operands.size(), MO, TiedDefIdx))
       return true;
-    if ((OpCode == TargetOpcode::DBG_VALUE ||
-         OpCode == TargetOpcode::DBG_VALUE_LIST) &&
-        MO.isReg())
-      MO.setIsDebug();
     Operands.push_back(
         ParsedMachineOperand(MO, Loc, Token.location(), TiedDefIdx));
     if (Token.isNewlineOrEOF() || Token.is(MIToken::coloncolon) ||
         Token.is(MIToken::lbrace))
       break;
     if (Token.isNot(MIToken::comma))
       return error("expected ',' before the next machine operand");
     lex();

llvm/lib/CodeGen/MachineFunction.cpp

 
 void MachineFunction::finalizeDebugInstrRefs() {
   auto *TII = getSubtarget().getInstrInfo();
 
   auto MakeDbgValue = [&](MachineInstr &MI) {
     const MCInstrDesc &RefII = TII->get(TargetOpcode::DBG_VALUE);
     MI.setDesc(RefII);
     MI.getOperand(1).ChangeToRegister(0, false);
-    MI.getOperand(0).setIsDebug();
   };
 
   if (!useDebugInstrRef())
     return;
 
   for (auto &MBB : *this) {
     for (auto &MI : MBB) {
       if (!MI.isDebugRef() || !MI.getOperand(0).isReg())

llvm/lib/CodeGen/MachineInstr.cpp

if (!isImpReg) {
         int DefIdx = MCID->getOperandConstraint(OpNo, MCOI::TIED_TO);
         if (DefIdx != -1)
           tieOperands(DefIdx, OpNo);
       }
       // If the register operand is flagged as early, mark the operand as such.
       if (MCID->getOperandConstraint(OpNo, MCOI::EARLY_CLOBBER) != -1)
         NewMO->setIsEarlyClobber(true);
     }
+    // Ensure DBG_VALUE* sets RegState::Debug on all register use operands.
+    if (NewMO->isUse() && isDebugValue())
+      NewMO->setIsDebug();
   }
 }
 
 /// RemoveOperand - Erase an operand  from an instruction, leaving it with one
 /// fewer operand than it started with.
 ///
 void MachineInstr::RemoveOperand(unsigned OpNo) {
   assert(OpNo < getNumOperands() && "Invalid operand number");

 MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,
                                   const MCInstrDesc &MCID, bool IsIndirect,
                                   Register Reg, const MDNode *Variable,
                                   const MDNode *Expr) {
   assert(isa<DILocalVariable>(Variable) && "not a variable");
   assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
   assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
-  auto MIB = BuildMI(MF, DL, MCID).addReg(Reg, RegState::Debug);
+  auto MIB = BuildMI(MF, DL, MCID).addReg(Reg);
   if (IsIndirect)
     MIB.addImm(0U);
   else
-    MIB.addReg(0U, RegState::Debug);
+    MIB.addReg(0U);
   return MIB.addMetadata(Variable).addMetadata(Expr);
 }
 
 MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,
                                   const MCInstrDesc &MCID, bool IsIndirect,
                                   const MachineOperand &MO,
                                   const MDNode *Variable, const MDNode *Expr) {
   assert(isa<DILocalVariable>(Variable) && "not a variable");
   assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
   assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   if (MO.isReg())
     return BuildMI(MF, DL, MCID, IsIndirect, MO.getReg(), Variable, Expr);
 
   auto MIB = BuildMI(MF, DL, MCID).add(MO);
   if (IsIndirect)
     MIB.addImm(0U);
   else
-    MIB.addReg(0U, RegState::Debug);
+    MIB.addReg(0U);
   return MIB.addMetadata(Variable).addMetadata(Expr);
 }
 
 MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,
                                   const MCInstrDesc &MCID, bool IsIndirect,
                                   ArrayRef<MachineOperand> MOs,
                                   const MDNode *Variable, const MDNode *Expr) {
   assert(isa<DILocalVariable>(Variable) && "not a variable");
   assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
   assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   if (MCID.Opcode == TargetOpcode::DBG_VALUE)
     return BuildMI(MF, DL, MCID, IsIndirect, MOs[0], Variable, Expr);
 
   auto MIB = BuildMI(MF, DL, MCID);
   MIB.addMetadata(Variable).addMetadata(Expr);
   for (const MachineOperand &MO : MOs)
     if (MO.isReg())
-      MIB.addReg(MO.getReg(), RegState::Debug);
+      MIB.addReg(MO.getReg());
     else
       MIB.add(MO);
   return MIB;
 }
 
 MachineInstrBuilder llvm::BuildMI(MachineBasicBlock &BB,
                                   MachineBasicBlock::iterator I,
                                   const DebugLoc &DL, const MCInstrDesc &MCID,

llvm/lib/CodeGen/MachineOperand.cpp

void MachineOperand::ChangeToRegister(Register Reg, bool isDef, bool isImp,
   if (MachineFunction *MF = getMFIfAvailable(*this))
     RegInfo = &MF->getRegInfo();
   // If this operand is already a register operand, remove it from the
   // register's use/def lists.
   bool WasReg = isReg();
   if (RegInfo && WasReg)
     RegInfo->removeRegOperandFromUseList(this);
 
+  // Ensure DBG_VALUE* sets RegState::Debug on all register use operands.
+  const MachineInstr *MI = getParent();
+  if (!isDef && MI && MI->isDebugValue())
+    isDebug = true;
+
   // Change this to a register and set the reg#.
   assert(!(isDead && !isDef) && "Dead flag on non-def");
   assert(!(isKill && isDef) && "Kill flag on def");
   OpKind = MO_Register;
   SmallContents.RegNo = Reg;
   SubReg_TargetFlags = 0;
   IsDef = isDef;
   IsImp = isImp;

llvm/lib/CodeGen/MachineVerifier.cpp

MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
   } else {
     // ARM adds %reg0 operands to indicate predicates. We'll allow that.
     if (MO->isReg() && !MO->isImplicit() && !MI->isVariadic() && MO->getReg())
       report("Extra explicit operand on non-variadic instruction", MO, MONum);
   }
 
   switch (MO->getType()) {
   case MachineOperand::MO_Register: {
+    // Verify debug state on DBG_VALUE* instructions. Check this first because
+    // reg0 indicates an undefined debug value.
+    if (MI->isDebugValue() && !MO->isDebug())
+      report("DBG_VALUE register operands must be debug", MO, MONum);
+
     const Register Reg = MO->getReg();
     if (!Reg)
       return;
     if (MRI->tracksLiveness() && !MI->isDebugValue())
       checkLiveness(MO, MONum);
 
     // Verify the consistency of tied operands.
     if (MO->isTied()) {
if (Register::isPhysicalRegister(Reg)) {
         }
       }
       if (MO->isRenamable()) {
         if (MRI->isReserved(Reg)) {
           report("isRenamable set on reserved register", MO, MONum);
           return;
         }
       }
       if (MI->isDebugValue() && MO->isUse() && !MO->isDebug()) {
         report("Use-reg is not IsDebug in a DBG_VALUE", MO, MONum);
         return;
       }
     } else {
       // Virtual register.
       const TargetRegisterClass *RC = MRI->getRegClassOrNull(Reg);
       if (!RC) {
         // This is a generic virtual register.
 
         // Do not allow undef uses for generic virtual registers. This ensures
         // getVRegDef can never fail and return null on a generic register.

llvm/lib/CodeGen/PrologEpilogInserter.cpp

for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
             " machine instruction");
         Register Reg;
         unsigned FrameIdx = Op.getIndex();
         unsigned Size = MF.getFrameInfo().getObjectSize(FrameIdx);
 
         StackOffset Offset =
             TFI->getFrameIndexReference(MF, FrameIdx, Reg);
         Op.ChangeToRegister(Reg, false /*isDef*/);
-        Op.setIsDebug();
 
         const DIExpression *DIExpr = MI.getDebugExpression();
 
         // If we have a direct DBG_VALUE, and its location expression isn't
         // currently complex, then adding an offset will morph it into a
         // complex location that is interpreted as being a memory address.
         // This changes a pointer-valued variable to dereference that pointer,
         // which is incorrect. Fix by adding DW_OP_stack_value.

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

void InstrEmitter::AddDbgValueLocationOps(
     ArrayRef<SDDbgOperand> LocationOps,
     DenseMap<SDValue, Register> &VRBaseMap) {
   for (const SDDbgOperand &Op : LocationOps) {
     switch (Op.getKind()) {
     case SDDbgOperand::FRAMEIX:
       MIB.addFrameIndex(Op.getFrameIx());
       break;
     case SDDbgOperand::VREG:
-      MIB.addReg(Op.getVReg(), RegState::Debug);
+      MIB.addReg(Op.getVReg());
       break;
     case SDDbgOperand::SDNODE: {
       SDValue V = SDValue(Op.getSDNode(), Op.getResNo());
       // It's possible we replaced this SDNode with other(s) and therefore
       // didn't generate code for it. It's better to catch these cases where
       // they happen and transfer the debug info, but trying to guarantee that
       // in all cases would be very fragile; this is a safeguard for any
       // that were missed.
MachineInstr *InstrEmitter::EmitDbgNoLocation(SDDbgValue *SD) {
   // An invalidated SDNode must generate an undef DBG_VALUE: although the
   // original value is no longer computed, earlier DBG_VALUEs live ranges
   // must not leak into later code.
   MDNode *Var = SD->getVariable();
   MDNode *Expr = SD->getExpression();
   DebugLoc DL = SD->getDebugLoc();
   auto MIB = BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE));
   MIB.addReg(0U);
-  MIB.addReg(0U, RegState::Debug);
+  MIB.addReg(0U);
   MIB.addMetadata(Var);
   MIB.addMetadata(Expr);
   return &*MIB;
 }
 
 MachineInstr *
 InstrEmitter::EmitDbgValueFromSingleOp(SDDbgValue *SD,
                                        DenseMap<SDValue, Register> &VRBaseMap) {
InstrEmitter::EmitDbgValueFromSingleOp(SDDbgValue *SD,
   // Emit non-variadic dbg_value nodes as DBG_VALUE.
   // DBG_VALUE := "DBG_VALUE" loc, isIndirect, var, expr
   auto MIB = BuildMI(*MF, DL, II);
   AddDbgValueLocationOps(MIB, II, LocationOps, VRBaseMap);
 
   if (SD->isIndirect())
     MIB.addImm(0U);
   else
-    MIB.addReg(0U, RegState::Debug);
+    MIB.addReg(0U);
 
   return MIB.addMetadata(Var).addMetadata(Expr);
 }
 
 MachineInstr *
 InstrEmitter::EmitDbgLabel(SDDbgLabel *SD) {
   MDNode *Label = SD->getLabel();
   DebugLoc DL = SD->getDebugLoc();

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

for (MachineBasicBlock &MBB : MF) {
       if (!SpillFIs.empty() && SeenDbgInstr) {
         // FIXME: The dead frame indices are replaced with a null register from
         // the debug value instructions. We should instead, update it with the
         // correct register value. But not sure the register value alone is
         for (MachineInstr &MI : MBB) {
           if (MI.isDebugValue() && MI.getOperand(0).isFI() &&
               SpillFIs[MI.getOperand(0).getIndex()]) {
             MI.getOperand(0).ChangeToRegister(Register(), false /*isDef*/);
-            MI.getOperand(0).setIsDebug();
           }
         }
       }
     }
   }
 
   FuncInfo->removeDeadFrameIndices(MFI);
   assert(allSGPRSpillsAreDead(MF) &&

llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp

for (MachineBasicBlock &MBB : MF) {
       // FIXME: The dead frame indices are replaced with a null register from
       // the debug value instructions. We should instead, update it with the
       // correct register value. But not sure the register value alone is
       // adequate to lower the DIExpression. It should be worked out later.
       for (MachineInstr &MI : MBB) {
         if (MI.isDebugValue() && MI.getOperand(0).isFI() &&
             SpillFIs[MI.getOperand(0).getIndex()]) {
           MI.getOperand(0).ChangeToRegister(Register(), false /*isDef*/);
-          MI.getOperand(0).setIsDebug();
         }
       }
     }
 
     MadeChange = true;
   } else if (FuncInfo->VGPRReservedForSGPRSpill) {
     FuncInfo->removeVGPRForSGPRSpill(FuncInfo->VGPRReservedForSGPRSpill, MF);
   }

llvm/lib/Target/NVPTX/NVPTXPrologEpilogPass.cpp

for (MachineInstr &MI : MBB) {
           assert(
               MI.isDebugOperand(&Op) &&
               "Frame indices can only appear as a debug operand in a DBG_VALUE*"
               " machine instruction");
           Register Reg;
           auto Offset =
               TFI.getFrameIndexReference(MF, Op.getIndex(), Reg);
           Op.ChangeToRegister(Reg, /*isDef=*/false);
-          Op.setIsDebug();
           const DIExpression *DIExpr = MI.getDebugExpression();
           if (MI.isNonListDebugValue()) {
             DIExpr = TRI.prependOffsetExpression(MI.getDebugExpression(), DIExpression::ApplyOffset, Offset);
           } else {
       SmallVector<uint64_t, 3> Ops;
             TRI.getOffsetOpcodes(Offset, Ops);
             unsigned OpIdx = MI.getDebugOperandIndex(&Op);
             DIExpr = DIExpression::appendOpsToArg(DIExpr, Ops, OpIdx);

llvm/lib/Target/WebAssembly/WebAssemblyReplacePhysRegs.cpp

for (MachineOperand &MO :
             FI->setFrameBaseVreg(VReg);
             LLVM_DEBUG({
               dbgs() << "replacing preg " << PReg << " with " << VReg << " ("
                      << Register::virtReg2Index(VReg) << ")\n";
             });
           }
         }
         MO.setReg(VReg);
-        if (MO.getParent()->isDebugValue())
-          MO.setIsDebug();
         Changed = true;
       }
     }
   }
 
   return Changed;
 }

llvm/lib/Target/X86/X86FastISel.cpp

bool X86FastISel::fastLowerIntrinsicCall(const IntrinsicInst *II) {
   }
   case Intrinsic::dbg_declare: {
     const DbgDeclareInst *DI = cast<DbgDeclareInst>(II);
     X86AddressMode AM;
     assert(DI->getAddress() && "Null address should be checked earlier!");
     if (!X86SelectAddress(DI->getAddress(), AM))
       return false;
     const MCInstrDesc &II = TII.get(TargetOpcode::DBG_VALUE);
-    // FIXME may need to add RegState::Debug to any registers produced,
-    // although ESP/EBP should be the only ones at the moment.
     assert(DI->getVariable()->isValidLocationForIntrinsic(DbgLoc) &&
            "Expected inlined-at fields to agree");
     addFullAddress(BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II), AM)
         .addImm(0)
         .addMetadata(DI->getVariable())
         .addMetadata(DI->getExpression());
     return true;
   }

llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp

TEST_F(AArch64GISelMITest, MatchMiscellaneous) {
   Register Reg = MIBAdd.getReg(0);
 
   // Only one use of Reg.
   B.buildCast(LLT::pointer(0, 32), MIBAdd);
   EXPECT_TRUE(mi_match(Reg, *MRI, m_OneUse(m_GAdd(m_Reg(), m_Reg()))));
   EXPECT_TRUE(mi_match(Reg, *MRI, m_OneNonDBGUse(m_GAdd(m_Reg(), m_Reg()))));
 
   // Add multiple debug uses of Reg.
-  B.buildInstr(TargetOpcode::DBG_VALUE, {}, {Reg})->getOperand(0).setIsDebug();
-  B.buildInstr(TargetOpcode::DBG_VALUE, {}, {Reg})->getOperand(0).setIsDebug();
+  B.buildInstr(TargetOpcode::DBG_VALUE, {}, {Reg});
+  B.buildInstr(TargetOpcode::DBG_VALUE, {}, {Reg});
 
   EXPECT_FALSE(mi_match(Reg, *MRI, m_OneUse(m_GAdd(m_Reg(), m_Reg()))));
   EXPECT_TRUE(mi_match(Reg, *MRI, m_OneNonDBGUse(m_GAdd(m_Reg(), m_Reg()))));
 
   // Multiple non-debug uses of Reg.
   B.buildCast(LLT::pointer(1, 32), MIBAdd);
   EXPECT_FALSE(mi_match(Reg, *MRI, m_OneUse(m_GAdd(m_Reg(), m_Reg()))));
   EXPECT_FALSE(mi_match(Reg, *MRI, m_OneNonDBGUse(m_GAdd(m_Reg(), m_Reg()))));

llvm/unittests/CodeGen/MachineInstrTest.cpp

TEST(MachineInstrExtraInfo, RemoveExtraInfo) {
 
   MI->setMemRefs(*MF, {});
   ASSERT_TRUE(MI->memoperands_empty());
   ASSERT_FALSE(MI->getPreInstrSymbol());
   ASSERT_FALSE(MI->getPostInstrSymbol());
   ASSERT_FALSE(MI->getHeapAllocMarker());
 }
 
+TEST(MachineInstrDebugValue, AddDebugValueOperand) {
+  LLVMContext Ctx;
+  Module Mod("Module", Ctx);
+  auto MF = createMachineFunction(Ctx, Mod);
+
+  for (const unsigned short Opcode :
+       {TargetOpcode::DBG_VALUE, TargetOpcode::DBG_VALUE_LIST}) {
+    const MCInstrDesc MCID = {
+        Opcode, 0,       0,
+        0,      0,       (1ULL << MCID::Pseudo) | (1ULL << MCID::Variadic),
+        0,      nullptr, nullptr,
+        nullptr};
+
+    auto *MI = MF->CreateMachineInstr(MCID, DebugLoc());
+    MI->addOperand(*MF, MachineOperand::CreateReg(0, /*isDef*/ false));
+
+    MI->addOperand(*MF, MachineOperand::CreateImm(0));
+    MI->getOperand(1).ChangeToRegister(0, false);
+
+    ASSERT_TRUE(MI->getOperand(0).isDebug());
+    ASSERT_TRUE(MI->getOperand(1).isDebug());
+  }
+}
+
 static_assert(std::is_trivially_copyable<MCOperand>::value,
               "trivially copyable");
 
 } // end namespace