[PATCH] D108960: [WebAssembly] Add Wasm SjLj support

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:11
+/// This file lowers exception-related instructions and setjmp/longjmp function
+/// calls to use Emscripten's library functions. The pass uses JavaScript's try
+/// and catch mechanism in case of Emscripten EH/SjLj and Wasm EH intrinsics in
----------------
dschuff wrote:
> Eventually it might be nice to make it possible to use wasm SJLJ without needing emscripten, which should be feasible since it doesn't need JS. I guess that would mean moving or copying some library functions out of emscripten proper, and maybe refactoring the code or something. It doesn't have to be soon though.
What this uses from Emscripten is just a couple of C library functions, mainly `saveSetjmp` and `testSetjmp` in https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/emscripten_setjmp.c. It doesn't need JS. Also it needs `__wasm_longjmp` which calls our Clang builtin. These functions have to be somewhere. If not Emscripten, where do you suggest? We can construct these functions in LLVM but not sure what the benefit for that is..?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:199
+///
+/// In case calls to setjmp() exists
+///
----------------
dschuff wrote:
> 
Changed to "If there are calls to setjmp()", to be consistent with your comment above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108960/new/

https://reviews.llvm.org/D108960