[WebAssembly] Unstackify registers with no uses in ExplicitLocals #149626
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
There are cases we end up removing some intructions that use stackified registers after RegStackify. For example, ```wasm bb.0: %0 = ... ;; %0 is stackified br_if %bb.1, %0 bb.1: ``` In this code, br_if will be removed in CFGSort, so we should unstackify %0 so that it can be correctly dropped in ExplicitLocals. Rather than handling this in case-by-case basis, this PR just unstackifies all stackifies register with no uses, so that they can be correctly dropped. Fixes llvm#149097.
Member
Author
Member
|
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesThere are cases we end up removing some intructions that use stackified registers after RegStackify. For example, bb.0:
%0 = ... ;; %0 is stackified
br_if %bb.1, %0
bb.1:In this code, br_if will be removed in CFGSort, so we should unstackify %0 so that it can be correctly dropped in ExplicitLocals. Rather than handling this in case-by-case basis, this PR just unstackifies all stackifies register with no uses, so that they can be correctly dropped. Fixes #149097. 2 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
index 2662241ef8499..ad28e36f29112 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
@@ -256,9 +256,17 @@ bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) {
// Precompute the set of registers that are unused, so that we can insert
// drops to their defs.
+ // And unstackify any stackified registers that don't have any uses, so that
+ // they can be dropped later. This can happen when transformations after
+ // RegStackify removes instructions that use stackified registers.
BitVector UseEmpty(MRI.getNumVirtRegs());
- for (unsigned I = 0, E = MRI.getNumVirtRegs(); I < E; ++I)
- UseEmpty[I] = MRI.use_empty(Register::index2VirtReg(I));
+ for (unsigned I = 0, E = MRI.getNumVirtRegs(); I < E; ++I) {
+ Register Reg = Register::index2VirtReg(I);
+ if (MRI.use_empty(Reg)) {
+ UseEmpty[I] = true;
+ MFI.unstackifyVReg(Reg);
+ }
+ }
// Visit each instruction in the function.
for (MachineBasicBlock &MBB : MF) {
diff --git a/llvm/test/CodeGen/WebAssembly/removed-terminator.ll b/llvm/test/CodeGen/WebAssembly/removed-terminator.ll
new file mode 100644
index 0000000000000..8c9b6ff4bca97
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/removed-terminator.ll
@@ -0,0 +1,14 @@
+; RUN: llc -O0 < %s
+
+target triple = "wasm32-unknown-unknown"
+
+define void @test(i1 %x) {
+ %y = xor i1 %x, true
+ ; This br_if's operand (%y) is stackified in RegStackify. But this terminator
+ ; will be removed in CFGSort after that. We need to make sure we unstackify %y
+ ; so that it can be dropped in ExplicitLocals.
+ br i1 %y, label %exit, label %exit
+
+exit:
+ ret void
+}
|
nikic
reviewed
Jul 19, 2025
nikic
approved these changes
Jul 22, 2025
Contributor
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
LGTM. I confirmed that this also fixes my original issue.
Contributor
|
/cherry-pick b13bca7 |
Member
|
/pull-request #150187 |
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jul 24, 2025
…vm#149626) There are cases we end up removing some intructions that use stackified registers after RegStackify. For example, ```wasm bb.0: %0 = ... ;; %0 is stackified br_if %bb.1, %0 bb.1: ``` In this code, br_if will be removed in CFGSort, so we should unstackify %0 so that it can be correctly dropped in ExplicitLocals. Rather than handling this in case-by-case basis, this PR just unstackifies all stackifies register with no uses in the beginning of ExplicitLocals, so that they can be correctly dropped. Fixes llvm#149097. (cherry picked from commit b13bca7)
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
…vm#149626) There are cases we end up removing some intructions that use stackified registers after RegStackify. For example, ```wasm bb.0: %0 = ... ;; %0 is stackified br_if %bb.1, %0 bb.1: ``` In this code, br_if will be removed in CFGSort, so we should unstackify %0 so that it can be correctly dropped in ExplicitLocals. Rather than handling this in case-by-case basis, this PR just unstackifies all stackifies register with no uses in the beginning of ExplicitLocals, so that they can be correctly dropped. Fixes llvm#149097.
aheejin
added a commit
to aheejin/llvm-project
that referenced
this pull request
Sep 25, 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.
aheejin
added a commit
that referenced
this pull request
Sep 25, 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 #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 #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 (#149097). Details here: #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.
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.
dyung
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Dec 12, 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)
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.
There are cases we end up removing some intructions that use stackified registers after RegStackify. For example,
In this code, br_if will be removed in CFGSort, so we should unstackify %0 so that it can be correctly dropped in ExplicitLocals.
Rather than handling this in case-by-case basis, this PR just unstackifies all stackifies register with no uses in the beginning of ExplicitLocals, so that they can be correctly dropped.
Fixes #149097.