[GlobalISel] Allow DBG_VALUE to use undefined vregs before LiveDebugValues

[jackoalan]

Summary

Expanding on D109750.

Since DBG_VALUE instructions have final register validity determined in
LDVImpl::handleDebugValue, there is no apparent reason to immediately prune
unused register operands as their defs are erased. Consequently, this renders
MachineInstr::eraseFromParentAndMarkDBGValuesForRemoval moot; gaining a
substantial performance improvement.

The only necessary changes involve making relevant passes consider invalid
DBG_VALUE vregs uses as valid.

[jackoalan]

Update comment

[MatzeB]

No final verdict for this change yet. But you should try to use a more specific wording than "invalid vreg"; is it a vreg without definition?

[MatzeB]

Before we weaken the verifier, do we have an idea how much benefit we get from allowing these half-initialize vregs? Is searching for these DBG_VALUE at the end of GlobalISel and remove them there an option?

[jackoalan]

Specifically check for vregs without defs when suppressing verification errors.

[jackoalan]

In D112852#3098095, @MatzeB wrote:

Before we weaken the verifier

The weakening conditions are now much more targeted to this specific case.

do we have an idea how much benefit we get from allowing these half-initialize vregs?

In terms of benefits, D109750 mentions a 0.5% codegen time reduction, but its effects are limited to the legalizer. This goes further by replacing all calls to eraseFromParentAndMarkDBGValuesForRemoval with eraseFromParent.

Is searching for these DBG_VALUE at the end of GlobalISel and remove them there an option?

The issue here is determining where the barrier for disallowing these undefined vregs should be. LiveDebugVariables naturally discards these particular uses, so this is sufficient based on my knowledge.

[MatzeB]

Requesting changes for the missing MIR serialization of the new property.

The issue here is determining where the barrier for disallowing these undefined vregs should be. LiveDebugVariables naturally discards these particular uses, so this is sufficient based on my knowledge.

Well to me this feels like we are making the intermediate representation more complicated (I consider it more complicated because DBG_VALUE instructions are special with lifetime rules now) for a very small gain.

That said I don't feel strongly about this. If others think this is benefitial then we can go ahead.

llvm/include/llvm/CodeGen/MachineFunction.h

166

New properties must be added to MIRYamlMapping.h / MIRParser.cpp

[MatzeB]

llvm/include/llvm/CodeGen/MachineFunction.h

153–155

Please document the new property here

166

FWIW: It seems there are others missing as well already :-(

[jackoalan]

In D112852#3146936, @MatzeB wrote:

That said I don't feel strongly about this. If others think this is benefitial then we can go ahead.

I agree this change should be open for comment. If it is decided to not be worth it, D109750 should be reverted (the test case provided here fails due to the changes made there).

[aemerson]

In D112852#3146936, @MatzeB wrote:

Requesting changes for the missing MIR serialization of the new property.

The issue here is determining where the barrier for disallowing these undefined vregs should be. LiveDebugVariables naturally discards these particular uses, so this is sufficient based on my knowledge.

Well to me this feels like we are making the intermediate representation more complicated (I consider it more complicated because DBG_VALUE instructions are special with lifetime rules now) for a very small gain.

That said I don't feel strongly about this. If others think this is benefitial then we can go ahead.

We haven’t changed the meaning of the MIR, we just clarified the existing semantics on the RFC thread. There’s no middle ground here as far as I can see, either it’s valid to have undefined uses or not. If it is, then we’re free to do anything that leaves them around.

[jmorse]

Drive by comment,

Possibly relevant to the interests of this thread, SelectionDAG will already produce half-formed DBG_VALUEs in it's current implementation. The regression test in the recently landed rGb8f68ad9cdb, if run with

llc llvm/test/DebugInfo/X86/instr-ref-sdag-empty-vreg.ll -o - -stop-before=finalize-isel -experimental-debug-variable-locations=false

produces:

body:             |
  bb.0.cond.false.i:
    successors: %bb.1(0x80000000)


  bb.1._ZN4Vec39normalizeEv.exit:
    DBG_VALUE %1:fr32, $noreg, !11, !DIExpression(), debug-location !12
    RET 0, debug-location !12

due to the argument value being optimised out late, leaving a DBG_VALUE referring to an undefined vreg. So I suppose you could say the approach discussed here has precedent.

I wasn't aware that SelectionDAG behaved like this until quite recently. NB: the -experimental...=false flag is the current llc default, I've added it because it might change in the next week or two.

[jackoalan]

• Rename DebugValuesAllocated -> TracksDebugUserValues
• Documenting comment for TracksDebugUserValues
• MIR key for TracksDebugUserValues
• TracksDebugUserValues tests for MachineVerifier
• LiveDebugValues test to ensure undefined debug uses delete DBG_VALUE instructions

[jackoalan]

Clarify TracksDebugUserValues documenting comment.

[MatzeB]

Technical side seems fine. LGTM

llvm/include/llvm/CodeGen/MachineFunction.h

155–158

I'd primary describe the properties as constraints on the MIR. (And only mention passes secondary to motivate things):

TracksDebugUserValues: Without this property enabled, debug instructions such as DBG_VALUE are allowed to reference virtual registers even if those registers do not have a definition. With the property enabled virtual registers must only be used if they have a definition. This property allows earlier passes in the pipeline to skip updates of `DBG_VALUE` instructions to save compile time.

[jackoalan]

Rebase, Change TracksDebugUserValues comment per review request

[jackoalan]

Thanks everyone for the comments. May I have somebody commit this for me?
--author="Jack Andersen <[e-mail]>"

[bogner]

llvm/lib/CodeGen/MachineVerifier.cpp

1998

Is this check in the right place? I'm seeing "Generic virtual register use cannot be undef" (just above) errors after this change on an out of tree target.

[jackoalan]

llvm/lib/CodeGen/MachineVerifier.cpp

1998

Does the error occur after the Debug Variable Analysis pass on a debug instruction?

If possible, it would be really helpful if a test could be created that triggers the error on the main branch.

[bogner]

llvm/lib/CodeGen/MachineVerifier.cpp

1998

The error occurs before the Debug Variable Analysis pass runs when running llc with -verify-machineinstrs, since that runs the machine verifier between each pass. I can try to see if I can find a reproducer for an in-tree backend, but I figured I'd sanity check this first, given that the comments says "Debug value instruction is permitted to use undefined vregs.", but the check that rejects undef vregs happens right before this.

[jackoalan]

llvm/lib/CodeGen/MachineVerifier.cpp

1998

Okay, I see what you're referring to.

The purpose of this added if block is to weaken the verifier just for DBG_VALUE register uses; making it convenient to erase instructions that define these values but then not explicitly undef-ing them on the DBG_VALUE use (i.e. just a dangling vreg). In this line of thinking, it would not be necessary to also weaken MO->isUndef (which only tests for explicit undefs).

Does the error contain a dump of the instruction? If it is indeed a DBG_VALUE or DBG_VALUE_LIST instruction, then MO->isUndef() should be weakened as well if it is determined there is no other root cause. If it is not a DBG_VALUE instruction, then it would not be applicable and the root cause must be elsewhere.

A triggering test would be the best way to determine this collaboratively.

[bogner]

llvm/lib/CodeGen/MachineVerifier.cpp

1998

The errors look something like this:

*** Bad machine code: Generic virtual register use cannot be undef ***
- function:    xyz
- basic block: %bb.4 while.end64.i.i (0x7fdf95dc8c48)
- instruction: DBG_VALUE undef %2423:gpr, $noreg, !"x", !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), debug-location !3302; /...:184:30 @[ program_source:273:69 @[ program_source:0 ] ] line no:184
- operand 0:   undef %2423:grp

Let me see if I can reduce this test to something shareable.

[jackoalan]

llvm/lib/CodeGen/MachineVerifier.cpp

1998

Have you tried using bugpoint to reduce your crashing function to something you can share? You may have to anonymize identifiers, but it usually does a good job minimizing the IR to just the blocks and instructions necessary to crash.

I’d be happy to weaken that undef test for DBG_VALUE, but It’d be ideal to justify it with a test if that change is made.

llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

private:
   /// process instructions which have been marked dead, but otherwise break the
   /// MIR by introducing multiple vreg defs. For those cases, allow the combines
   /// to explicitly delete the instructions before we run into trouble.
   void deleteMarkedDeadInsts(SmallVectorImpl<MachineInstr *> &DeadInsts,
                              GISelObserverWrapper &WrapperObserver) {
     for (auto *DeadMI : DeadInsts) {
       LLVM_DEBUG(dbgs() << *DeadMI << "Is dead, eagerly deleting\n");
       WrapperObserver.erasingInstr(*DeadMI);
-      DeadMI->eraseFromParentAndMarkDBGValuesForRemoval();
+      DeadMI->eraseFromParent();
     }
     DeadInsts.clear();
   }
 
   /// Checks if the target legalizer info has specified anything about the
   /// instruction, or if unsupported.
   bool isInstUnsupported(const LegalityQuery &Query) const {
     using namespace LegalizeActions;

llvm/include/llvm/CodeGen/MachineFunction.h

public:
   //  target-specific or non-pre-isel generic instructions (e.g., COPY).
   //  Since only pre-isel generic instructions can have generic virtual register
   //  operands, this also means that all generic virtual registers have been
   //  constrained to virtual registers (assigned to register classes) and that
   //  all sizes attached to them have been eliminated.
   // TiedOpsRewritten: The twoaddressinstruction pass will set this flag, it
   //  means that tied-def have been rewritten to meet the RegConstraint.
   // FailsVerification: Means that the function is not expected to pass machine
   //  verification. This can be set by passes that introduce known problems that
   //  have not been fixed yet.
   enum class Property : unsigned {
     IsSSA,
     NoPHIs,
     TracksLiveness,
     NoVRegs,
     FailedISel,
     Legalized,
     RegBankSelected,
     Selected,
     TiedOpsRewritten,
     FailsVerification,
-    LastProperty = FailsVerification,
+    DebugValuesAllocated,
+    LastProperty = DebugValuesAllocated,
   };
 
   bool hasProperty(Property P) const {
     return Properties[static_cast<unsigned>(P)];
   }
 
   MachineFunctionProperties &set(Property P) {
     Properties.set(static_cast<unsigned>(P));

llvm/include/llvm/CodeGen/MachineInstr.h

public:
 
   /// Unlink 'this' from the containing basic block and delete it.
   ///
   /// If this instruction is the header of a bundle, the whole bundle is erased.
   /// This function can not be used for instructions inside a bundle, use
   /// eraseFromBundle() to erase individual bundled instructions.
   void eraseFromParent();
 
-  /// Unlink 'this' from the containing basic block and delete it.
-  ///
-  /// For all definitions mark their uses in DBG_VALUE nodes
-  /// as undefined. Otherwise like eraseFromParent().
-  void eraseFromParentAndMarkDBGValuesForRemoval();
-
   /// Unlink 'this' form its basic block and delete it.
   ///
   /// If the instruction is part of a bundle, the other instructions in the
   /// bundle remain bundled.
   void eraseFromBundle();
 
   bool isEHLabel() const { return getOpcode() == TargetOpcode::EH_LABEL; }
   bool isGCLabel() const { return getOpcode() == TargetOpcode::GC_LABEL; }

llvm/lib/CodeGen/DeadMachineInstructionElim.cpp

for (MachineBasicBlock *MBB : post_order(&MF)) {
 
     // Now scan the instructions and delete dead ones, tracking physreg
     // liveness as we go.
     for (MachineInstr &MI : llvm::make_early_inc_range(llvm::reverse(*MBB))) {
       // If the instruction is dead, delete it!
       if (isDead(&MI)) {
         LLVM_DEBUG(dbgs() << "DeadMachineInstructionElim: DELETING: " << MI);
         // It is possible that some DBG_VALUE instructions refer to this
-        // instruction.  They get marked as undef and will be deleted
-        // in the live debug variable analysis.
-        MI.eraseFromParentAndMarkDBGValuesForRemoval();
+        // instruction. They will be deleted in the live debug variable
+        // analysis.
+        MI.eraseFromParent();
         AnyChanges = true;
         ++NumDeletes;
         continue;
       }
 
       // Record the physreg defs.
       for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
         const MachineOperand &MO = MI.getOperand(i);

llvm/lib/CodeGen/GlobalISel/Combiner.cpp

if (CSEInfo)
       WrapperObserver.addObserver(CSEInfo);
     RAIIDelegateInstaller DelInstall(MF, &WrapperObserver);
     for (MachineBasicBlock *MBB : post_order(&MF)) {
       for (MachineInstr &CurMI :
            llvm::make_early_inc_range(llvm::reverse(*MBB))) {
         // Erase dead insts before even adding to the list.
         if (isTriviallyDead(CurMI, *MRI)) {
           LLVM_DEBUG(dbgs() << CurMI << "Is dead; erasing.\n");
-          CurMI.eraseFromParentAndMarkDBGValuesForRemoval();
+          CurMI.eraseFromParent();
           continue;
         }
         WorkList.deferred_insert(&CurMI);
       }
     }
     WorkList.finalize();
     // Main Loop. Process the instructions here.
     while (!WorkList.empty()) {

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

Register Shift2 = Builder
                         .buildInstr(Opcode, {DestType},
                                     {MatchInfo.LogicNonShiftReg, Shift2Const})
                         .getReg(0);
 
   Register Dest = MI.getOperand(0).getReg();
   Builder.buildInstr(MatchInfo.Logic->getOpcode(), {Dest}, {Shift1, Shift2});
 
   // These were one use so it's safe to remove them.
-  MatchInfo.Shift2->eraseFromParentAndMarkDBGValuesForRemoval();
-  MatchInfo.Logic->eraseFromParentAndMarkDBGValuesForRemoval();
+  MatchInfo.Shift2->eraseFromParent();
+  MatchInfo.Logic->eraseFromParent();
 
   MI.eraseFromParent();
 }
 
 bool CombinerHelper::matchCombineMulToShl(MachineInstr &MI,
                                           unsigned &ShiftVal) {
   assert(MI.getOpcode() == TargetOpcode::G_MUL && "Expected a G_MUL");
   auto MaybeImmVal =

llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp

#endif
         --MII;
 
       LLVM_DEBUG(dbgs() << "Selecting: \n  " << MI);
 
       // We could have folded this instruction away already, making it dead.
       // If so, erase it.
       if (isTriviallyDead(MI, MRI)) {
         LLVM_DEBUG(dbgs() << "Is dead; erasing.\n");
-        MI.eraseFromParentAndMarkDBGValuesForRemoval();
+        MI.eraseFromParent();
         continue;
       }
 
       // Eliminate hints.
       if (isPreISelGenericOptimizationHint(MI.getOpcode())) {
         Register DstReg = MI.getOperand(0).getReg();
         Register SrcReg = MI.getOperand(1).getReg();
 
#ifndef NDEBUG
   // that the size of the now-constrained vreg is unchanged and that it has a
   // register class.
   for (unsigned I = 0, E = MRI.getNumVirtRegs(); I != E; ++I) {
     unsigned VReg = Register::index2VirtReg(I);
 
     MachineInstr *MI = nullptr;
     if (!MRI.def_empty(VReg))
       MI = &*MRI.def_instr_begin(VReg);
-    else if (!MRI.use_empty(VReg))
+    else if (!MRI.use_empty(VReg)) {
       MI = &*MRI.use_instr_begin(VReg);
+      // Debug value instruction is permitted to use undefined vregs.
+      if (MI->isDebugValue())
+        continue;
+    }
     if (!MI)
       continue;
 
     const TargetRegisterClass *RC = MRI.getRegClassOrNull(VReg);
     if (!RC) {
       reportGISelFailure(MF, TPC, MORE, "gisel-select",
                          "VReg has no regclass after selection", *MI);
       return false;

llvm/lib/CodeGen/GlobalISel/Utils.cpp

 
 bool llvm::shouldOptForSize(const MachineBasicBlock &MBB,
                             ProfileSummaryInfo *PSI, BlockFrequencyInfo *BFI) {
   const auto &F = MBB.getParent()->getFunction();
   return F.hasOptSize() || F.hasMinSize() ||
          llvm::shouldOptimizeForSize(MBB.getBasicBlock(), PSI, BFI);
 }
 
-/// These artifacts generally don't have any debug users because they don't
-/// directly originate from IR instructions, but instead usually from
-/// legalization. Avoiding checking for debug users improves compile time.
-/// Note that truncates or extends aren't included because they have IR
-/// counterparts which can have debug users after translation.
-static bool shouldSkipDbgValueFor(MachineInstr &MI) {
-  switch (MI.getOpcode()) {
-  case TargetOpcode::G_UNMERGE_VALUES:
-  case TargetOpcode::G_MERGE_VALUES:
-  case TargetOpcode::G_CONCAT_VECTORS:
-  case TargetOpcode::G_BUILD_VECTOR:
-  case TargetOpcode::G_EXTRACT:
-  case TargetOpcode::G_INSERT:
-    return true;
-  default:
-    return false;
-  }
-}
-
 void llvm::saveUsesAndErase(MachineInstr &MI, MachineRegisterInfo &MRI,
                             LostDebugLocObserver *LocObserver,
                             SmallInstListTy &DeadInstChain) {
   for (MachineOperand &Op : MI.uses()) {
     if (Op.isReg() && Op.getReg().isVirtual())
       DeadInstChain.insert(MRI.getVRegDef(Op.getReg()));
   }
   LLVM_DEBUG(dbgs() << MI << "Is dead; erasing.\n");
   DeadInstChain.remove(&MI);
-  if (shouldSkipDbgValueFor(MI))
-    MI.eraseFromParent();
-  else
-    MI.eraseFromParentAndMarkDBGValuesForRemoval();
   if (LocObserver)
     LocObserver->checkpoint(false);
 }
 
 void llvm::eraseInstrs(ArrayRef<MachineInstr *> DeadInstrs,
                        MachineRegisterInfo &MRI,
                        LostDebugLocObserver *LocObserver) {
   SmallInstListTy DeadInstChain;

llvm/lib/CodeGen/LiveDebugVariables.h

public:
 
   /// dump - Print data structures to dbgs().
   void dump() const;
 
 private:
   bool runOnMachineFunction(MachineFunction &) override;
   void releaseMemory() override;
   void getAnalysisUsage(AnalysisUsage &) const override;
+
+  MachineFunctionProperties getSetProperties() const override {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::DebugValuesAllocated);
+  }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_LIVEDEBUGVARIABLES_H

llvm/lib/CodeGen/MachineCombiner.cpp

static void insertDeleteInstructions(MachineBasicBlock *MBB, MachineInstr &MI,
   // Otherwise the constant pool entry created for InsInstrs will not be deleted
   // even if InsInstrs is not the better pattern.
   TII->finalizeInsInstrs(MI, Pattern, InsInstrs);
 
   for (auto *InstrPtr : InsInstrs)
     MBB->insert((MachineBasicBlock::iterator)&MI, InstrPtr);
 
   for (auto *InstrPtr : DelInstrs) {
-    InstrPtr->eraseFromParentAndMarkDBGValuesForRemoval();
+    InstrPtr->eraseFromParent();
     // Erase all LiveRegs defined by the removed instruction
     for (auto I = RegUnits.begin(); I != RegUnits.end(); ) {
       if (I->MI == InstrPtr)
         I = RegUnits.erase(I);
       else
         I++;
     }
   }

llvm/lib/CodeGen/MachineFunction.cpp

static const char *getPropertyName(MachineFunctionProperties::Property Prop) {
   case P::Legalized: return "Legalized";
   case P::NoPHIs: return "NoPHIs";
   case P::NoVRegs: return "NoVRegs";
   case P::RegBankSelected: return "RegBankSelected";
   case P::Selected: return "Selected";
   case P::TracksLiveness: return "TracksLiveness";
   case P::TiedOpsRewritten: return "TiedOpsRewritten";
   case P::FailsVerification: return "FailsVerification";
+  case P::DebugValuesAllocated: return "DebugValuesAllocated";
   }
   llvm_unreachable("Invalid machine function property");
 }
 
 // Pin the vtable to this file.
 void MachineFunction::Delegate::anchor() {}
 
 void MachineFunctionProperties::print(raw_ostream &OS) const {

llvm/lib/CodeGen/MachineInstr.cpp

MachineInstr *MachineInstr::removeFromBundle() {
   return getParent()->remove_instr(this);
 }
 
 void MachineInstr::eraseFromParent() {
   assert(getParent() && "Not embedded in a basic block!");
   getParent()->erase(this);
 }
 
-void MachineInstr::eraseFromParentAndMarkDBGValuesForRemoval() {
-  assert(getParent() && "Not embedded in a basic block!");
-  MachineBasicBlock *MBB = getParent();
-  MachineFunction *MF = MBB->getParent();
-  assert(MF && "Not embedded in a function!");
-
-  MachineInstr *MI = (MachineInstr *)this;
-  MachineRegisterInfo &MRI = MF->getRegInfo();
-
-  for (const MachineOperand &MO : MI->operands()) {
-    if (!MO.isReg() || !MO.isDef())
-      continue;
-    Register Reg = MO.getReg();
-    if (!Reg.isVirtual())
-      continue;
-    MRI.markUsesInDebugValueAsUndef(Reg);
-  }
-  MI->eraseFromParent();
-}
-
 void MachineInstr::eraseFromBundle() {
   assert(getParent() && "Not embedded in a basic block!");
   getParent()->erase_instr(this);
 }
 
 bool MachineInstr::isCandidateForCallSiteEntry(QueryType Type) const {
   if (!isCall(Type))
     return false;

llvm/lib/CodeGen/MachineVerifier.cpp

struct MachineVerifier {
     const TargetRegisterInfo *TRI;
     const MachineRegisterInfo *MRI;
 
     unsigned foundErrors;
 
     // Avoid querying the MachineFunctionProperties for each operand.
     bool isFunctionRegBankSelected;
     bool isFunctionSelected;
+    bool isFunctionDebugValuesAllocated;
 
     using RegVector = SmallVector<Register, 16>;
     using RegMaskVector = SmallVector<const uint32_t *, 4>;
     using RegSet = DenseSet<Register>;
     using RegMap = DenseMap<Register, const MachineInstr *>;
     using BlockSet = SmallPtrSet<const MachineBasicBlock *, 8>;
 
     const MachineInstr *FirstNonPHI;
unsigned MachineVerifier::verify(const MachineFunction &MF) {
   // reset it and clear the FailedISel attribute in ResetMachineFunctions.
   if (isFunctionFailedISel)
     return foundErrors;
 
   isFunctionRegBankSelected = MF.getProperties().hasProperty(
       MachineFunctionProperties::Property::RegBankSelected);
   isFunctionSelected = MF.getProperties().hasProperty(
       MachineFunctionProperties::Property::Selected);
+  isFunctionDebugValuesAllocated = MF.getProperties().hasProperty(
+      MachineFunctionProperties::Property::DebugValuesAllocated);
 
   LiveVars = nullptr;
   LiveInts = nullptr;
   LiveStks = nullptr;
   Indexes = nullptr;
   if (PASS) {
     LiveInts = PASS->getAnalysisIfAvailable<LiveIntervals>();
     // We don't want to verify LiveVariables if LiveIntervals is available.
if (Register::isPhysicalRegister(Reg)) {
         // getVRegDef can never fail and return null on a generic register.
         //
         // FIXME: This restriction should probably be broadened to all SSA
         // MIR. However, DetectDeadLanes/ProcessImplicitDefs technically still
         // run on the SSA function just before phi elimination.
         if (MO->isUndef())
           report("Generic virtual register use cannot be undef", MO, MONum);
 
+        // Debug value instruction is permitted to use undefined vregs.
+        // This is a performance measure to skip the overhead of immediately
+        // pruning unused debug operands. The final undef substitution occurs
+        // when debug values are allocated in LDVImpl::handleDebugValue, so
+        // these verifications always apply after this pass.
+        if (isFunctionDebugValuesAllocated || !MO->isUse() ||
+            !MI->isDebugValue() || !MRI->def_empty(Reg)) {
           // If we're post-Select, we can't have gvregs anymore.
           if (isFunctionSelected) {
             report("Generic virtual register invalid in a Selected function",
                    MO, MONum);
             return;
           }
 
           // The gvreg must have a type and it must not have a SubIdx.
           LLT Ty = MRI->getType(Reg);
           if (!Ty.isValid()) {
             report("Generic virtual register must have a valid type", MO,
                    MONum);
             return;
           }
 
           const RegisterBank *RegBank = MRI->getRegBankOrNull(Reg);
 
           // If we're post-RegBankSelect, the gvreg must have a bank.
           if (!RegBank && isFunctionRegBankSelected) {
             report("Generic virtual register must have a bank in a "
                    "RegBankSelected function",
                    MO, MONum);
             return;
           }
 
           // Make sure the register fits into its register bank if any.
           if (RegBank && Ty.isValid() &&
               RegBank->getSize() < Ty.getSizeInBits()) {
             report("Register bank is too small for virtual register", MO,
                    MONum);
             errs() << "Register bank " << RegBank->getName() << " too small("
                    << RegBank->getSize() << ") to fit " << Ty.getSizeInBits()
                    << "-bits\n";
             return;
           }
+        }
+
         if (SubIdx)  {
           report("Generic virtual register does not allow subregister index", MO,
                  MONum);
           return;
         }
 
         // If this is a target specific instruction and this operand
         // has register class constraint, the virtual register must

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

if (!TII->isOperandLegal(*UseMI, OpIdx, Op)) {
     return false;
   }
 
   LLVM_DEBUG(dbgs() << "Folded " << *RS << " into " << *UseMI);
 
   // Erase the REG_SEQUENCE eagerly, unless we followed a chain of COPY users,
   // in which case we can erase them all later in runOnMachineFunction.
   if (MRI->use_nodbg_empty(MI.getOperand(0).getReg()))
-    MI.eraseFromParentAndMarkDBGValuesForRemoval();
+    MI.eraseFromParent();
   return true;
 }
 
 // Try to hoist an AGPR to VGPR copy out of the loop across a LCSSA PHI.
 // This should allow folding of an AGPR into a consumer which may support it.
 // I.e.:
 //
 // loop:                             // loop:
for (auto &MI : make_early_inc_range(*MBB)) {
       // delete it now.
       // The only reason we need to follow chains of copies here is that
       // tryFoldRegSequence looks forward through copies before folding a
       // REG_SEQUENCE into its eventual users.
       auto *InstToErase = &MI;
       while (MRI->use_nodbg_empty(InstToErase->getOperand(0).getReg())) {
         auto &SrcOp = InstToErase->getOperand(1);
         auto SrcReg = SrcOp.isReg() ? SrcOp.getReg() : Register();
-        InstToErase->eraseFromParentAndMarkDBGValuesForRemoval();
+        InstToErase->eraseFromParent();
         InstToErase = nullptr;
         if (!SrcReg || SrcReg.isPhysical())
           break;
         InstToErase = MRI->getVRegDef(SrcReg);
         if (!InstToErase || !TII->isFoldableCopy(*InstToErase))
           break;
       }
       if (InstToErase && InstToErase->isRegSequence() &&
           MRI->use_nodbg_empty(InstToErase->getOperand(0).getReg()))
-        InstToErase->eraseFromParentAndMarkDBGValuesForRemoval();
+        InstToErase->eraseFromParent();
     }
   }
   return true;
 }

llvm/lib/Target/NVPTX/NVPTXPeephole.cpp

MachineInstrBuilder MIB =
               Root.getOperand(0).getReg())
           .addReg(NRI->getFrameLocalRegister(MF))
           .add(Prev.getOperand(2));
 
   MBB.insert((MachineBasicBlock::iterator)&Root, MIB);
 
   // Check if MRI has only one non dbg use, which is Root
   if (MRI.hasOneNonDBGUse(Prev.getOperand(0).getReg())) {
-    Prev.eraseFromParentAndMarkDBGValuesForRemoval();
+    Prev.eraseFromParent();
   }
-  Root.eraseFromParentAndMarkDBGValuesForRemoval();
+  Root.eraseFromParent();
 }
 
 bool NVPTXPeephole::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
 
   bool Changed = false;
   // Loop over all of the basic blocks.
bool NVPTXPeephole::runOnMachineFunction(MachineFunction &MF) {
 
   const NVPTXRegisterInfo *NRI =
       MF.getSubtarget<NVPTXSubtarget>().getRegisterInfo();
 
   // Remove unnecessary %VRFrame = cvta.local %VRFrameLocal
   const auto &MRI = MF.getRegInfo();
   if (MRI.use_empty(NRI->getFrameRegister(MF))) {
     if (auto MI = MRI.getUniqueVRegDef(NRI->getFrameRegister(MF))) {
-      MI->eraseFromParentAndMarkDBGValuesForRemoval();
+      MI->eraseFromParent();
     }
   }
 
   return Changed;
 }
 
 MachineFunctionPass *llvm::createNVPTXPeephole() { return new NVPTXPeephole(); }

llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir

 name:            test_dbg_value_dead
 legalized:       true
 regBankSelected: true
 body: |
   bb.0:
     liveins: $w0
     ; CHECK-LABEL: name: test_dbg_value_dead
     ; CHECK-NOT: COPY
-    ; CHECK: DBG_VALUE $noreg, $noreg, !7, !DIExpression(), debug-location !9
+    ; CHECK: DBG_VALUE %0:gpr, $noreg, !7, !DIExpression(), debug-location !9
     %0:gpr(s32) = COPY $w0
     DBG_VALUE %0(s32), $noreg, !7, !DIExpression(), debug-location !9
 ...

llvm/test/CodeGen/AArch64/GlobalISel/uaddo-8-16-bits.mir

body:             |
   ; CHECK-NEXT:   [[ASSERT_ZEXT:%[0-9]+]]:_(s32) = G_ASSERT_ZEXT [[COPY]], 16
   ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:_(s32) = COPY $w1
   ; CHECK-NEXT:   [[ASSERT_ZEXT1:%[0-9]+]]:_(s32) = G_ASSERT_ZEXT [[COPY1]], 16
   ; CHECK-NEXT:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[ASSERT_ZEXT]], [[ASSERT_ZEXT1]]
   ; CHECK-NEXT:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 65536
   ; CHECK-NEXT:   [[AND:%[0-9]+]]:_(s32) = G_AND [[ADD]], [[C]]
   ; CHECK-NEXT:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
   ; CHECK-NEXT:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(eq), [[AND]](s32), [[C1]]
-  ; CHECK-NEXT:   DBG_VALUE $noreg
+  ; CHECK-NEXT:   DBG_VALUE {{%[0-9]+}}:_(s16)
   ; CHECK-NEXT:   G_BRCOND [[ICMP]](s1), %bb.2
   ; CHECK-NEXT:   G_BR %bb.1
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.1:
   ; CHECK-NEXT:   successors:{{ $}}
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.trap)
   ; CHECK-NEXT: {{  $}}

llvm/test/CodeGen/AMDGPU/fold-readlane.mir

 # RUN: llc -march=amdgcn -run-pass si-fold-operands -verify-machineinstrs %s -o - | FileCheck -check-prefix=GCN %s
 
 # GCN-LABEL: name: fold-imm-readfirstlane{{$}}
 # GCN: %1:sreg_32_xm0 = S_MOV_B32 123
 ---
 name: fold-imm-readfirstlane
 tracksRegLiveness: true
 body:             |
   bb.0:
     %0:vgpr_32 = V_MOV_B32_e32 123, implicit $exec
     %1:sreg_32_xm0 = V_READFIRSTLANE_B32 %0, implicit $exec
     S_NOP 0, implicit %1
 ...
 
 # GCN-LABEL: name: fold-imm-readfirstlane-dbgvalue{{$}}
 # GCN: %1:sreg_32_xm0 = S_MOV_B32 123
-# GCN: DBG_VALUE $noreg, 0, 0
+# GCN: DBG_VALUE %0:vgpr_32, 0, 0
 ---
 name: fold-imm-readfirstlane-dbgvalue
 tracksRegLiveness: true
 body:             |
   bb.0:
     %0:vgpr_32 = V_MOV_B32_e32 123, implicit $exec
     %1:sreg_32_xm0 = V_READFIRSTLANE_B32 %0, implicit $exec
     DBG_VALUE %0, 0, 0

llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll

+; RUN: llc -mtriple=i386-linux-gnu -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL
+
+; This file is the output of clang -g -O2
+; int test_dbg_trunc(unsigned long long a) { return a; }
+;
+; The intent of this check is to ensure the DBG_VALUE use of G_MERGE_VALUES is undef'd when the legalizer erases it.
+
+; ModuleID = 'x86-calllowering-dbg-trunc.c'
+source_filename = "x86-calllowering-dbg-trunc.c"
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386"
+
+; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn
+define dso_local i32 @test_dbg_trunc(i64 %a) local_unnamed_addr #0 !dbg !9 {
+; ALL-LABEL: test_dbg_trunc:
+; ALL:       # %bb.0: # %entry
+; ALL:       pushl  %ebp
+; ALL:       movl %esp, %ebp
+; ALL:       movl 8(%ebp), %eax
+; ALL:       #DEBUG_VALUE: test_dbg_trunc:a <- undef
+; ALL:       popl %ebp
+; ALL:       retl
+entry:
+  call void @llvm.dbg.value(metadata i64 %a, metadata !15, metadata !DIExpression()), !dbg !16
+  %conv = trunc i64 %a to i32, !dbg !17
+  ret i32 %conv, !dbg !18
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 14.0.0 (https://github.com/llvm/llvm-project ...)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "x86-calllowering-dbg-trunc.c", directory: "/tmp")
+!2 = !{i32 1, !"NumRegisterParameters", i32 0}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"uwtable", i32 1}
+!7 = !{i32 7, !"frame-pointer", i32 2}
+!8 = !{!"clang version 14.0.0 (https://github.com/llvm/llvm-project ...)"}
+!9 = distinct !DISubprogram(name: "test_dbg_trunc", scope: !1, file: !1, line: 1, type: !10, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !14)
+!10 = !DISubroutineType(types: !11)
+!11 = !{!12, !13}
+!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!13 = !DIBasicType(name: "unsigned long long", size: 64, encoding: DW_ATE_unsigned)
+!14 = !{!15}
+!15 = !DILocalVariable(name: "a", arg: 1, scope: !9, file: !1, line: 1, type: !13)
+!16 = !DILocation(line: 0, scope: !9)
+!17 = !DILocation(line: 1, column: 51, scope: !9)
+!18 = !DILocation(line: 1, column: 44, scope: !9)