Skip to content

Commit 445f4e7

Browse files
committed
[WebAssembly] Disable wasm.lsda() optimization in WasmEHPrepare
In every catchpad except `catch (...)`, we add a call to `_Unwind_CallPersonality`, which is a wapper to call the personality function. (In most of other Itanium-based architectures the call is done from libunwind, but in wasm we don't have the control over the VM.) Because the personatlity function is called to figure out whether the current exception is a type we should catch, such as `int` or `SomeClass&`, `catch (...)` does not need the personality function call. For the same reason, all cleanuppads don't need it. When we call `_Unwind_CallPersonality`, we store some necessary info in a data structure called `__wasm_lpad_context` of type `_Unwind_LandingPadContext`, which is defined in the wasm's port of libunwind in Emscripten. Also the personality wrapper function returns some info (selector and the caught pointer) in that data structure, so it is used as a medium for communication. One of the info we need to store is the address for LSDA info for the current function. `wasm.lsda()` intrinsic returns that address. (This intrinsic will be lowered to a symbol that points to the LSDA address.) The simpliest thing is call `wasm.lsda()` every time we need to call `_Unwind_CallPersonality` and store that info in `__wasm_lpad_context` data structure. But we tried to be better than that (D77423 and some more previous CLs), so if catchpad A dominates catchpad B and catchpad A is not `catch (...)`, we didn't insert `wasm.lsda()` call in catchpad B, thinking that the LSDA address is the same for a single function and we already visited catchpad A and `__wasm_lpad_context.lsda` field would already have that value. But this can be incorrect if there is a call to another function, which also can have the personality function and LSDA, between catchpad A and catchpad B, because `__wasm_lpad_context` is a globally defined structure and the callee function will overwrite its `lsda` field. So in this CL we don't try to do any optimizaions on adding `wasm.lsda()` call; we store the result of `wasm.lsda()` every time we call `_Unwind_CallPersonality`. We can do some complicated analysis, like checking if there is a function call between the dominating catchpad and the current catchpad, but at this time it seems overkill. This deletes three tests because they all tested `wasm.ldsa()` call optimization. Fixes emscripten-core/emscripten#13548. Reviewed By: tlively Differential Revision: https://reviews.llvm.org/D97309
1 parent abd3c6f commit 445f4e7

File tree

2 files changed

+47
-401
lines changed

2 files changed

+47
-401
lines changed

llvm/lib/CodeGen/WasmEHPrepare.cpp

+39-101
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@
7777
//
7878
//===----------------------------------------------------------------------===//
7979

80-
#include "llvm/ADT/BreadthFirstIterator.h"
8180
#include "llvm/ADT/SetVector.h"
8281
#include "llvm/ADT/Statistic.h"
8382
#include "llvm/ADT/Triple.h"
@@ -117,13 +116,9 @@ class WasmEHPrepare : public FunctionPass {
117116
FunctionCallee CallPersonalityF =
118117
nullptr; // _Unwind_CallPersonality() wrapper
119118

120-
bool prepareEHPads(Function &F);
121119
bool prepareThrows(Function &F);
122-
123-
bool IsEHPadFunctionsSetUp = false;
124-
void setupEHPadFunctions(Function &F);
125-
void prepareEHPad(BasicBlock *BB, bool NeedPersonality, bool NeedLSDA = false,
126-
unsigned Index = 0);
120+
bool prepareEHPads(Function &F);
121+
void prepareEHPad(BasicBlock *BB, bool NeedPersonality, unsigned Index = 0);
127122

128123
public:
129124
static char ID; // Pass identification, replacement for typeid
@@ -176,7 +171,6 @@ static void eraseDeadBBsAndChildren(const Container &BBs, DomTreeUpdater *DTU) {
176171
}
177172

178173
bool WasmEHPrepare::runOnFunction(Function &F) {
179-
IsEHPadFunctionsSetUp = false;
180174
bool Changed = false;
181175
Changed |= prepareThrows(F);
182176
Changed |= prepareEHPads(F);
@@ -216,95 +210,23 @@ bool WasmEHPrepare::prepareThrows(Function &F) {
216210
}
217211

218212
bool WasmEHPrepare::prepareEHPads(Function &F) {
219-
auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
220-
bool Changed = false;
213+
Module &M = *F.getParent();
214+
IRBuilder<> IRB(F.getContext());
221215

222-
// There are two things to decide: whether we need a personality function call
223-
// and whether we need a `wasm.lsda()` call and its store.
224-
//
225-
// For the personality function call, catchpads with `catch (...)` and
226-
// cleanuppads don't need it, because exceptions are always caught. Others all
227-
// need it.
228-
//
229-
// For `wasm.lsda()` and its store, in order to minimize the number of them,
230-
// we need a way to figure out whether we have encountered `wasm.lsda()` call
231-
// in any of EH pads that dominates the current EH pad. To figure that out, we
232-
// now visit EH pads in BFS order in the dominator tree so that we visit
233-
// parent BBs first before visiting its child BBs in the domtree.
234-
//
235-
// We keep a set named `ExecutedLSDA`, which basically means "Do we have
236-
// `wasm.lsda() either in the current EH pad or any of its parent EH pads in
237-
// the dominator tree?". This is to prevent scanning the domtree up to the
238-
// root every time we examine an EH pad, in the worst case: each EH pad only
239-
// needs to check its immediate parent EH pad.
240-
//
241-
// - If any of its parent EH pads in the domtree has `wasm.lsda`, this means
242-
// we don't need `wasm.lsda()` in the current EH pad. We also insert the
243-
// current EH pad in `ExecutedLSDA` set.
244-
// - If none of its parent EH pad has `wasm.lsda()`,
245-
// - If the current EH pad is a `catch (...)` or a cleanuppad, done.
246-
// - If the current EH pad is neither a `catch (...)` nor a cleanuppad,
247-
// add `wasm.lsda()` and the store in the current EH pad, and add the
248-
// current EH pad to `ExecutedLSDA` set.
249-
//
250-
// TODO Can we not store LSDA address in user function but make libcxxabi
251-
// compute it?
252-
DenseSet<Value *> ExecutedLSDA;
253-
unsigned Index = 0;
254-
for (auto DomNode : breadth_first(&DT)) {
255-
auto *BB = DomNode->getBlock();
256-
auto *Pad = BB->getFirstNonPHI();
257-
if (!Pad || (!isa<CatchPadInst>(Pad) && !isa<CleanupPadInst>(Pad)))
216+
SmallVector<BasicBlock *, 16> CatchPads;
217+
SmallVector<BasicBlock *, 16> CleanupPads;
218+
for (BasicBlock &BB : F) {
219+
if (!BB.isEHPad())
258220
continue;
259-
Changed = true;
260-
261-
Value *ParentPad = nullptr;
262-
if (CatchPadInst *CPI = dyn_cast<CatchPadInst>(Pad)) {
263-
ParentPad = CPI->getCatchSwitch()->getParentPad();
264-
if (ExecutedLSDA.count(ParentPad)) {
265-
ExecutedLSDA.insert(CPI);
266-
// We insert its associated catchswitch too, because
267-
// FuncletPadInst::getParentPad() returns a CatchSwitchInst if the child
268-
// FuncletPadInst is a CleanupPadInst.
269-
ExecutedLSDA.insert(CPI->getCatchSwitch());
270-
}
271-
} else { // CleanupPadInst
272-
ParentPad = cast<CleanupPadInst>(Pad)->getParentPad();
273-
if (ExecutedLSDA.count(ParentPad))
274-
ExecutedLSDA.insert(Pad);
275-
}
276-
277-
if (CatchPadInst *CPI = dyn_cast<CatchPadInst>(Pad)) {
278-
if (CPI->getNumArgOperands() == 1 &&
279-
cast<Constant>(CPI->getArgOperand(0))->isNullValue())
280-
// In case of a single catch (...), we need neither personality call nor
281-
// wasm.lsda() call
282-
prepareEHPad(BB, false);
283-
else {
284-
if (ExecutedLSDA.count(CPI))
285-
// catch (type), but one of parents already has wasm.lsda() call
286-
prepareEHPad(BB, true, false, Index++);
287-
else {
288-
// catch (type), and none of parents has wasm.lsda() call. We have to
289-
// add the call in this EH pad, and record this EH pad in
290-
// ExecutedLSDA.
291-
ExecutedLSDA.insert(CPI);
292-
ExecutedLSDA.insert(CPI->getCatchSwitch());
293-
prepareEHPad(BB, true, true, Index++);
294-
}
295-
}
296-
} else if (isa<CleanupPadInst>(Pad)) {
297-
// Cleanup pads need neither personality call nor wasm.lsda() call
298-
prepareEHPad(BB, false);
299-
}
221+
auto *Pad = BB.getFirstNonPHI();
222+
if (isa<CatchPadInst>(Pad))
223+
CatchPads.push_back(&BB);
224+
else if (isa<CleanupPadInst>(Pad))
225+
CleanupPads.push_back(&BB);
300226
}
227+
if (CatchPads.empty() && CleanupPads.empty())
228+
return false;
301229

302-
return Changed;
303-
}
304-
305-
void WasmEHPrepare::setupEHPadFunctions(Function &F) {
306-
Module &M = *F.getParent();
307-
IRBuilder<> IRB(F.getContext());
308230
assert(F.hasPersonalityFn() && "Personality function not found");
309231

310232
// __wasm_lpad_context global variable
@@ -336,16 +258,30 @@ void WasmEHPrepare::setupEHPadFunctions(Function &F) {
336258
"_Unwind_CallPersonality", IRB.getInt32Ty(), IRB.getInt8PtrTy());
337259
if (Function *F = dyn_cast<Function>(CallPersonalityF.getCallee()))
338260
F->setDoesNotThrow();
261+
262+
unsigned Index = 0;
263+
for (auto *BB : CatchPads) {
264+
auto *CPI = cast<CatchPadInst>(BB->getFirstNonPHI());
265+
// In case of a single catch (...), we don't need to emit a personalify
266+
// function call
267+
if (CPI->getNumArgOperands() == 1 &&
268+
cast<Constant>(CPI->getArgOperand(0))->isNullValue())
269+
prepareEHPad(BB, false);
270+
else
271+
prepareEHPad(BB, true, Index++);
272+
}
273+
274+
// Cleanup pads don't need a personality function call.
275+
for (auto *BB : CleanupPads)
276+
prepareEHPad(BB, false);
277+
278+
return true;
339279
}
340280

341281
// Prepare an EH pad for Wasm EH handling. If NeedPersonality is false, Index is
342282
// ignored.
343283
void WasmEHPrepare::prepareEHPad(BasicBlock *BB, bool NeedPersonality,
344-
bool NeedLSDA, unsigned Index) {
345-
if (!IsEHPadFunctionsSetUp) {
346-
IsEHPadFunctionsSetUp = true;
347-
setupEHPadFunctions(*BB->getParent());
348-
}
284+
unsigned Index) {
349285
assert(BB->isEHPad() && "BB is not an EHPad!");
350286
IRBuilder<> IRB(BB->getContext());
351287
IRB.SetInsertPoint(&*BB->getFirstInsertionPt());
@@ -399,9 +335,11 @@ void WasmEHPrepare::prepareEHPad(BasicBlock *BB, bool NeedPersonality,
399335
IRB.CreateStore(IRB.getInt32(Index), LPadIndexField);
400336

401337
auto *CPI = cast<CatchPadInst>(FPI);
402-
if (NeedLSDA)
403-
// Pseudocode: __wasm_lpad_context.lsda = wasm.lsda();
404-
IRB.CreateStore(IRB.CreateCall(LSDAF), LSDAField);
338+
// TODO Sometimes storing the LSDA address every time is not necessary, in
339+
// case it is already set in a dominating EH pad and there is no function call
340+
// between from that EH pad to here. Consider optimizing those cases.
341+
// Pseudocode: __wasm_lpad_context.lsda = wasm.lsda();
342+
IRB.CreateStore(IRB.CreateCall(LSDAF), LSDAField);
405343

406344
// Pseudocode: _Unwind_CallPersonality(exn);
407345
CallInst *PersCI = IRB.CreateCall(CallPersonalityF, CatchCI,

0 commit comments

Comments
 (0)