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

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Otherwise it looks good to me, although @majnemer would know more about the subtleties of what IR actually gets generated.



================
Comment at: lib/CodeGen/CGCleanup.h:630
   static const EHPersonality MSVC_CxxFrameHandler3;
+  static const EHPersonality GNU_Wasm_CPlusCPlus;
 
----------------
aheejin wrote:
> dschuff wrote:
> > We might consider having 2 personalities: one for use with builtin exceptions, and other for emulated exceptions? I'm imagining that with this style of IR we might be able to do emulated exceptions better than we have for emscripten today. We don't have to think about that now, but maybe the name of the personality might reflect it: e.g. `GNU_Wasm_CPlusPlus_Native` (or builtin) vs `GNU_Wasm_CPlusPlus_Emulated` (or external).
> (We talked in person :) ) I'll think about it. But I guess we can change the name once we start implementing that feature?
Sounds good. BTW this should actually be `GNU_Wasm_CPlusPlus` instead of `GNU_Wasm_CPlusCPlus`


================
Comment at: lib/CodeGen/CGException.cpp:1534
+  // In case of wasm personality, we need to pass the exception value to
+  // __clang_call_terminate function.
+  if (getLangOpts().CPlusPlus &&
----------------
aheejin wrote:
> dschuff wrote:
> > Why?
> Not strictly necessarily, because we can modify libcxxabi to our liking. I was trying to keep the same behavior as Itanium-style libcxxabi. The `__clang_call_terminate` function that's called when an EH cleanup throws is as follows:
> ```
> ; Function Attrs: noinline noreturn nounwind                                     
> define linkonce_odr hidden void @__clang_call_terminate(i8*) #6 comdat {         
>   %2 = call i8* @__cxa_begin_catch(i8* %0) #2                                    
>   call void @_ZSt9terminatev() #8                                                
>   unreachable                                                                    
> }   
> ```
> 
> So it calls `__cxa_begin_catch` on the exception value before calling `std::terminate`. We can change this behavior for wasm if we want, and I guess we need some proxy object in case of a foreign exception, but anyway I was trying to keep the behavior the same unless there's any reason not to.
Oh I see, we are deviating from MSVC behavior to be more like itanium here. Makes sense.


================
Comment at: lib/CodeGen/CGException.cpp:1541
+  }
   llvm::CallInst *terminateCall =
+      CGM.getCXXABI().emitTerminateForUnexpectedException(*this, Exn);
----------------
Should this be in an else block? No need to emit it after we emit the call to `__clang_call_terminate`


Repository:
  rC Clang

https://reviews.llvm.org/D44931