[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.


================
Comment at: lib/CodeGen/CGException.cpp:1241-1245
+    while (llvm::TerminatorInst *TI = RethrowBlock->getTerminator()) {
+      llvm::BranchInst *BI = cast<llvm::BranchInst>(TI);
+      assert(BI->isConditional());
+      RethrowBlock = BI->getSuccessor(1);
+    }
----------------
aheejin wrote:
> majnemer wrote:
> > This seems pretty fragile, why is this guaranteed to work? Could we maintain a map from CatchSwitchInst to catch-all block?
> The function call sequence here is `CodeGenFunction::ExitCXXTryStmt` -> `emitCatchDispatchBlock` (static) -> `emitWasmCatchDispatchBlock` (static) and `emitCatchDispatchBlock` also has other callers, so it is a little cumbersome to pass a map to those functions to be filled in. (We have to make a parameter that's only gonna be used for wasm to both `emitCatchDispatchBlock` and `emitWasmCatchDispatchBlock`)
> 
> The other way is also change those static `emit` functions into `CodeGenFunction` class's member functions and make the map as a member variable.
> 
> But first, in which case do you think this will be fragile? `emitWasmCatchDispatchBlock` follows the structure of the landingpad model, so for a C++ code like this
> ```
> try {
>   ...
> } catch (int) {
>   ...
> } catch (float) {
>   ...
> }
> ```
> the BB structure that starts from wasm's `catch.start` block will look like
> ```
> catch.dispatch:                                   ; preds = %entry
>   %0 = catchswitch within none [label %catch.start] unwind to caller
> 
> catch.start:                                      ; preds = %catch.dispatch
>   %1 = catchpad within %0 [i8* bitcast (i8** @_ZTIi to i8*), i8* bitcast (i8** @_ZTIf to i8*)]
>   %2 = call i8* @llvm.wasm.get.exception()
>   %3 = call i32 @llvm.wasm.get.ehselector()
>   %4 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*)) #2
>   %matches = icmp eq i32 %3, %4
>   br i1 %matches, label %catch12, label %catch.fallthrough
> 
> catch12:                                          ; preds = %catch.start
>   body of catch (int)
> 
> catch.fallthrough:                                ; preds = %catch.start
>   %8 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIf to i8*)) #2
>   %matches1 = icmp eq i32 %3, %8
>   br i1 %matches1, label %catch, label %rethrow
> 
> catch:                                            ; preds = %catch.fallthrough
>   body of catch (float)
> 
> rethrow:                                          ; preds = %catch.fallthrough
>   call void @__cxa_rethrow() #5 [ "funclet"(token %1) ]
>   unreachable
> ```
> 
> So to me it looks like, no matter how the bodies of `catch (int)` or `catch (float)` are complicated, there should always be blocks like `catch.start` and `catch.fallthrough`, which compares typeids and divide control flow depending on the typeid comparison. I could very well be mistaken, so please let me know if so.
Oh and the `RethrowBlock` in the code is not the same as the `catch_all` block... cleanuppads will be `catch_all` blocks in wasm, and catchpads will be `catch <C++>`. That `RethrowBlock` belongs to `catch <C++>` block, and is entered when the current exception caught is a C++ exception but does not match any of the catch clauses, so it can be rethrown to the enclosing scope.


Repository:
  rC Clang

https://reviews.llvm.org/D44931