[WebAssembly] Added Debug Fixup pass

[aardappel]

Summary

This pass changes debug_value instructions referring to stackified registers into TI_OPERAND_STACK with correct stack depth.

[dschuff]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

75

style nit:
LLVM style would be something like
for (auto MII = MBB.begin, MIE = MBB.end(); MII != MIE; ++MII)

98

So I guess this is building DBG_VALUE which sets the value to register 0, which clears it? Why advance the iterator by 2? Wouldn't we need to keep advancing until the use of this def? Or until it comes off the stack?

[aheejin]

Nice that this approach works!

• Please clang-format
• Can we have some more test cases other than one-def/one-use case? Like, when we have multiple stackified values at a time and uses them in one or multiple instruction, or when we have multivalue calls. I guess some tests in reg-stackify.ll or multivalue.ll can be adapted here.

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

80

Nit: MO.getReg() can be MO.getReg().isValid() to be more readable

98

In a simple scenario that a register is used right after it is stackified (as in the test case in this CL), it can be iterator+2, but

• Does this handle cases where multiple values are stackified and later used, i.e., does this handle when stack depth > 1?
• Even in one-def / one-use scenario, is it guaranteed that the popping instruction is right after DBG_VALUE? There can be other instructions that do not consume stack values in between. Also there can be other debug instructions and label instructions.

I think you need to find the 'use' instruction of the given register here, using some functions like these in MachineRegisterInfo, I guess.

And there are [[ https://github.com/llvm/llvm-project/blob/9b509bca858cbc37ab8e36383f2550d5e2f8a312/llvm/include/llvm/CodeGen/MachineInstrBuilder.h#L428-L459 | overloaded BuildMI functions ]] that are designed to be used to create DBG_VALUE.

119

I'm not sure we should exclude the condition MO.isDead() here.

There can be dead registers (and drops) in normal scenarios in which we don't run -wasm-disable-explicit-locals. Shouldn't we preserve DBG_VALUEs between the value-pushing instruction and the subsequent drop? For example, when @somefunction pushes one value onto the stack,

call @somefunction
                               <-Here we need to preserve debug values, no?
drop

In general we can't even guarantee that the drop will follow right after the value-pushing instruction. So it is possible:

call @somefunction
call @somefunction
drop
drop

The problem of -wasm-disable-explicit-locals is it does not insert necessary drops, and thus making the stack invalid, as you saw. How about not running this pass entirely when -wasm-disable-explicit-locals is given? I mean, in WebAssemblyTargetMachine.cpp, we can do something like

// Some comments about why -wasm-disable-explicit-locals matters
if (!WasmDisableExplicitLocals)
  // Fix debug_values whose defs have been stackified.                           
  addPass(createWebAssemblyDebugFixup());

To do this, we need to move WasmDisableExplicitLocals from WebAssemblyExplicitLocals.cpp to WebAssemblyTargetMachine.cpp, but it's no big deal.

llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

988 ↗

(On Diff #262142)

I don't think we need to exclude MO.isDead() here, regardless of what we decide to do with ExplicitLocal pass above, because this code runs before ExplicitLocal pass, so this will not fail anyway.

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

24

Is this the encoded stack depth? Then how about mentioning that in the comment, like

; CHECK:   .int8  2                       # TI_OPERAND_STACK (2)
; CHECK:   .int8  0                       # Stack offset

29

Nit: This can be deleted

31

Nit: No need of -wasm

33

Nit: No need of hidden

51

Do we need these attributes for debug value testing? If not we can remove all these I think (or leave only the necessary ones)

[aardappel]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

75

We are inserting new instructions, so depending on the container its better to be calling end() here every time?

98

You are both right, this should be at the use not at the def.
Will add comments to make it clearer too.
Will use DBG_VALUE variant.

119

That's what I had originally, disable the pass when Explicit Locals is off. But I thought this was more elegant since I didn't want to disable a whole bunch of tests unnecessarily. I can revert it, though.

Note that this only happens in the case of an unused variable, but yeah, I guess worth preserving.

[aardappel]

DBG_VALUE $noreg now on pop of stackified reg.

[aardappel]

Improved test

[dschuff]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

37

Since this pass only handles stackified values (and other debug info is handled elsewhere) should we name it something more specific like WebAssemblyDebugStackify instead?

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

I might be mistaken, but I'm not sure it is guaranteed that dbg.value instruction follows right after the corresponding def instruction. For example, this seems possible, in which dbg.value does not follow right after its def:

r0 = ...
r1 = ...
...
dbg.value r0, ... ;; at this point stack size is 2 (depth is 1), but r0's depth should be 0
dbg.value r1, ...

In that case it seems we can't use the stack depth as a target index.? (We may need to search the whole stack to find the matching register.)

It would be good to add a test case for this kind of case as well.

Also typo in the comment: need it construct -> need it to construct

114

Nit: If we do

BuildMI(*Prev.DebugValue->getParent(), std::next(MII), ...

We don't need a separate TMII variable.

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

56

Nit: clang-format

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

59

Nit: We may not need bitcasts and tail calls. How about making this simpler by something like

define void @foo() {                                                             
entry:                                                                           
  %call = call i32 @input()
  call void @llvm.dbg.value(metadata i32 %call, metadata !16, metadata !DIExpression()), !dbg !19                                                  
  %call1 = call i32 @input()
  call void @llvm.dbg.value(metadata i32 %call1, metadata !17, metadata !DIExpression()), !dbg !19                                            
  call void @output(i32 %call, i32 %call1)                                       
  ret void                                                                       
}                                                                                
                                                                                 
declare i32 @input()                                                             
declare void @output(i32, i32)

? (This does not include llvm.dbg.value; they have to be added)
Also it looks we don't need attributes and local_unnamed_addrs for this test.

Also it'd be better to add a test case when llvm.dbg.value is not right after its corresponding def instruction.

[aheejin]

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

59

"(This does not include llvm.dbg.value; they have to be added)" was added by mistake, nvm

[aardappel]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

I don't believe this is the case. For example, https://llvm.org/docs/SourceLevelDebugging.html says "The dbg.value’s position in the IR defines where in the instruction stream the variable’s value changes.". Reading our existing code we seem to assume in quite a few places that this is the case (e.g. WebAssemblyDebugValueManager).

We could maybe add some kind of check/assert that this is indeed the case? But then again we'd need that in a few other places too.

[aardappel]

addressed aheejin's last comments.

[dschuff]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

Wrt the source-level debugging doc, it has an example of dbg.value in LLVM IR converting to DBG_VALUE in MIR:
https://llvm.org/docs/SourceLevelDebugging.html#variable-locations-in-instruction-selection-and-mir
it has and example where the DBG_VALUE for %2 is not even in the same BB as the def. It describes a case where the value is a compound DIE expression computed with a GEP and folded into a load instruction.

In the debugging meeting today, @yurydelendik also said he thought maybe there were cases where the dbg.value wasn't directly following the def, but didn't have any specific examples.

More generally we should probably not ship an emscripten with an assert if we aren't sure the invariant actually holds. But if this CL can cover a useful subset of cases it might make sense to get it in now, unless we think the algorithm is fundamentally broken or something.
It would be nice if we had a way other than an assert to discover these kinds of cases.
Is there a way we can make the output obviously wrong in those cases so we can figure that out? Or maybe a flag or something we could enable? That way we could turn it on, and compile a bunch of our tests in debug mode to find things, without having it on for everyone.

[aardappel]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

Note that the code in this pass only deals with DBG_VALUE that refers to a valid register that has been stackified, which would exclude the example above (no stackification across blocks, for starters).

If it is indeed possible to generate out-of-order DBG_VALUE instructions for stackified values, then likely one of the asserts we already have would hit:

assert(Depth > 0 &&
       "WebAssemblyDebugFixup: DBG_VALUE: No value on the stack!");
assert(MO.getReg() == Stack.back().Reg &&
       "WebAssemblyDebugFixup: DBG_VALUE: Register not matched!");

Those asserts haven't hit with any of the tests so far.

Do we want to replace these asserts with an if that.. just leaves the register? Or sets an obviously invalid stack offset? (unsigned)-1 ? Or introduce a TI _INVALID debug location, that clearly signals we had debug information we weren't able to track..

[aardappel]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

Actually, a more resilient algo would be: rather than assert that the Reg is on the top of the stack, I could simply search the stack for that Reg, and derive the Depth from it. This is nice because the stack should still be correct if a DBG_VALUE can still work if its moved anywhere between the push(def) and pop(use).

If its not found in the stack that means its moved outside of its def-use range alltogether, in which case it is probably fine to leave it untouched, and let it be removed as an orphaned Reg.

Of course, if a DBG_VALUE is moved further ahead from its def, then a debugger that is stopped right after the def can potentially not view it, but that is a different matter. We could also fix this with an algorithm that places DBG_VALUEs back where they belong, but I am not sure this complexity is warranted until we find common cases of this.

[aardappel]

Allowed DBG_VALUE for stackified regs to appear anywhere between its def-use.

[aardappel]

forgot 1 .back() reference + clang-format

[dschuff]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

Ah, that's a good point that this only applies to stackified values.

Searching the stack seems like a good idea though, and simple enough to implement.
Also yeah you're probably right that moving the DBG_VALUEs back isn't worth it.

[dschuff]

great, still LGTM

[aardappel]

Landed: https://github.com/llvm/llvm-project/commit/2b7fe0863ac3c076797404f90b49cee696af0564
(forgot to add phabricator url to commit)

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

98

Actually, a more resilient algo would be: rather than assert that the Reg is on the top of the stack, I could simply search the stack for that Reg, and derive the Depth from it.

This is what I suggested in the last comment. But I think it'd be better to do the reverse search, because the value is likely on top of the stack (or near the top of the stack).

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

59

As I said in the last comment, it'd be good to have a test case when llvm.dbg.value is not right after the corresponding def instruction..? We now can handle this but don't have a test case for it.

[sbc100]

Looks like this breaks the emscripen-releases build:

https://chromium-review.googlesource.com/c/2203178 -> https://ci.chromium.org/p/emscripten-releases/builders/try/linux/b8880288675006681888?

[paolosev]

I get assertion failures in debug builds:

assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
       "Expected inlined-at fields to agree");

fails, when I compile a test program with -O3, with the following call stack, where MachineFunctionPass is a WebAssemblyDebugFixup:

>	clang++.exe!llvm::BuildMI(llvm::MachineFunction & MF, const llvm::DebugLoc & DL, const llvm::MCInstrDesc & MCID, bool IsIndirect, llvm::Register Reg, const llvm::MDNode * Variable, const llvm::MDNode * Expr) Line 2040	C++
 	clang++.exe!llvm::BuildMI(llvm::MachineBasicBlock & BB, llvm::MachineInstrBundleIterator<llvm::MachineInstr,0> I, const llvm::DebugLoc & DL, const llvm::MCInstrDesc & MCID, bool IsIndirect, llvm::Register Reg, const llvm::MDNode * Variable, const llvm::MDNode * Expr) Line 2073	C++
 	clang++.exe!`anonymous namespace'::WebAssemblyDebugFixup::runOnMachineFunction(llvm::MachineFunction & MF) Line 125	C++
 	clang++.exe!llvm::MachineFunctionPass::runOnFunction(llvm::Function & F) Line 73	C++
 	clang++.exe!llvm::FPPassManager::runOnFunction(llvm::Function & F) Line 1482	C++

The assertion fails because in llvm/include/llvm/IR/DebugInfoMetadata.h

 bool isValidLocationForIntrinsic(const DILocation *DL) const {
  return DL && getScope()->getSubprogram() == DL->getScope()->getSubprogram();
}

the DISubprogram* don't match:

0x0000020cdabea990 {Line=0x00000075 ScopeLine=0x00000076 VirtualIndex=0x00000000 ...}	const llvm::DISubprogram *
0x0000020cdabefe20 {Line=0x0000004b ScopeLine=0x0000004b VirtualIndex=0x00000000 ...}	const llvm::DISubprogram *

[aardappel]

@sbc100 @paolosev that crash was just fixed here: https://reviews.llvm.org/D80019

@aheejin I didn't think such a test was necessary, since a) the current code is not explicitly dependent on this adjacency anymore, b) we have code elsewhere (e.g. WebAssemblyDebugValueManager) that does assume this adjacency, so if there's any def + dbg_value that is not adjacent, this likely will require RegStackify to be fixed, and more likely this pass will continue to work. And c) we don't have an example yet where this is even produced.

[aheejin]

@aheejin I didn't think such a test was necessary, since a) the current code is not explicitly dependent on this adjacency anymore, b) we have code elsewhere (e.g. WebAssemblyDebugValueManager) that does assume this adjacency, so if there's any def + dbg_value that is not adjacent, this likely will require RegStackify to be fixed, and more likely this pass will continue to work. And c) we don't have an example yet where this is even produced.

WebAssemblyDebugValueManager does not assume this adjacency. It calls [[ https://github.com/llvm/llvm-project/blob/96d85726b0fc3c5154265cea156483a3f63ebe21/llvm/lib/CodeGen/MachineInstr.cpp#L2123-L2138 | MachineInstr::collectDebugValues ]], which traverses the whole BB to collect debug values instructions for a matching register. But when I change the order of llvm.dbg.values in your example and run it with -print-machineinstrs, somehow instruction selection manages to emit DBG_VALUEs after their defs. Not sure it is always guaranteed though, as in the link @dschuff pasted. Anyway, to test this case we need an mir test case, which I think is an overkill too.

But I still think reverse traversing of Stack is worth it, given that the register will be most likely to be on top of the stack? (This is also a thing I suggested, but you didn't answer that one either)

And on the other note, in general, even when you think some of my suggestions are unnecessary, I'd greatly appreciate if you answer them by explaining why they are unnecessary, rather than just ignoring them.

[aardappel]

@aheejin Apologies if you think I ignored your comments, I feel I responded to all of them, though maybe I could have responded earlier (I wanted to first bring up your point about adjacency in the debugging subgroup meeting yesterday).

WebAssemblyDebugValueManager: ah, didn't see that.. I guess it makes them adjacent when it clones/moves them.

As for the loop in reverse.. I did think of that, but given the average expected stack depth, I didn't think it was necessary.. would be happy to add it though.

llvm/lib/Target/WebAssembly/CMakeLists.txt

 add_public_tablegen_target(WebAssemblyCommonTableGen)
 
 add_llvm_target(WebAssemblyCodeGen
   WebAssemblyAddMissingPrototypes.cpp
   WebAssemblyArgumentMove.cpp
   WebAssemblyAsmPrinter.cpp
   WebAssemblyCFGStackify.cpp
   WebAssemblyCFGSort.cpp
+  WebAssemblyDebugFixup.cpp
   WebAssemblyDebugValueManager.cpp
   WebAssemblyLateEHPrepare.cpp
   WebAssemblyExceptionInfo.cpp
   WebAssemblyExplicitLocals.cpp
   WebAssemblyFastISel.cpp
   WebAssemblyFixIrreducibleControlFlow.cpp
   WebAssemblyFixFunctionBitcasts.cpp
   WebAssemblyFrameLowering.cpp

llvm/lib/Target/WebAssembly/WebAssembly.h

 FunctionPass *createWebAssemblyRegColoring();
 FunctionPass *createWebAssemblyFixIrreducibleControlFlow();
 FunctionPass *createWebAssemblyLateEHPrepare();
 FunctionPass *createWebAssemblyCFGSort();
 FunctionPass *createWebAssemblyCFGStackify();
 FunctionPass *createWebAssemblyExplicitLocals();
 FunctionPass *createWebAssemblyLowerBrUnless();
 FunctionPass *createWebAssemblyRegNumbering();
+FunctionPass *createWebAssemblyDebugFixup();
 FunctionPass *createWebAssemblyPeephole();
 
 // PassRegistry initialization declarations.
 void initializeWebAssemblyAddMissingPrototypesPass(PassRegistry &);
 void initializeWebAssemblyLowerEmscriptenEHSjLjPass(PassRegistry &);
 void initializeLowerGlobalDtorsPass(PassRegistry &);
 void initializeFixFunctionBitcastsPass(PassRegistry &);
 void initializeOptimizeReturnedPass(PassRegistry &);
 void initializeWebAssemblyArgumentMovePass(PassRegistry &);
 void initializeWebAssemblySetP2AlignOperandsPass(PassRegistry &);
 void initializeWebAssemblyReplacePhysRegsPass(PassRegistry &);
 void initializeWebAssemblyPrepareForLiveIntervalsPass(PassRegistry &);
 void initializeWebAssemblyOptimizeLiveIntervalsPass(PassRegistry &);
 void initializeWebAssemblyMemIntrinsicResultsPass(PassRegistry &);
 void initializeWebAssemblyRegStackifyPass(PassRegistry &);
 void initializeWebAssemblyRegColoringPass(PassRegistry &);
 void initializeWebAssemblyFixIrreducibleControlFlowPass(PassRegistry &);
 void initializeWebAssemblyLateEHPreparePass(PassRegistry &);
 void initializeWebAssemblyExceptionInfoPass(PassRegistry &);
 void initializeWebAssemblyCFGSortPass(PassRegistry &);
 void initializeWebAssemblyCFGStackifyPass(PassRegistry &);
 void initializeWebAssemblyExplicitLocalsPass(PassRegistry &);
 void initializeWebAssemblyLowerBrUnlessPass(PassRegistry &);
 void initializeWebAssemblyRegNumberingPass(PassRegistry &);
+void initializeWebAssemblyDebugFixupPass(PassRegistry &);
 void initializeWebAssemblyPeepholePass(PassRegistry &);
 
 namespace WebAssembly {
 enum TargetIndex {
   // Followed by a local index (ULEB).
   TI_LOCAL,
   // Followed by an absolute global index (ULEB). DEPRECATED.
   TI_GLOBAL_FIXED,

llvm/lib/Target/WebAssembly/WebAssemblyDebugFixup.cpp

+//===-- WebAssemblyDebugFixup.cpp - Debug Fixup ------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Several prior passes may "stackify" registers, here we ensure any references
+/// in such registers in debug_value instructions become stack relative also.
+/// This is done in a separate pass such that not all previous passes need to
+/// track stack depth when values get stackified.
+///
+//===----------------------------------------------------------------------===//
+
+#include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
+#include "WebAssembly.h"
+#include "WebAssemblyMachineFunctionInfo.h"
+#include "WebAssemblySubtarget.h"
+#include "WebAssemblyUtilities.h"
+#include "llvm/ADT/SCCIterator.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineLoopInfo.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+using namespace llvm;
+
+#define DEBUG_TYPE "wasm-debug-fixup"
+
+namespace {
+class WebAssemblyDebugFixup final : public MachineFunctionPass {
+  StringRef getPassName() const override { return "WebAssembly Debug Fixup"; }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesCFG();
+    MachineFunctionPass::getAnalysisUsage(AU);
+  }
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+
+public:
+  static char ID; // Pass identification, replacement for typeid
+  WebAssemblyDebugFixup() : MachineFunctionPass(ID) {}
+};
+} // end anonymous namespace
+
+char WebAssemblyDebugFixup::ID = 0;
+INITIALIZE_PASS(
+    WebAssemblyDebugFixup, DEBUG_TYPE,
+    "Ensures debug_value's that have been stackified become stack relative",
+    false, false)
+
+FunctionPass *llvm::createWebAssemblyDebugFixup() {
+  return new WebAssemblyDebugFixup();
+}
+
+bool WebAssemblyDebugFixup::runOnMachineFunction(MachineFunction &MF) {
+  LLVM_DEBUG(dbgs() << "********** Debug Fixup **********\n"
+                       "********** Function: "
+                    << MF.getName() << '\n');
+
+  WebAssemblyFunctionInfo &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
+  const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
+
+  struct StackElem {
+    unsigned Reg;
+    MachineInstr *DebugValue;
+  };
+  std::vector<StackElem> Stack;
+  for (MachineBasicBlock &MBB : MF) {
+    // We may insert into this list.
+    for (auto MII = MBB.begin(); MII != MBB.end(); ++MII) {
+      MachineInstr &MI = *MII;
+      if (MI.isDebugValue()) {
+        auto &MO = MI.getOperand(0);
+        // Also check if not a $noreg: likely a DBG_VALUE we just inserted.
+        if (MO.isReg() && MO.getReg().isValid() &&
+            MFI.isVRegStackified(MO.getReg())) {
+          // Found a DBG_VALUE with a stackified register we will
+          // change into a stack operand.
+          // Search for register rather than assume it is on top (which it
+          // typically is if it appears right after the def), since
+          // DBG_VALUE's may shift under some circumstances.
+          size_t Depth = 0;
+          for (auto &Elem : Stack) {
+            if (MO.getReg() == Elem.Reg) {
+              LLVM_DEBUG(dbgs() << "Debug Value VReg " << MO.getReg()
+                                << " -> Stack Relative " << Depth << "\n");
+              MO.ChangeToTargetIndex(WebAssembly::TI_OPERAND_STACK, Depth);
+              // Save the DBG_VALUE instruction that defined this stackified
+              // variable since later we need it to construct another one on
+              // pop.
+              Elem.DebugValue = &MI;
+              break;
+            }
+            Depth++;
+          }
+          // If the Reg was not found, we have a DBG_VALUE outside of its
+          // def-use range, and we leave it unmodified as reg, which means
+          // it will be culled later.
+        }
+      } else {
+        // Track stack depth.
+        for (MachineOperand &MO : reverse(MI.explicit_uses())) {
+          if (MO.isReg() && MFI.isVRegStackified(MO.getReg())) {
+            auto Prev = Stack.back();
+            Stack.pop_back();
+            assert(Prev.Reg == MO.getReg() &&
+                   "WebAssemblyDebugFixup: Pop: Register not matched!");
+            if (Prev.DebugValue) {
+              // This stackified reg is a variable that started life at
+              // Prev.DebugValue, so now that we're popping it we must insert
+              // a $noreg DBG_VALUE for the variable to end it, right after
+              // the current instruction.
+              BuildMI(*Prev.DebugValue->getParent(), std::next(MII),
+                      MI.getDebugLoc(), TII->get(WebAssembly::DBG_VALUE), false,
+                      Register(), Prev.DebugValue->getOperand(2).getMetadata(),
+                      Prev.DebugValue->getOperand(3).getMetadata());
+            }
+          }
+        }
+        for (MachineOperand &MO : MI.defs()) {
+          if (MO.isReg() && MFI.isVRegStackified(MO.getReg())) {
+            Stack.push_back({MO.getReg(), nullptr});
+          }
+        }
+      }
+    }
+    assert(Stack.empty() &&
+           "WebAssemblyDebugFixup: Stack not empty at end of basic block!");
+  }
+
+  return true;
+}

llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp

 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "wasm-explicit-locals"
 
-// A command-line option to disable this pass, and keep implicit locals
-// for the purpose of testing with lit/llc ONLY.
-// This produces output which is not valid WebAssembly, and is not supported
-// by assemblers/disassemblers and other MC based tools.
-static cl::opt<bool> WasmDisableExplicitLocals(
-    "wasm-disable-explicit-locals", cl::Hidden,
-    cl::desc("WebAssembly: output implicit locals in"
-             " instruction output for test purposes only."),
-    cl::init(false));
-
 namespace {
 class WebAssemblyExplicitLocals final : public MachineFunctionPass {
   StringRef getPassName() const override {
     return "WebAssembly Explicit Locals";
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
static MachineInstr *findStartOfTree(MachineOperand &MO,
   return Def;
 }
 
 bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) {
   LLVM_DEBUG(dbgs() << "********** Make Locals Explicit **********\n"
                        "********** Function: "
                     << MF.getName() << '\n');
 
-  // Disable this pass if directed to do so.
-  if (WasmDisableExplicitLocals)
-    return false;
-
   bool Changed = false;
   MachineRegisterInfo &MRI = MF.getRegInfo();
   WebAssemblyFunctionInfo &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
 
   // Map non-stackified virtual registers to their local ids.
   DenseMap<unsigned, unsigned> Reg2Local;
 

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

static cl::opt<bool> EnableEmException(
     cl::init(false));
 
 // Emscripten's asm.js-style setjmp/longjmp handling
 static cl::opt<bool> EnableEmSjLj(
     "enable-emscripten-sjlj",
     cl::desc("WebAssembly Emscripten-style setjmp/longjmp handling"),
     cl::init(false));
 
+// A command-line option to keep implicit locals
+// for the purpose of testing with lit/llc ONLY.
+// This produces output which is not valid WebAssembly, and is not supported
+// by assemblers/disassemblers and other MC based tools.
+static cl::opt<bool> WasmDisableExplicitLocals(
+    "wasm-disable-explicit-locals", cl::Hidden,
+    cl::desc("WebAssembly: output implicit locals in"
+             " instruction output for test purposes only."),
+    cl::init(false));
+
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
   // Register the target.
   RegisterTargetMachine<WebAssemblyTargetMachine> X(
       getTheWebAssemblyTarget32());
   RegisterTargetMachine<WebAssemblyTargetMachine> Y(
       getTheWebAssemblyTarget64());
 
   // Register backend passes
extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
   initializeWebAssemblyFixIrreducibleControlFlowPass(PR);
   initializeWebAssemblyLateEHPreparePass(PR);
   initializeWebAssemblyExceptionInfoPass(PR);
   initializeWebAssemblyCFGSortPass(PR);
   initializeWebAssemblyCFGStackifyPass(PR);
   initializeWebAssemblyExplicitLocalsPass(PR);
   initializeWebAssemblyLowerBrUnlessPass(PR);
   initializeWebAssemblyRegNumberingPass(PR);
+  initializeWebAssemblyDebugFixupPass(PR);
   initializeWebAssemblyPeepholePass(PR);
 }
 
 //===----------------------------------------------------------------------===//
 // WebAssembly Lowering public interface.
 //===----------------------------------------------------------------------===//
 
 static Reloc::Model getEffectiveRelocModel(Optional<Reloc::Model> RM,
void WebAssemblyPassConfig::addPreEmitPass() {
   // Sort the blocks of the CFG into topological order, a prerequisite for
   // BLOCK and LOOP markers.
   addPass(createWebAssemblyCFGSort());
 
   // Insert BLOCK and LOOP markers.
   addPass(createWebAssemblyCFGStackify());
 
   // Insert explicit local.get and local.set operators.
+  if (!WasmDisableExplicitLocals)
     addPass(createWebAssemblyExplicitLocals());
 
   // Lower br_unless into br_if.
   addPass(createWebAssemblyLowerBrUnless());
 
   // Perform the very last peephole optimizations on the code.
   if (getOptLevel() != CodeGenOpt::None)
     addPass(createWebAssemblyPeephole());
 
   // Create a mapping from LLVM CodeGen virtual registers to wasm registers.
   addPass(createWebAssemblyRegNumbering());
+
+  // Fix debug_values whose defs have been stackified.
+  if (!WasmDisableExplicitLocals)
+    addPass(createWebAssemblyDebugFixup());
 }
 
 yaml::MachineFunctionInfo *
 WebAssemblyTargetMachine::createDefaultFuncInfoYAML() const {
   return new yaml::WebAssemblyFunctionInfo();
 }
 
 yaml::MachineFunctionInfo *WebAssemblyTargetMachine::convertFuncInfoToYAML(

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

+; RUN: llc < %s | FileCheck %s
+
+; Input C code:
+
+;  int i = input();  // Nested case
+;  int j = input();  // Trivial def-use.
+;  output(i, j);
+
+; The ll below generates 330 lines of .S, so relevant parts that the
+; WebAssemblyDebugFixup pass affects:
+
+; CHECK:  call  input
+; CHECK: .Ltmp0:
+; CHECK:  call  input
+; CHECK: .Ltmp1:
+; CHECK:  call  output
+; CHECK: .Ltmp2:
+
+; This defines variable "i" which is live on the stack between Ltmp0 and Ltmp2,
+; 2 = TI_OPERAND_STACK and 0 = stack offset.
+
+; CHECK:  .section  .debug_loc,"",@
+; CHECK: .Ldebug_loc0:
+; CHECK:  .int32  .Ltmp0-.Lfunc_begin0
+; CHECK:  .int32  .Ltmp2-.Lfunc_begin0
+; CHECK:  .int16  4                       # Loc expr size
+; CHECK:  .int8 237                       # DW_OP_WASM_location
+; CHECK:  .int8 2                         # 2
+; CHECK:  .int8 0                         # 0
+; CHECK:  .int8 159                       # DW_OP_stack_value
+
+; This defines variable "j" which is live on the stack between Ltmp1 and Ltmp2,
+; 2 = TI_OPERAND_STACK and 1 = stack offset.
+
+; CHECK: .Ldebug_loc1:
+; CHECK:  .int32  .Ltmp1-.Lfunc_begin0
+; CHECK:  .int32  .Ltmp2-.Lfunc_begin0
+; CHECK:  .int16  4                       # Loc expr size
+; CHECK:  .int8 237                       # DW_OP_WASM_location
+; CHECK:  .int8 2                         # 2
+; CHECK:  .int8 1                         # 1
+; CHECK:  .int8 159                       # DW_OP_stack_value
+
+
+
+
+source_filename = "stackified.c"
+target triple = "wasm32-unknown-unknown"
+
+define void @foo() !dbg !12 {
+entry:
+  %call = call i32 @input(), !dbg !18
+  call void @llvm.dbg.value(metadata i32 %call, metadata !16, metadata !DIExpression()), !dbg !19
+  %call1 = call i32 @input(), !dbg !20
+  call void @llvm.dbg.value(metadata i32 %call1, metadata !17, metadata !DIExpression()), !dbg !19
+  call void @output(i32 %call, i32 %call1), !dbg !21
+  ret void, !dbg !22
+}
+
+declare i32 @input()
+
+declare !dbg !4 void @output(i32, i32)
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git ed7aaf832444411ce93aa0443425ce401f5c7a8e)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+!1 = !DIFile(filename: "stackified.c", directory: "C:\\stuff\\llvm-project")
+!2 = !{}
+!3 = !{!4}
+!4 = !DISubprogram(name: "output", scope: !1, file: !1, line: 2, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!5 = !DISubroutineType(types: !6)
+!6 = !{null, !7, !7}
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 7, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git ed7aaf832444411ce93aa0443425ce401f5c7a8e)"}
+!12 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !13, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null}
+!15 = !{!16, !17}
+!16 = !DILocalVariable(name: "i", scope: !12, file: !1, line: 4, type: !7)
+!17 = !DILocalVariable(name: "j", scope: !12, file: !1, line: 5, type: !7)
+!18 = !DILocation(line: 4, column: 11, scope: !12)
+!19 = !DILocation(line: 0, scope: !12)
+!20 = !DILocation(line: 5, column: 11, scope: !12)
+!21 = !DILocation(line: 6, column: 3, scope: !12)
+!22 = !DILocation(line: 7, column: 1, scope: !12)