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

Reply via email to