[PATCH] D49208: [WebAssembly] Update WebAssemblyLowerEmscriptenEHSjLj to support separate compilation.

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:72
+///    The global variables in 1) will exist in wasm address space,
+///    but their values should be set in JS code, so these functions
 ///    as interfaces to JS glue code. These functions are equivalent to the
----------------
Because you are gonna provide `setThrew` and `setTempRet0` in a separate cpp file, "their values should be set in JS code" does not strictly sound correct, because that cpp file is gonna be a part of wasm as well. Could you elaborate that these functions will be provided from library that's a part of emscripten?

And for the equivalent asm.js version of JS code below ↓ , I know that's not a part of your change, but I think we can delete this by now, because we are gonna switch to the wasm as a default backend anyway. Maybe show the code in cpp form would be better.


================
Comment at: lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:339
+                                         const std::string &Name) {
+  GlobalVariable* V = M.getNamedGlobal(Name);
+  if (V)
----------------
Nit: LLVM style is `GlobalVariable *V`


================
Comment at: lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:632
 
-  // Create global variables __THREW__, threwValue, and __tempRet0, which are
-  // used in common for both exception handling and setjmp/longjmp handling
-  ThrewGV = new GlobalVariable(M, IRB.getInt32Ty(), false,
-                               GlobalValue::ExternalLinkage, IRB.getInt32(0),
-                               createGlobalValueName(M, ThrewGVName));
-  ThrewValueGV = new GlobalVariable(
-      M, IRB.getInt32Ty(), false, GlobalValue::ExternalLinkage, IRB.getInt32(0),
-      createGlobalValueName(M, ThrewValueGVName));
-  TempRet0GV = new GlobalVariable(M, IRB.getInt32Ty(), false,
-                                  GlobalValue::ExternalLinkage, IRB.getInt32(0),
-                                  createGlobalValueName(M, TempRet0GVName));
+  // Declare (or get) global variables __THREW__, threwValue, and __tempRet0,
+  // which are used in common for both exception handling and setjmp/longjmp
----------------
`threwValue` -> `__threwValue`


Repository:
  rL LLVM

https://reviews.llvm.org/D49208