Skip to content

Commit 29cf6f7

Browse files
aheejingithub-actions[bot]
authored andcommitted
Automerge: [WebAssembly] Remove FAKE_USEs before ExplicitLocals (#160768)
`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: llvm/llvm-project#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.
2 parents 7122f38 + e5b2a06 commit 29cf6f7

File tree

3 files changed

+43
-0
lines changed

3 files changed

+43
-0
lines changed

llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,18 @@ static MachineInstr *findStartOfTree(MachineOperand &MO,
216216
return Def;
217217
}
218218

219+
// FAKE_USEs are no-ops, so remove them here so that the values used by them
220+
// will be correctly dropped later.
221+
static void removeFakeUses(MachineFunction &MF) {
222+
SmallVector<MachineInstr *> ToDelete;
223+
for (auto &MBB : MF)
224+
for (auto &MI : MBB)
225+
if (MI.isFakeUse())
226+
ToDelete.push_back(&MI);
227+
for (auto *MI : ToDelete)
228+
MI->eraseFromParent();
229+
}
230+
219231
bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) {
220232
LLVM_DEBUG(dbgs() << "********** Make Locals Explicit **********\n"
221233
"********** Function: "
@@ -226,6 +238,8 @@ bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) {
226238
WebAssemblyFunctionInfo &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
227239
const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
228240

241+
removeFakeUses(MF);
242+
229243
// Map non-stackified virtual registers to their local ids.
230244
DenseMap<unsigned, unsigned> Reg2Local;
231245

llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,10 @@ bool WebAssemblyRegStackify::runOnMachineFunction(MachineFunction &MF) {
867867
if (Insert->isDebugValue())
868868
continue;
869869

870+
// Ignore FAKE_USEs, which are no-ops and will be deleted later.
871+
if (Insert->isFakeUse())
872+
continue;
873+
870874
// Iterate through the inputs in reverse order, since we'll be pulling
871875
// operands off the stack in LIFO order.
872876
CommutingState Commuting;
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
; RUN: llc < %s | llvm-mc -triple=wasm32-unknown-unknown
2+
3+
target triple = "wasm32-unknown-unknown"
4+
5+
define void @fake_use() {
6+
%t = call i32 @foo()
7+
tail call void (...) @llvm.fake.use(i32 %t)
8+
ret void
9+
}
10+
11+
; %t shouldn't be converted to TEE in RegStackify, because the FAKE_USE will be
12+
; deleted in the beginning of ExplicitLocals.
13+
define void @fake_use_no_tee() {
14+
%t = call i32 @foo()
15+
tail call void (...) @llvm.fake.use(i32 %t)
16+
call void @use(i32 %t)
17+
ret void
18+
}
19+
20+
declare i32 @foo()
21+
declare void @use(i32 %t)
22+
; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite)
23+
declare void @llvm.fake.use(...) #0
24+
25+
attributes #0 = { mustprogress nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite) }

0 commit comments

Comments
 (0)