Add WebAssemblyLateEHPrepare pass that does several small jobs for
exception handling. This runs before CFGSort, and is different from
WasmEHPrepare pass that runs before ISel, even though the names are
similar.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
@majnemer Continuing the discussion from D44134:
So the background was:
__clang_call_terminate is called within a cleanuppad. But cleanuppad does not give you an exception object, which __clang_call_terminate takes as an argument. So even if it is a cleanuppad, we added a wasm catch instruction there to catch an exception object. But this cleanuppad should be entered even when we catch a foreign (e.g. JavaScript) exception, which we have no exception object to deal with. So in that case we just directly all std::terminate.
So, when the clang-generated code is like
terminate: %exn = catch(0) call void @__clang_call_terminate(%exn) unreachable
processTerminatePads() appends one more terminatepad right after this terminatepad, like:
terminate-catchall: catch_all call @std::terminate unreachable
So the intention was, if we catch a C++ exception we go to terminate, and if we catch a foreign exception we end up in terminate-catchall. Here I also assumed that the terminatepad is a single BB, but as you noted, it may not be. But because this pass runs even before ExceptinInfo (D44134), this does not have the exception grouping information here.
Do you think I can merge the terminatepad into a single BB in this pass, so this duplication of terminatepad process will be easier? I can't think of a situation in which a terminatepad can have complicated control flows other than a few straight-line BBs. (Actually I can't even imagine what kind of llvm pass is gonna divide a single terminate into two or more, but we can't guarantee that can't happen, so..)
Also, the other thing is, I merge terminate pads if there are more than two of them in mergeTerminatePads() here. This is in principle violation of EH scope membership, but this happens after all MachineFunctionPasses and only remaining wasm passes after this are CFGSort and CFGStackify, so I guess it should be OK.
- Rebased onto D44090
- Not this pass deletes the following unreachable instruction after adding a rethrow instruction.
Planning two changes:
- Handling of the terminate pad
- Removing unreachable BB after rethrow
- Added a method to ensure a terminate pad is a single BB
- Using WasmEHInfo, now this adds RETHROW or RETHROW_TO_CALLER instruction depending on its unwind destination
- Ensured a rethrow instruction is in the same BB with __cxa_rethrow() call and deleted all unreachable children
- In hoistCatches, added handling of a case when catch instruction is not in the EH pad BB anymore
- Now uses several utility functions in WebAssemblyUtilities.cpp
- Added helper functions
- Misc. improvements here and there
lib/Target/WebAssembly/WebAssemblyExceptionPrepare.cpp | ||
---|---|---|
27 ↗ | (On Diff #151640) | I didn't think about the confusing-ness of this name when we put it in originally; maybe we could call it 'WebAssemblyMachineEHPrepare' or 'WebAssemblyLateEHPrepare' |
110 ↗ | (On Diff #151640) | So a function can rethrow but not have a personality function? |
168 ↗ | (On Diff #151640) | 'instruction is' |
212 ↗ | (On Diff #151640) | Would it make more sense to just have the rethrow inside of cxa_rethrow (at least if the destination is the caller)? It might save some on code size? I guess we would just have to put an 'unreachable' after the call and it wouldn't matter; or is there some other reason? |
lib/Target/WebAssembly/WebAssemblyExceptionPrepare.cpp | ||
---|---|---|
27 ↗ | (On Diff #151640) | Hmm, not to be confused with WasmEHPrepare? Yeah, that makes sense.. I'll go with WebAssemblyLateEHPrepare because 'Machine' sounds like it has its same or similar counterpart in LLVM IR passes. (like, LoopInfo vs MachineLoopInfo) |
110 ↗ | (On Diff #151640) | A function has its associated personality function when it has an EH pad (landingpad or catchpad/cleanuppad). Functions that call __cxa_rethrow() can rethrow to the caller, in which case the function doesn't have an EH pad. I didn't want to run all the functions below needlessly in that case. |
212 ↗ | (On Diff #151640) | That is the spec question. The current implementation is based on the first proposal, in which rethrow can only be placed within a catch block. We can change this later if that is shown to save code size. I'll do some experiments on that once I'm able to run things end-to-end, but I actually doubt if that would be saving much, because we can save one rethrow instruction if we put that within a function, but that also means we have to pass a first-class exception argument to __cxa_rethrow(), and to put that argument on the wasm stack we have to consume one more instruction anyway. And also, if we use first-class exception objects, now every catch clause has to have that if_except ~ else ~ end instruction sequence too, which also contributes to the code size. Anyway, I'm not saying I'm against it though. I'm saying just the current implementation is based on the first proposal :$ |