[PATCH] D43746: [WebAssembly] Add Wasm exception handling prepare pass

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/CodeGen/WasmEHPrepare.cpp:269-275
+  for (auto &I : *BB)
+    if (auto *CI = dyn_cast<CallInst>(&I)) {
+      if (CI->getCalledValue() == GetExnF)
+        GetExnCI = CI;
+      if (CI->getCalledValue() == GetSelectorF)
+        GetSelectorCI = CI;
+    }
----------------
majnemer wrote:
> aheejin wrote:
> > majnemer wrote:
> > > What if BB branches to a block which calls wasm.get.exception? What if two different catchpads branch to a block which calls wasm.get.exception? ISTM that you might want to give it the token of the catchpad/cleanuppad somehow?
> > Oh, I haven't thought about that possibility.. I kind of assumed that `wasm.get.exception` and `wasm.get.selector` intrinsics cannot be reordered past a catchpad's terminator because [[ https://github.com/llvm-mirror/llvm/blob/0ec6e3688b87112c68c882be89e59a7f6c1a0382/include/llvm/IR/IntrinsicsWebAssembly.td#L45-L47 | they are marked as `IntrHasSideEffects` ]]. Is this assumption wrong?
> > 
> > And can I attach a funclet bundle to function calls that are marked as `nounwind`? Looks like `CodeGenFunction::getBundlesForFunclet` function [[ https://github.com/llvm-mirror/clang/blob/9d9b33c8989461d0a4faffa865da817cba4d0bf4/lib/CodeGen/CGCall.cpp#L3667-L3670 |  does not add a funclet bundle operand in case the callee cannot throw ]]. I guess I still manually can add a funclet operand bundle though. Did you mean that?
> I don't think they will get reordered past a catchret. However, I don't see anything that prevents:
> 
> 
> ```
> bb0:
>   catchpad []
>   br cont
> 
> cont:
>   wasm.get.exception
> ```
> 
> or
> 
> 
> ```
> bb0:
>   catchpad []
>   br common
> 
> bb1:
>   catchpad []
>   br common
> 
> common:
>   wasm.get.exception
> ```
> 
> Instead of attaching a funclet bundle to the intrinsic call, I'd do what the int_coro_* family of intrinsics do and pass the token as an argument to wasm.get.exception/wasm.get.selector.
> 
> Then, it is as easy as walking all the users of the catchpad token to find which wasm.get.exception/wasm.get.selector you need to replace.
Thank you! That's a good point I have't thought about. I updated D44931 accordingly as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D43746