[WebAssembly] Remove FAKE_USEs before ExplicitLocals

[aheejin]

FAKE_USEs are essentially no-ops, so they have to be removed before running ExplicitLocals so that drops will be correctly inserted to drop those values used by the FAKE_USEs.

Fixes emscripten-core/emscripten#25301.

[llvmbot]

@llvm/pr-subscribers-backend-webassembly

Author: Heejin Ahn (aheejin)

Changes

FAKE_USEs are essentially no-ops, so they have to be removed before running ExplicitLocals so that drops will be correctly inserted to drop those values used by the FAKE_USEs.

Fixes emscripten-core/emscripten#25301.

---

Full diff: https://github.com/llvm/llvm-project/pull/160228.diff

2 Files Affected:

• (modified) llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp (+14)
• (added) llvm/test/CodeGen/WebAssembly/fake-use.ll (+15)

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
index e6486e247209b..014cd99b9aa23 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
@@ -216,6 +216,18 @@ static MachineInstr *findStartOfTree(MachineOperand &MO,
   return Def;
 }
 
+// FAKE_USEs are no-ops, so remove them here so that the values used by them
+// will be correctly dropped later.
+static void removeFakeUses(MachineFunction &MF) {
+  SmallVector<MachineInstr*> ToDelete;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      if (MI.isFakeUse())
+        ToDelete.push_back(&MI);
+  for (auto *MI : ToDelete)
+    MI->eraseFromParent();
+}
+
 bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) {
   LLVM_DEBUG(dbgs() << "********** Make Locals Explicit **********\n"
                        "********** Function: "
@@ -226,6 +238,8 @@ bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) {
   WebAssemblyFunctionInfo &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
 
+  removeFakeUses(MF);
+
   // Map non-stackified virtual registers to their local ids.
   DenseMap<unsigned, unsigned> Reg2Local;
 
diff --git a/llvm/test/CodeGen/WebAssembly/fake-use.ll b/llvm/test/CodeGen/WebAssembly/fake-use.ll
new file mode 100644
index 0000000000000..d5732e628440a
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/fake-use.ll
@@ -0,0 +1,15 @@
+; RUN: llc < %s | llvm-mc -triple=wasm32-unknown-unknown
+
+target triple = "wasm32-unknown-unknown"
+
+define void @fake_use_test() {
+  %t = call i32 @foo()
+  tail call void (...) @llvm.fake.use(i32 %t)
+  ret void
+}
+
+declare void @foo()
+; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite)
+declare void @llvm.fake.use(...) #0
+
+attributes #0 = { mustprogress nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite) }

[aheejin]

llvm-mc runs the type checker. Without this patch this wouldn't succeed.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[dschuff]

So if I understand correctly, the FAKE_USEs are for extending the lifetime of values (and therefore will extend the lifetime of registers/live ranges). Because explicit locals is so late in the pipeline, it seems like it will also have that effect in Wasm, right? In other words, I don't think there's really anything else we need to do in order for this intrinsic to have its desired effect of improving debugging?

[aheejin]

That's my understanding. If we reaaaally want to delay this until the very end, we can insert a pass right before the AsmPrinter to essentially convert these to drops, but I don't think it's worth it and we can just rely on ExplicitLocal to do its normal job here.

[dschuff]

No that makes sense to me. I don't think there are any passes later than this that would move things around anyway.

[dschuff]

This is breaking Emscripten's test_oz_size:

https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8702897759465673873/+/u/Emscripten_testsuite__other_/stdout

Assertion failed: (MFI.isVRegStackified(MI.getOperand(0).getReg())), function runOnMachineFunction, file WebAssemblyExplicitLocals.cpp, line 330.

I'll go ahead and revert for now to unblock the roller, we can figure out the fix later.