[WebAssembly] Remove FAKE_USEs before ExplicitLocals #160768
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`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.
|
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) Changes
This is reapplication of #160228, which broke Wasm waterfall. This PR additionally prevents Previously, a 'def' whose first use was a
And this assumes But this PR removes llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp Lines 257 to 269 in 7b28fcd TEEs because it was fixing the bug where a terminator was removed in CFGSort (#149097).Details here: #149432 (comment))
And now This prevents Fixes emscripten-core/emscripten#25301. Full diff: https://github.com/llvm/llvm-project/pull/160768.diff 3 Files Affected:
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 7591541779884..97545872e251b 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
@@ -870,6 +870,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
approved these changes
Sep 25, 2025
|
Now that -Og, is not just exactly the same as -O1, I wonder if we should have more emscripten tests that cover it. |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FAKE_USEs are essentially no-ops, so they have to be removed before running ExplicitLocals so thatdrops will be correctly inserted to drop those values used by theFAKE_USEs.This is reapplication of #160228, which broke Wasm waterfall. This PR additionally prevents
FAKE_USEs uses from being stackified.Previously, a 'def' whose first use was a
FAKE_USEwas able to be stackified asTEE:And this assumes
DefRegandTeeRegare stackified.But this PR removes
FAKE_USEs in the beginning of ExplicitLocals. And later in ExplicitLocals we have a routine to unstackify registers that have no uses left:llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
Lines 257 to 269 in 7b28fcd
TEEs because it was fixing the bug where a terminator was removed in CFGSort (#149097).Details here: #149432 (comment))
FAKE_USEremoval and unstackificationAnd now
TeeRegis unstackified. This triggered the assertion here, thatTeeRegshould be stackified:llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
Line 316 in 7b28fcd
This prevents
FAKE_USEs' uses from being stackified altogether, includingTEEtransformation. Even when it is not aTEEtransformation 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.