dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:278
+ // Two SjLj modes cannot be turned on at the same time
+ assert(!(EnableEmSjLj && EnableWasmSjLj));
+ // Wasm SjLj should be only used with Wasm EH
----------------
You could put the comment text directly in the assert, i.e. `assert(condition
&& "text")`
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:48
+// Wasm exception handling using wasm EH instructions
+cl::opt<bool> WasmEnableEH("wasm-enable-eh",
----------------
this description seems a bit redundant. I guess "C++ exception handling using
wasm EH instructions" might be a bit too specific since it could be anything
that generates WinEH-style IR. Maybe just leave out the first "wasm"?
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:449
+ // done in WasmEHPrepare pass after these IR passes, but Wasm SjLj requires
+ // Emscripten libraries and processed together in LowerEmscriptenEHSjLJ pass.
+ if (WasmEnableEmEH || WasmEnableEmSjLj || WasmEnableSjLj)
----------------
it's not clear what "and processed" is intended to mean here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107685/new/
https://reviews.llvm.org/D107685
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
