majnemer added inline comments.
================
Comment at: lib/CodeGen/CGException.cpp:1173
+ cast<llvm::CatchPadInst>(CatchStartBlock->getFirstNonPHI());
+ CurrentFuncletPad = CPI;
+ }
----------------
aheejin wrote:
> majnemer wrote:
> > Hmm, why is this done? Won't RestoreCurrentFuncletPad undo this?
> Isn't `RestoreCurrentFuncletPad` outside of `if
> (EHPersonality::get(*this).isWasmPersonality())`? Isn't this supposed to stay
> until this function finishes?
Ah, true! Nevermind!
================
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:
> 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...
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
