shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[WebAssembly] Demote PHIs in catchswitch BB only (#81570)

Browse Source 
`DemoteCatchSwitchPHIOnly` option in `WinEHPrepare` pass was added in
99d60e0dab,
because Wasm EH uses `WinEHPrepare`, but it doesn't need to demote all
PHIs. PHIs in `catchswitch` BBs have to be removed (= demoted) because
`catchswitch`s are removed in ISel and `catchswitch` BBs are removed as
well, so they can't have other instructions.

But because Wasm EH doesn't use funclets, so PHIs in `catchpad` or
`cleanuppad` BBs don't need to be demoted. That was the reason
`DemoteCatchSwitchPHIOnly` option was added, in order not to demote more
instructions unnecessarily.

The problem is it should have been set to `true` for Wasm EH. (Its
default value is `false` for WinEH) And I mistakenly set it to `false`
and wasn't aware about this for more than 5 years. This was not the end
of the world; it just means we've been demoting more instructions than
we should, possibly huting code size. In practice I think it would've
had hardly any effect in real performance given that the occurrence of
PHIs in `catchpad` or `cleanuppad` BBs are not very frequent and many
people run other optimizers like Binaryen anyway.
This commit is contained in:
Heejin Ahn 2024-02-13 13:43:21 -08:00 committed by GitHub
parent 8c56e78ec5
commit 473ef10b0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 4 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
View File

@ -249,7 +249,8 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
"WinEHPrepare failed to remove PHIs from imaginary BBs"); "WinEHPrepare failed to remove PHIs from imaginary BBs");
continue; continue;
} }
if (isa<FuncletPadInst>(PadInst)) if (isa<FuncletPadInst>(PadInst) &&
Personality != EHPersonality::Wasm_CXX)
assert(&*BB.begin() == PadInst && "WinEHPrepare failed to demote PHIs"); assert(&*BB.begin() == PadInst && "WinEHPrepare failed to demote PHIs");
} }

2
llvm/lib/CodeGen/TargetPassConfig.cpp
View File

@ -918,7 +918,7 @@ void TargetPassConfig::addPassesToHandleExceptions() {
// on catchpads and cleanuppads because it does not outline them into // on catchpads and cleanuppads because it does not outline them into
// funclets. Catchswitch blocks are not lowered in SelectionDAG, so we // funclets. Catchswitch blocks are not lowered in SelectionDAG, so we
// should remove PHIs there. // should remove PHIs there.
addPass(createWinEHPass(/*DemoteCatchSwitchPHIOnly=*/false)); addPass(createWinEHPass(/*DemoteCatchSwitchPHIOnly=*/true));
addPass(createWasmEHPass()); addPass(createWasmEHPass());
break; break;
case ExceptionHandling::None: case ExceptionHandling::None:

2
llvm/test/CodeGen/WebAssembly/wasm-eh-prepare.ll
View File

@ -2,6 +2,7 @@
; RUN: opt < %s -win-eh-prepare -demote-catchswitch-only -wasm-eh-prepare -S --mattr=+atomics,+bulk-memory | FileCheck %s ; RUN: opt < %s -win-eh-prepare -demote-catchswitch-only -wasm-eh-prepare -S --mattr=+atomics,+bulk-memory | FileCheck %s
; RUN: opt < %s -passes='win-eh-prepare<demote-catchswitch-only>,wasm-eh-prepare' -S | FileCheck %s ; RUN: opt < %s -passes='win-eh-prepare<demote-catchswitch-only>,wasm-eh-prepare' -S | FileCheck %s
; RUN: opt < %s -passes='win-eh-prepare<demote-catchswitch-only>,wasm-eh-prepare' -S --mattr=+atomics,+bulk-memory | FileCheck %s ; RUN: opt < %s -passes='win-eh-prepare<demote-catchswitch-only>,wasm-eh-prepare' -S --mattr=+atomics,+bulk-memory | FileCheck %s
; RUN: llc < %s -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling -stop-after=wasm-eh-prepare | FileCheck %s
target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128" target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
target triple = "wasm32-unknown-unknown" target triple = "wasm32-unknown-unknown"
@ -245,7 +246,6 @@ bb.true: ; preds = %entry
bb.true.0: ; preds = %bb.true bb.true.0: ; preds = %bb.true
br label %merge br label %merge
; CHECK: bb.false
bb.false: ; preds = %entry bb.false: ; preds = %entry
br label %merge br label %merge