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); + } ---------------- majnemer wrote: > aheejin wrote: > > 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. > I guess I'm worried that we could have emitted statements inside the > catch(int) and catch(float) blocks and we'd either run into a terminator > which isn't a BranchInst. > If we could not emit any statements yet, then I think this is OK... Actually it's after we emit all catch handlers, but I think this routine is not gonna touch the contents of those handlers. So it's like ``` Start --> int? (N) --> float? (N) --> rethrow (Y) (Y) handler handler ``` So however much control flow each handler contains, if we follow only (N)s, we will end up in the rethrow block. Repository: rC Clang https://reviews.llvm.org/D44931 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits