Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Dec 8, 2025

Backport e5b2a06

Requested by: @dschuff

@llvmbot
Copy link
Member Author

llvmbot commented Dec 8, 2025

@dschuff What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Dec 8, 2025

@llvm/pr-subscribers-backend-webassembly

Author: None (llvmbot)

Changes

Backport e5b2a06

Requested by: @dschuff


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

3 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp (+14)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp (+4)
  • (added) llvm/test/CodeGen/WebAssembly/fake-use.ll (+25)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
index e6486e247209b..5c3127e2d3dc6 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/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
index bc91c6424b63e..fd13ef9a1921d 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
@@ -866,6 +866,10 @@ bool WebAssemblyRegStackify::runOnMachineFunction(MachineFunction &MF) {
       if (Insert->isDebugValue())
         continue;
 
+      // Ignore FAKE_USEs, which are no-ops and will be deleted later.
+      if (Insert->isFakeUse())
+        continue;
+
       // Iterate through the inputs in reverse order, since we'll be pulling
       // operands off the stack in LIFO order.
       CommutingState Commuting;
diff --git a/llvm/test/CodeGen/WebAssembly/fake-use.ll b/llvm/test/CodeGen/WebAssembly/fake-use.ll
new file mode 100644
index 0000000000000..a18ce33566df0
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/fake-use.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | llvm-mc -triple=wasm32-unknown-unknown
+
+target triple = "wasm32-unknown-unknown"
+
+define void @fake_use() {
+  %t = call i32 @foo()
+  tail call void (...) @llvm.fake.use(i32 %t)
+  ret void
+}
+
+; %t shouldn't be converted to TEE in RegStackify, because the FAKE_USE will be
+; deleted in the beginning of ExplicitLocals.
+define void @fake_use_no_tee() {
+  %t = call i32 @foo()
+  tail call void (...) @llvm.fake.use(i32 %t)
+  call void @use(i32 %t)
+  ret void
+}
+
+declare i32 @foo()
+declare void @use(i32 %t)
+; 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) }

@dschuff
Copy link
Member

dschuff commented Dec 8, 2025

This patch fixes a regression bug from the previous LLVM version and maintains API and ABI compatibility, so I think it qualifies for a release branch patch.

@dyung dyung moved this from Needs Triage to Needs Merge in LLVM Release Status Dec 11, 2025
`FAKE_USE`s are essentially no-ops, so they have to be removed before
running ExplicitLocals so that `drop`s will be correctly inserted to
drop those values used by the `FAKE_USE`s.

---

This is reapplication of llvm#160228, which broke Wasm waterfall. This PR
additionally prevents `FAKE_USE`s uses from being stackified.

Previously, a 'def' whose first use was a `FAKE_USE` was able to be
stackified as `TEE`:
- Before
```
Reg = INST ...            // Def
FAKE_USE ..., Reg, ...    // Insert
INST ..., Reg, ...
INST ..., Reg, ...
```

- After RegStackify
```
DefReg = INST ...            // Def
TeeReg, Reg = TEE ... DefReg
FAKE_USE ..., TeeReg, ...    // Insert
INST ..., Reg, ...
INST ..., Reg, ...
```
And this assumes `DefReg` and `TeeReg` are stackified.

But this PR removes `FAKE_USE`s in the beginning of ExplicitLocals. And
later in ExplicitLocals we have a routine to unstackify registers that
have no uses left:

https://github.com/llvm/llvm-project/blob/7b28fcd2b182ba2c9d2d71c386be92fc0ee3cc9d/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp#L257-L269
(This was added in llvm#149626. Then it didn't seem it would trigger the
same assertions for `TEE`s because it was fixing the bug where a
terminator was removed in CFGSort (llvm#149097).
Details here:
llvm#149432 (comment))

- After `FAKE_USE` removal and unstackification
```
DefReg = INST ...
TeeReg, Reg = TEE ... DefReg
INST ..., Reg, ...
INST ..., Reg, ...
```
And now `TeeReg` is unstackified. This triggered the assertion here,
that `TeeReg` should be stackified:

https://github.com/llvm/llvm-project/blob/7b28fcd2b182ba2c9d2d71c386be92fc0ee3cc9d/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp#L316

This prevents `FAKE_USE`s' uses from being stackified altogether,
including `TEE` transformation. Even when it is not a `TEE`
transformation and just a single use stackification, it does not trigger
the assertion but there's no point stackifying it given that it will be
deleted.

---

Fixes emscripten-core/emscripten#25301.

(cherry picked from commit e5b2a06)
@dyung dyung merged commit 4b24e73 into llvm:release/21.x Dec 12, 2025
2 of 3 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Dec 12, 2025
@github-actions
Copy link

@dschuff (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

4 participants