[WebAssembly] Fix updating/moving DBG_VALUEs in RegStackify

[yurydelendik]

Summary

As described in PR40209, there can be issues in DBG_VALUEs handling when multiple defs present in a BB. This patch
adds logic for detection of related to def DBG_VALUEs and localizes register update and movement to found DBG_VALUEs.

[aheejin]

Haven't read the whole thing yet, but LLVM naming convention nits first:

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

501 ↗

(On Diff #180527)

Variable names should start with an uppercase letter

520 ↗

(On Diff #180527)

Function names should start with a lowercase letter

539 ↗

(On Diff #180527)

Function names should start with a lowercase letter

543 ↗

(On Diff #180527)

i and e: Variable names should start with an uppercase letter. I know that feels particularly weird with for loop indices :(

[yurydelendik]

fix code style

[aheejin]

I haven't finished reading other parts, but I think I should understand this AttachedDebugValues constructor first to proceed.

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

477 ↗

(On Diff #180594)

Please move this class to anonymous namespace above, because now this is in the global namespace

492 ↗

(On Diff #180594)

By the way the loop can be replaced with MRI->def_instructions(Reg)

494 ↗

(On Diff #180594)

It looks like you assume Op is a use and not a def in this loop, right? Maybe you should use not reg_operands but def_operands.

498 ↗

(On Diff #180594)

So by this point it looks like you assume Op is a use and not a def, right? Maybe you should also add a clause to exclude defs as well, like

if (Op.isDef())
  continue;

498 ↗

(On Diff #180594)

Oh I meant to delete this comment and I didn't. If you use MRI.def_operands(Reg)) at the top of the loop, you wouldn't need this. Sorry

499 ↗

(On Diff #180594)

What is isDef set? Maybe you mean Defs set?

500 ↗

(On Diff #180594)

associated

502 ↗

(On Diff #180594)

Maybe we can use auto here?

506 ↗

(On Diff #180594)

You can use Defs.count(&*I) to check for an element's existence

514 ↗

(On Diff #180594)

Instrs is a set. Why do we need to test if it already exists in the set when we add it?

517 ↗

(On Diff #180594)

Sorry, but I can't understand what this loop is doing. I thought what we were supposed to do to fix the 2nd point in PR40209 was we needed to figure out which DBG_VALUE instruction was associated with which def, in case there were multiple defs to a register within a BB.

What this loop seems to be doing is gather all use operands into Operands and get all defining instructions into Instrs, and I can't figure out how we are gonna figure out the mapping between MI and DBG_VALUE.

490 ↗

(On Diff #180527)

Defs is a set. Why do we need to test if it already exists in the set when we add it?

[aheejin]

And one thing I'm not sure is, surely this kind of logic, moving DBG_VALUEs with instructions when you move instructions around, will be needed in many places in the backend. Is there a more general way to handle things? How do other passes, such as many optimization passes that move instruction in lib/Codegen solve this problem? Have you taken a look at other passes in lib/CodeGen that moves code, such as MachineSink or MachineLICM?

And I also happened to notice there are functions called [[ https://github.com/llvm-mirror/llvm/blob/e25b508818ed061f87d3a97bd3482dca9a726f66/lib/CodeGen/MachineInstr.cpp#L2077-L2092 | collectDebugValues ]] and [[ https://github.com/llvm-mirror/llvm/blob/e25b508818ed061f87d3a97bd3482dca9a726f66/lib/CodeGen/MachineInstr.cpp#L2094-L2102 | changeDebugValuesDefReg ]] in MachineInstr class. Wouldn't using these functions simplify things a lot?

[yurydelendik]

• Use collectDebugValues
• Rename AdjacentDebugValues and DefDIs

[yurydelendik]

In D56401#1350516, @aheejin wrote:

I haven't finished reading other parts, but I think I should understand this AttachedDebugValues constructor first to proceed.

I over-complicated search for MI's DBG_VALUE, but essentially it is collectDebugValues -- using that instead. Thank you for finding and pointing this out. I could not find this initially.

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

517 ↗

(On Diff #180594)

The collectDebugValues defines relationship between MI and DBG_VALUE. For the updateRef we could use changeDebugValuesDefReg, but we may need to perform subsequent move of the updated regs, so I just kept move the same as the second part of the changeDebugValuesDefReg.

[aheejin]

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

43 ↗

(On Diff #180853)

It's still not in the anonymous namespace I think..?
Also, not sure about the name AdjacentDebugValues.. which sounds like there are also non-adjacent DBG_VALUE instructions.

68 ↗

(On Diff #180853)

I don't think we need to pass TII here.. It looks like you can call MachineFunction::CloneMachineInstrBundle to clone an instruction, which WebAssemblyInstrInfo::duplicate is calling anyway?

78 ↗

(On Diff #180853)

Do we need to loop through all operands just to set the 0th operand? Why don't we just do getOperand(0).setReg(NewReg)?

[yurydelendik]

• Move MachineInstrAdjacentDebugValues
• Use CloneMachineInstr

[yurydelendik]

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

43 ↗

(On Diff #180853)

As I understand DBG_VALUE concept, there can be DBG_VALUE without defining MI. Changed name to MachineInstrAdjacentDebugValues

68 ↗

(On Diff #180853)

I'm not sure if CloneMachineInstrBundle will be a right choice here: if it's applicable to DBG_VALUE, and, also, RematerializeCheapDef tried to clone origin MI via TII->reMaterialize, probably breaking any bundle. Use CloneMachineInstr for now.

[yurydelendik]

Remove TII param from clone()

[aheejin]

One thing I'm curious about is, is this kind of logic that gathers/moves DBG_VALUEs only needed in RegStackify? Don't you need that also in other passes? What about your new ExplicitLocal patch? If you need that in several place, how about moving this class to WebAssemblyUtilities or something and use it from many places?

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

43 ↗

(On Diff #180853)

Hmm, sorry, how about DebugValueHandler or something?

[aheejin]

lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

43 ↗

(On Diff #180853)

Or, DebugValueManager?

[aheejin]

test/DebugInfo/WebAssembly/dbg-value-move-reg-stackify.mir

7 ↗

(On Diff #181150)

Can we specify the whole body so that it shows the two previous defs of %1 and their DBG_VALUEs don't change but only the last def w/ its DBG_VALUE does? Also, here the new register is %2, but I don't think it is guaranteed. So how about something like

# CHECK: %1:i32 = I32_WRAP_I64 %0
# CHECK: DBG_VALUE %1
# CHECK: %1:i32 = CALL_I32 @bar
# CHECK: DBG_VALUE %1
# CHECK: %[[NEWREG:.*]]:i32 = CALL_I32 @bar
# CHECK: DBG_VALUE %[[NEWREG]]
# CHECK: CALL_VOID @foo, %[[NEWREG]]

[aheejin]

Oh and I think we need test cases for

1. Rematerializing a def in another BB that has DBG_VALUE attached
2. LOCAL_TEE case within a BB that has multiple defs to the same register (as in the current test case), to see clone() is working

[yurydelendik]

• Move WebAssembly::DebugValueManager into WebAssemblyUtilities.*
• Fix testing of dbg-value-move-reg-stackify.mir

[yurydelendik]

In D56401#1353800, @aheejin wrote:

Oh and I think we need test cases for

1. Rematerializing a def in another BB that has DBG_VALUE attached

Yep, still working on this one

2. LOCAL_TEE case within a BB that has multiple defs to the same register (as in the current test case), to see clone() is working

The test in llvm/test/DebugInfo/WebAssembly/dbg-value-move-2.ll . Would you like to see it in MIR format?

[yurydelendik]

Revert bad test change

[yurydelendik]

• Fix trivially clonable instruction moving/cloning

[aheejin]

In D56401#1354321, @yurydelendik wrote:

In D56401#1353800, @aheejin wrote:

Oh and I think we need test cases for

1. Rematerializing a def in another BB that has DBG_VALUE attached

Yep, still working on this one

2. LOCAL_TEE case within a BB that has multiple defs to the same register (as in the current test case), to see clone() is working

The test in llvm/test/DebugInfo/WebAssembly/dbg-value-move-2.ll . Would you like to see it in MIR format?

Is this a case that you rematerialize an instruction from another BB and move its DBG_VALUE along with it? I don't think so, because the earlier code didn't handle this case and this test was still passing.

And a minor thing: should we have a separate file for each of the test cases? Can we have both of them in dbg-value-move.ll, and merge mir cases in thie CL in one file?

lib/Target/WebAssembly/WebAssemblyUtilities.cpp

389 ↗

(On Diff #181368)

Sorry, looking at the size of class, I guess it deserves an own file of its own, like DebugValueManager.cpp/h, other than putting this into Utilities. Sorry for going back and forth.

[aheejin]

And another question: I haven't been a reviewer for your other debug info related CLs so I don't know, but are there any other places or your pending CLs that can make use of this new class?

[yurydelendik]

In D56401#1354978, @aheejin wrote:

And another question: I haven't been a reviewer for your other debug info related CLs so I don't know, but are there any other places or your pending CLs that can make use of this new class?

I check pass CLs, and looks like they do not need DebugValueManager functionality. There will be https://reviews.llvm.org/D52634 and this will benefit from having the DebugValueManager. Also there is an issue I was going to report later, that might need such functionality.

[yurydelendik]

• Fix WebAssemblyDebugValueManager::move
• Remove DebugValueManager::reMaterialize

[aheejin]

lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp

13 ↗

(On Diff #181396)

Please fix file name and description: it still says WebAssemblyUtilities.cpp

47 ↗

(On Diff #181396)

For move and clone functions, in the current code I think we are reversing the order of DBG_VALUE instructions if there are multiple. I'm not sure if we should preserve the order of those, but unless there's reason not to, I guess it's better to preserve the order? We can do this by simply reversely iterating the DbgValues vector.

lib/Target/WebAssembly/WebAssemblyDebugValueManager.h

19 ↗

(On Diff #181396)

Do we need this here? Can we move this to cpp file?

[aheejin]

lib/Target/WebAssembly/WebAssemblyDebugValueManager.h

19 ↗

(On Diff #181396)

Looks like we don't need this in cpp file either. We just need a declaration
class MachineInstr;

24 ↗

(On Diff #181396)

I don't think we need these two declarations?

[yurydelendik]

• Prune unused #include
• Fix comments
• Reduce default size of vector to 2
• Reverse insert order for move/clone

[aheejin]

lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp

2 ↗

(On Diff #181549)

This now exceeds 80 cols.. you can delete some -s to make it fit.

lib/Target/WebAssembly/WebAssemblyDebugValueManager.h

13 ↗

(On Diff #181549)

Nit: Is any functionality of this class actually WebAssembly specific? I don't think so... Actually we can have something like this in CodeGen/ or wherever, but putting that there might take some more work or take some more convincing of people, so I'm not necessarily suggesting it now, but anyway, this class does not contain anything wasm-specific.

[yurydelendik]

lib/Target/WebAssembly/WebAssemblyDebugValueManager.h

13 ↗

(On Diff #181549)

It will contains WebAssembly specific methods in the future, when we will try to replace virtual registers with WebAssembly specific locations.

[aheejin]

lib/Target/WebAssembly/WebAssemblyDebugValueManager.h

2 ↗

(On Diff #181549)

This exceeds 80 cols too

[yurydelendik]

Top comment fix

[aheejin]

LGTM! Thanks.

llvm/trunk/lib/Target/WebAssembly/CMakeLists.txt

 
 add_llvm_target(WebAssemblyCodeGen
   WebAssemblyAddMissingPrototypes.cpp
   WebAssemblyArgumentMove.cpp
   WebAssemblyAsmPrinter.cpp
   WebAssemblyCallIndirectFixup.cpp
   WebAssemblyCFGStackify.cpp
   WebAssemblyCFGSort.cpp
+  WebAssemblyDebugValueManager.cpp
   WebAssemblyLateEHPrepare.cpp
   WebAssemblyEHRestoreStackPointer.cpp
   WebAssemblyExceptionInfo.cpp
   WebAssemblyExplicitLocals.cpp
   WebAssemblyFastISel.cpp
   WebAssemblyFixIrreducibleControlFlow.cpp
   WebAssemblyFixFunctionBitcasts.cpp
   WebAssemblyFrameLowering.cpp

llvm/trunk/lib/Target/WebAssembly/WebAssemblyDebugValueManager.h

+// WebAssemblyDebugValueManager.h - WebAssembly DebugValue Manager -*- C++ -*-//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the declaration of the WebAssembly-specific
+/// manager for DebugValues associated with the specific MachineInstr.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_WEBASSEMBLY_WEBASSEMBLYDEBUGVALUEMANAGER_H
+#define LLVM_LIB_TARGET_WEBASSEMBLY_WEBASSEMBLYDEBUGVALUEMANAGER_H
+
+#include "llvm/ADT/SmallVector.h"
+
+namespace llvm {
+
+class MachineInstr;
+
+class WebAssemblyDebugValueManager {
+  SmallVector<MachineInstr *, 2> DbgValues;
+
+public:
+  WebAssemblyDebugValueManager(MachineInstr *Instr);
+
+  void move(MachineInstr *Insert);
+  void updateReg(unsigned Reg);
+  void clone(MachineInstr *Insert, unsigned NewReg);
+};
+
+} // end namespace llvm
+
+#endif

llvm/trunk/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp

+//===-- WebAssemblyDebugValueManager.cpp - WebAssembly DebugValue Manager -===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file implements the manager for MachineInstr DebugValues.
+///
+//===----------------------------------------------------------------------===//
+
+#include "WebAssemblyDebugValueManager.h"
+#include "WebAssemblyMachineFunctionInfo.h"
+#include "llvm/CodeGen/MachineInstr.h"
+
+using namespace llvm;
+
+WebAssemblyDebugValueManager::WebAssemblyDebugValueManager(
+    MachineInstr *Instr) {
+  Instr->collectDebugValues(DbgValues);
+}
+
+void WebAssemblyDebugValueManager::move(MachineInstr *Insert) {
+  MachineBasicBlock *MBB = Insert->getParent();
+  for (MachineInstr *DBI : reverse(DbgValues))
+    MBB->splice(Insert, DBI->getParent(), DBI);
+}
+
+void WebAssemblyDebugValueManager::updateReg(unsigned Reg) {
+  for (auto *DBI : DbgValues)
+    DBI->getOperand(0).setReg(Reg);
+}
+
+void WebAssemblyDebugValueManager::clone(MachineInstr *Insert,
+                                         unsigned NewReg) {
+  MachineBasicBlock *MBB = Insert->getParent();
+  MachineFunction *MF = MBB->getParent();
+  for (MachineInstr *DBI : reverse(DbgValues)) {
+    MachineInstr *Clone = MF->CloneMachineInstr(DBI);
+    Clone->getOperand(0).setReg(NewReg);
+    MBB->insert(Insert, Clone);
+  }
+}

llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

 ///
 /// This is primarily a code size optimization, since temporary values on the
 /// value stack don't need to be named.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h" // for WebAssembly::ARGUMENT_*
 #include "WebAssembly.h"
+#include "WebAssemblyDebugValueManager.h"
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblySubtarget.h"
 #include "WebAssemblyUtilities.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/CodeGen/LiveIntervals.h"
 #include "llvm/CodeGen/MachineBlockFrequencyInfo.h"
 #include "llvm/CodeGen/MachineDominators.h"

 // Shrink LI to its uses, cleaning up LI.
 static void ShrinkToUses(LiveInterval &LI, LiveIntervals &LIS) {
   if (LIS.shrinkToUses(&LI)) {
     SmallVector<LiveInterval *, 4> SplitLIs;
     LIS.splitSeparateComponents(LI, SplitLIs);
   }
 }
 
-static void MoveDebugValues(unsigned Reg, MachineInstr *Insert,
-                            MachineBasicBlock &MBB, MachineRegisterInfo &MRI) {
-  for (auto &Op : MRI.reg_operands(Reg)) {
-    MachineInstr *MI = Op.getParent();
-    assert(MI != nullptr);
-    if (MI->isDebugValue() && MI->getParent() == &MBB)
-      MBB.splice(Insert, &MBB, MI);
-  }
-}
-
-static void UpdateDebugValuesReg(unsigned Reg, unsigned NewReg,
-                                 MachineBasicBlock &MBB,
-                                 MachineRegisterInfo &MRI) {
-  for (auto &Op : MRI.reg_operands(Reg)) {
-    MachineInstr *MI = Op.getParent();
-    assert(MI != nullptr);
-    if (MI->isDebugValue() && MI->getParent() == &MBB)
-      Op.setReg(NewReg);
-  }
-}
-
 /// A single-use def in the same block with no intervening memory or register
 /// dependencies; move the def down and nest it with the current instruction.
 static MachineInstr *MoveForSingleUse(unsigned Reg, MachineOperand &Op,
                                       MachineInstr *Def, MachineBasicBlock &MBB,
                                       MachineInstr *Insert, LiveIntervals &LIS,
                                       WebAssemblyFunctionInfo &MFI,
                                       MachineRegisterInfo &MRI) {
   LLVM_DEBUG(dbgs() << "Move for single use: "; Def->dump());
 
+  WebAssemblyDebugValueManager DefDIs(Def);
   MBB.splice(Insert, &MBB, Def);
-  MoveDebugValues(Reg, Insert, MBB, MRI);
+  DefDIs.move(Insert);
   LIS.handleMove(*Def);
 
   if (MRI.hasOneDef(Reg) && MRI.hasOneUse(Reg)) {
     // No one else is using this register for anything so we can just stackify
     // it in place.
     MFI.stackifyVReg(Reg);
   } else {
     // The register may have unrelated uses or defs; create a new register for
     // just our one def and use so that we can stackify it.
     unsigned NewReg = MRI.createVirtualRegister(MRI.getRegClass(Reg));
     Def->getOperand(0).setReg(NewReg);
     Op.setReg(NewReg);
 
     // Tell LiveIntervals about the new register.
     LIS.createAndComputeVirtRegInterval(NewReg);
 
     // Tell LiveIntervals about the changes to the old register.
     LiveInterval &LI = LIS.getInterval(Reg);
     LI.removeSegment(LIS.getInstructionIndex(*Def).getRegSlot(),
                      LIS.getInstructionIndex(*Op.getParent()).getRegSlot(),
                      /*RemoveDeadValNo=*/true);
 
     MFI.stackifyVReg(NewReg);
 
-    UpdateDebugValuesReg(Reg, NewReg, MBB, MRI);
+    DefDIs.updateReg(NewReg);
 
     LLVM_DEBUG(dbgs() << " - Replaced register: "; Def->dump());
   }
 
   ImposeStackOrdering(Def);
   return Def;
 }
 
-static void CloneDebugValues(unsigned Reg, MachineInstr *Insert,
-                             unsigned TargetReg, MachineBasicBlock &MBB,
-                             MachineRegisterInfo &MRI,
-                             const WebAssemblyInstrInfo *TII) {
-  SmallPtrSet<MachineInstr *, 4> Instrs;
-  for (auto &Op : MRI.reg_operands(Reg)) {
-    MachineInstr *MI = Op.getParent();
-    assert(MI != nullptr);
-    if (MI->isDebugValue() && MI->getParent() == &MBB &&
-        Instrs.find(MI) == Instrs.end())
-      Instrs.insert(MI);
-  }
-  for (const auto &MI : Instrs) {
-    MachineInstr &Clone = TII->duplicate(MBB, Insert, *MI);
-    for (unsigned i = 0, e = Clone.getNumOperands(); i != e; ++i) {
-      MachineOperand &MO = Clone.getOperand(i);
-      if (MO.isReg() && MO.getReg() == Reg)
-        MO.setReg(TargetReg);
-    }
-    LLVM_DEBUG(dbgs() << " - - Cloned DBG_VALUE: "; Clone.dump());
-  }
-}
-
 /// A trivially cloneable instruction; clone it and nest the new copy with the
 /// current instruction.
 static MachineInstr *RematerializeCheapDef(
     unsigned Reg, MachineOperand &Op, MachineInstr &Def, MachineBasicBlock &MBB,
     MachineBasicBlock::instr_iterator Insert, LiveIntervals &LIS,
     WebAssemblyFunctionInfo &MFI, MachineRegisterInfo &MRI,
     const WebAssemblyInstrInfo *TII, const WebAssemblyRegisterInfo *TRI) {
   LLVM_DEBUG(dbgs() << "Rematerializing cheap def: "; Def.dump());
   LLVM_DEBUG(dbgs() << " - for use in "; Op.getParent()->dump());
 
+  WebAssemblyDebugValueManager DefDIs(&Def);
+
   unsigned NewReg = MRI.createVirtualRegister(MRI.getRegClass(Reg));
   TII->reMaterialize(MBB, Insert, NewReg, 0, Def, *TRI);
   Op.setReg(NewReg);
   MachineInstr *Clone = &*std::prev(Insert);
   LIS.InsertMachineInstrInMaps(*Clone);
   LIS.createAndComputeVirtRegInterval(NewReg);
   MFI.stackifyVReg(NewReg);
   ImposeStackOrdering(Clone);
static MachineInstr *RematerializeCheapDef(
   if (IsDead) {
     LLVM_DEBUG(dbgs() << " - Deleting original\n");
     SlotIndex Idx = LIS.getInstructionIndex(Def).getRegSlot();
     LIS.removePhysRegDefAt(WebAssembly::ARGUMENTS, Idx);
     LIS.removeInterval(Reg);
     LIS.RemoveMachineInstrFromMaps(Def);
     Def.eraseFromParent();
 
-    MoveDebugValues(Reg, &*Insert, MBB, MRI);
-    UpdateDebugValuesReg(Reg, NewReg, MBB, MRI);
+    DefDIs.move(&*Insert);
+    DefDIs.updateReg(NewReg);
   } else {
-    CloneDebugValues(Reg, &*Insert, NewReg, MBB, MRI, TII);
+    DefDIs.clone(&*Insert, NewReg);
   }
 
   return Clone;
 }
 
 /// A multiple-use def in the same block with no intervening memory or register
 /// dependencies; move the def down, nest it with the current instruction, and
 /// insert a tee to satisfy the rest of the uses. As an illustration, rewrite

 /// with DefReg and TeeReg stackified. This eliminates a local.get from the
 /// resulting code.
 static MachineInstr *MoveAndTeeForMultiUse(
     unsigned Reg, MachineOperand &Op, MachineInstr *Def, MachineBasicBlock &MBB,
     MachineInstr *Insert, LiveIntervals &LIS, WebAssemblyFunctionInfo &MFI,
     MachineRegisterInfo &MRI, const WebAssemblyInstrInfo *TII) {
   LLVM_DEBUG(dbgs() << "Move and tee for multi-use:"; Def->dump());
 
+  WebAssemblyDebugValueManager DefDIs(Def);
+
   // Move Def into place.
   MBB.splice(Insert, &MBB, Def);
   LIS.handleMove(*Def);
 
   // Create the Tee and attach the registers.
   const auto *RegClass = MRI.getRegClass(Reg);
   unsigned TeeReg = MRI.createVirtualRegister(RegClass);
   unsigned DefReg = MRI.createVirtualRegister(RegClass);
   MachineOperand &DefMO = Def->getOperand(0);
   MachineInstr *Tee = BuildMI(MBB, Insert, Insert->getDebugLoc(),
                               TII->get(GetTeeOpcode(RegClass)), TeeReg)
                           .addReg(Reg, RegState::Define)
                           .addReg(DefReg, getUndefRegState(DefMO.isDead()));
   Op.setReg(TeeReg);
   DefMO.setReg(DefReg);
   SlotIndex TeeIdx = LIS.InsertMachineInstrInMaps(*Tee).getRegSlot();
   SlotIndex DefIdx = LIS.getInstructionIndex(*Def).getRegSlot();
 
-  MoveDebugValues(Reg, Insert, MBB, MRI);
+  DefDIs.move(Insert);
 
   // Tell LiveIntervals we moved the original vreg def from Def to Tee.
   LiveInterval &LI = LIS.getInterval(Reg);
   LiveInterval::iterator I = LI.FindSegmentContaining(DefIdx);
   VNInfo *ValNo = LI.getVNInfoAt(DefIdx);
   I->start = TeeIdx;
   ValNo->def = TeeIdx;
   ShrinkToUses(LI, LIS);
 
   // Finish stackifying the new regs.
   LIS.createAndComputeVirtRegInterval(TeeReg);
   LIS.createAndComputeVirtRegInterval(DefReg);
   MFI.stackifyVReg(DefReg);
   MFI.stackifyVReg(TeeReg);
   ImposeStackOrdering(Def);
   ImposeStackOrdering(Tee);
 
-  CloneDebugValues(Reg, Tee, DefReg, MBB, MRI, TII);
-  CloneDebugValues(Reg, Insert, TeeReg, MBB, MRI, TII);
+  DefDIs.clone(Tee, DefReg);
+  DefDIs.clone(Insert, TeeReg);
 
   LLVM_DEBUG(dbgs() << " - Replaced register: "; Def->dump());
   LLVM_DEBUG(dbgs() << " - Tee instruction: "; Tee->dump());
   return Def;
 }
 
 namespace {
 /// A stack for walking the tree of instructions being built, visiting the

llvm/trunk/test/DebugInfo/WebAssembly/dbg-value-move-clone.mir

+# RUN: llc < %s -run-pass=wasm-reg-stackify -x=mir 2>&1 | FileCheck %s
+
+# CHECK: body:
+# CHECK: bb.0:
+# CHECK: %[[REG1:[0-9]+]]:i32 = CONST_I32 0,
+# CHECK-NEXT: DBG_VALUE %[[REG1]],
+# CHECK-NEXT: CALL_VOID @foo, %[[REG1]],
+# CHECK: bb.1:
+# CHECK: %[[REG2:[0-9]+]]:i32 = CONST_I32 0,
+# CHECK-NEXT: DBG_VALUE %[[REG2]],
+# CHECK-NEXT: CALL_VOID @foo, %[[REG2]],
+# CHECK: %[[REG3:[0-9]+]]:i32 = CONST_I32 0,
+# CHECK-NEXT: DBG_VALUE %[[REG3]],
+# CHECK-NEXT: CALL_VOID @foo, %[[REG3]],
+
+--- |
+  target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+  target triple = "wasm32-unknown-unknown"
+
+  declare void @foo(i32)
+  declare i32 @bar()
+
+  define void @test(i64 %arg) {
+    unreachable
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!4}
+  !0 = distinct !DICompileUnit(language: DW_LANG_Rust, file: !2, producer: "clang LLVM (rustc version 1.30.0-dev)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !1, globals: !1)
+  !1 = !{}
+  !2 = !DIFile(filename: "<unknown>", directory: "")
+  !3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "&str", file: !2, size: 64, align: 32, elements: !{}, identifier: "111094d970b097647de579f9c509ef08")
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = distinct !DILexicalBlock(scope: !6, file: !2, line: 357, column: 8)
+  !6 = distinct !DISubprogram(name: "testfoo", linkageName: "_testba", scope: !7, file: !2, line: 353, type: !8, isLocal: true, isDefinition: true, scopeLine: 353, flags: DIFlagPrototyped, isOptimized: true, unit: !0, templateParams: !1, retainedNodes: !9)
+  !7 = !DINamespace(name: "ptr", scope: null)
+  !8 = !DISubroutineType(types: !1)
+  !9 = !{!10}
+  !10 = !DILocalVariable(name: "val0", scope: !5, file: !2, line: 357, type: !3, align: 4)
+  !11 = !DILocalVariable(name: "val1", scope: !5, file: !2, line: 358, type: !3, align: 4)
+  !12 = !DILocalVariable(name: "val2", scope: !5, file: !2, line: 359, type: !3, align: 4)
+  !13 = !DILocation(line: 357, column: 12, scope: !5)
+  !14 = !DILocation(line: 358, column: 12, scope: !5)
+  !15 = !DILocation(line: 359, column: 12, scope: !5)
+
+---
+name: test
+liveins:
+  - { reg: '$arguments' }
+tracksRegLiveness: true
+body: |
+  bb.0:
+    successors: %bb.1
+    liveins: $arguments
+    %0:i64 = ARGUMENT_i64 0, implicit $arguments
+    %1:i32 = CONST_I32 0, implicit-def dead $arguments
+    DBG_VALUE %1:i32, $noreg, !10, !DIExpression(), debug-location !13; <unknown>:357:12 line no:357
+    CALL_VOID @foo, %1:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+
+  bb.1:
+    CALL_VOID @foo, %1:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+    CALL_VOID @foo, %1:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+    RETURN_VOID implicit-def dead $arguments
+
+...

llvm/trunk/test/DebugInfo/WebAssembly/dbg-value-move-reg-stackify.mir

+# RUN: llc < %s -run-pass=wasm-reg-stackify -x=mir 2>&1 | FileCheck %s
+
+# CHECK: body:
+# CHECK: %1:i32 = I32_WRAP_I64 %0,
+# CHECK-NEXT: DBG_VALUE %1,
+# CHECK-NEXT: %1:i32 = CALL_I32 @bar,
+# CHECK-NEXT: DBG_VALUE %1,
+# CHECK-NEXT: %[[NEWREG:.*]]:i32 = CALL_I32 @bar,
+# CHECK-NEXT: DBG_VALUE %[[NEWREG]],
+# CHECK-NEXT: CALL_VOID @foo, %[[NEWREG]],
+
+--- |
+  target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+  target triple = "wasm32-unknown-unknown"
+
+  declare void @foo(i32)
+  declare i32 @bar()
+
+  define void @test(i64 %arg) {
+    unreachable
+  }
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!4}
+  !0 = distinct !DICompileUnit(language: DW_LANG_Rust, file: !2, producer: "clang LLVM (rustc version 1.30.0-dev)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !1, globals: !1)
+  !1 = !{}
+  !2 = !DIFile(filename: "<unknown>", directory: "")
+  !3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "&str", file: !2, size: 64, align: 32, elements: !{}, identifier: "111094d970b097647de579f9c509ef08")
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = distinct !DILexicalBlock(scope: !6, file: !2, line: 357, column: 8)
+  !6 = distinct !DISubprogram(name: "testfoo", linkageName: "_testba", scope: !7, file: !2, line: 353, type: !8, isLocal: true, isDefinition: true, scopeLine: 353, flags: DIFlagPrototyped, isOptimized: true, unit: !0, templateParams: !1, retainedNodes: !9)
+  !7 = !DINamespace(name: "ptr", scope: null)
+  !8 = !DISubroutineType(types: !1)
+  !9 = !{!10}
+  !10 = !DILocalVariable(name: "val0", scope: !5, file: !2, line: 357, type: !3, align: 4)
+  !11 = !DILocalVariable(name: "val1", scope: !5, file: !2, line: 358, type: !3, align: 4)
+  !12 = !DILocalVariable(name: "val2", scope: !5, file: !2, line: 359, type: !3, align: 4)
+  !13 = !DILocation(line: 357, column: 12, scope: !5)
+  !14 = !DILocation(line: 358, column: 12, scope: !5)
+  !15 = !DILocation(line: 359, column: 12, scope: !5)
+
+---
+name: test
+liveins:
+  - { reg: '$arguments' }
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $arguments
+    %0:i64 = ARGUMENT_i64 0, implicit $arguments
+    %1:i32 = I32_WRAP_I64 %0:i64, implicit-def dead $arguments
+    DBG_VALUE %1:i32, $noreg, !10, !DIExpression(), debug-location !13; <unknown>:357:12 line no:357
+    %1:i32 = CALL_I32 @bar, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+    DBG_VALUE %1:i32, $noreg, !11, !DIExpression(), debug-location !14; <unknown>:357:12 line no:357
+    %1:i32 = CALL_I32 @bar, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+    DBG_VALUE %1:i32, $noreg, !12, !DIExpression(), debug-location !15; <unknown>:357:12 line no:357
+    CALL_VOID @foo, %1:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+    RETURN_VOID implicit-def dead $arguments
+
+...