[WebAssembly] Update WebAssemblyLowerEmscriptenEHSjLj to handle separate compilation

[sbc100]

Summary

Previously we were assuming whole program compilation. Now that
separate compilation is a thing we need to update this pass.
Firstly, it can no longer assert on the existence of malloc and free.
This functions might not be in the current translation unit. If we
need them then we will generate not imports for them.

Secondly the global helper function we create should be marked as
weak since we will be generating a separate copy in each translation
unit.

Finally the names of the symbols used must be unique and fixed since
they need to agree across translation units.

[sbc100]

• cleanup

[dschuff]

I think I actually prefer the approach of linking this code in from a library rather than generating N copies of it. It's more consistent with how system code is usually included into programs (i.e. linking it in rather than generating it magically from the compiler). It's also easier to read and understand for non-compiler developers.

[sbc100]

I agree it that the other approach is more elegant. However, I decided to go with this one for now since:
(1) Its a smaller incremental change
(2) Hopefully all this code will be removed pretty soon anyway

We can always improve on this incrementally too.

[aheejin]

lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

310

Why delete and inline these strings in code?

336

Nit: Maybe createInt32GlobalVariable? Because the int32 type is hardcoded here

723

What happens now with the new linker when malloc and free are not in the module?

[sbc100]

• feedback

[sbc100]

• feedback

[sbc100]

lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

310

I find this pattern needlessly complex for single use strings. At least in lld we try to avoid this.

723

If you don't include malloc/free (i.e. if you don't link with libc) and this pass adds references you will get "undefined symbol: malloc" at link time. Seems reasonable?

[aheejin]

lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

310

Hmm, I see. Then maybe it would be better to inline other strings to be consistent, not necessarily in this CL though...

723

Yeah that sounds good.

[aheejin]

I'm OK with this, but @dschuff might prefer the library approach..?

[sbc100]

I think we can still do that library option, with this as an intermediate step. This change has the nice property of not requiring an emscripten-side change.

lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

static cl::list<std::string>
     EHWhitelist("emscripten-cxx-exceptions-whitelist",
                 cl::desc("The list of function names in which Emscripten-style "
                          "exception handling is enabled (see emscripten "
                          "EMSCRIPTEN_CATCHING_WHITELIST options)"),
                 cl::CommaSeparated);
 
 namespace {
 class WebAssemblyLowerEmscriptenEHSjLj final : public ModulePass {
-  static const char *ThrewGVName;
-  static const char *ThrewValueGVName;
-  static const char *TempRet0GVName;
   static const char *ResumeFName;
   static const char *EHTypeIDFName;
-  static const char *SetThrewFName;
-  static const char *SetTempRet0FName;
   static const char *EmLongjmpFName;
   static const char *EmLongjmpJmpbufFName;
   static const char *SaveSetjmpFName;
   static const char *TestSetjmpFName;
   static const char *FindMatchingCatchPrefix;
   static const char *InvokePrefix;
 
   bool EnableEH;   // Enable exception handling
public:
   bool runOnModule(Module &M) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<DominatorTreeWrapperPass>();
   }
 };
 } // End anonymous namespace
 
-const char *WebAssemblyLowerEmscriptenEHSjLj::ThrewGVName = "__THREW__";
-const char *WebAssemblyLowerEmscriptenEHSjLj::ThrewValueGVName = "__threwValue";
-const char *WebAssemblyLowerEmscriptenEHSjLj::TempRet0GVName = "__tempRet0";
 const char *WebAssemblyLowerEmscriptenEHSjLj::ResumeFName = "__resumeException";
 const char *WebAssemblyLowerEmscriptenEHSjLj::EHTypeIDFName =
     "llvm_eh_typeid_for";
-const char *WebAssemblyLowerEmscriptenEHSjLj::SetThrewFName = "setThrew";
-const char *WebAssemblyLowerEmscriptenEHSjLj::SetTempRet0FName = "setTempRet0";
 const char *WebAssemblyLowerEmscriptenEHSjLj::EmLongjmpFName =
     "emscripten_longjmp";
 const char *WebAssemblyLowerEmscriptenEHSjLj::EmLongjmpJmpbufFName =
     "emscripten_longjmp_jmpbuf";
 const char *WebAssemblyLowerEmscriptenEHSjLj::SaveSetjmpFName = "saveSetjmp";
 const char *WebAssemblyLowerEmscriptenEHSjLj::TestSetjmpFName = "testSetjmp";
 const char *WebAssemblyLowerEmscriptenEHSjLj::FindMatchingCatchPrefix =
     "__cxa_find_matching_catch_";
if (const auto *F = dyn_cast<const Function>(V)) {
     if (Name == "setjmp" || Name == "longjmp")
       return false;
     return !F->doesNotThrow();
   }
   // not a function, so an indirect call - can throw, we can't tell
   return true;
 }
 
-// Returns an available name for a global value.
-// If the proposed name already exists in the module, adds '_' at the end of
-// the name until the name is available.
-static inline std::string createGlobalValueName(const Module &M,
-                                                const std::string &Propose) {
-  std::string Name = Propose;
-  while (M.getNamedGlobal(Name))
-    Name += "_";
-  return Name;
+static GlobalVariable *createGlobalVariable(Module &M, IRBuilder<> &IRB,
+                                            const char *Name) {
+  if (M.getNamedGlobal(Name))
+    report_fatal_error(Twine("variable name is reserved: ") + Name);
+
+  return new GlobalVariable(M, IRB.getInt32Ty(), false,
+                            GlobalValue::WeakODRLinkage, IRB.getInt32(0), Name);
 }
 
 // Simple function name mangler.
 // This function simply takes LLVM's string representation of parameter types
 // and concatenate them with '_'. There are non-alphanumeric characters but llc
 // is ok with it, and we need to postprocess these names after the lowering
 // phase anyway.
 static std::string getSignature(FunctionType *FTy) {

 //     __THREW__ = threw;
 //     __threwValue = value;
 //   }
 // }
 void WebAssemblyLowerEmscriptenEHSjLj::createSetThrewFunction(Module &M) {
   LLVMContext &C = M.getContext();
   IRBuilder<> IRB(C);
 
-  assert(!M.getNamedGlobal(SetThrewFName) && "setThrew already exists");
+  if (M.getNamedGlobal("setThrew"))
+    report_fatal_error("setThrew already exists");
+
   Type *Params[] = {IRB.getInt32Ty(), IRB.getInt32Ty()};
   FunctionType *FTy = FunctionType::get(IRB.getVoidTy(), Params, false);
   Function *F =
-      Function::Create(FTy, GlobalValue::ExternalLinkage, SetThrewFName, &M);
+      Function::Create(FTy, GlobalValue::WeakODRLinkage, "setThrew", &M);
   Argument *Arg1 = &*(F->arg_begin());
   Argument *Arg2 = &*std::next(F->arg_begin());
   Arg1->setName("threw");
   Arg2->setName("value");
   BasicBlock *EntryBB = BasicBlock::Create(C, "entry", F);
   BasicBlock *ThenBB = BasicBlock::Create(C, "if.then", F);
   BasicBlock *EndBB = BasicBlock::Create(C, "if.end", F);
 

 // Create setTempRet0 function
 // function setTempRet0(value) {
 //   __tempRet0 = value;
 // }
 void WebAssemblyLowerEmscriptenEHSjLj::createSetTempRet0Function(Module &M) {
   LLVMContext &C = M.getContext();
   IRBuilder<> IRB(C);
 
-  assert(!M.getNamedGlobal(SetTempRet0FName) && "setTempRet0 already exists");
+  if (M.getNamedGlobal("setTempRet0"))
+    report_fatal_error("setTempRet0 already exists");
   Type *Params[] = {IRB.getInt32Ty()};
   FunctionType *FTy = FunctionType::get(IRB.getVoidTy(), Params, false);
   Function *F =
-      Function::Create(FTy, GlobalValue::ExternalLinkage, SetTempRet0FName, &M);
+      Function::Create(FTy, GlobalValue::WeakODRLinkage, "setTempRet0", &M);
   F->arg_begin()->setName("value");
   BasicBlock *EntryBB = BasicBlock::Create(C, "entry", F);
   IRB.SetInsertPoint(EntryBB);
   IRB.CreateStore(&*F->arg_begin(), TempRet0GV);
   IRB.CreateRetVoid();
 }
 
 void WebAssemblyLowerEmscriptenEHSjLj::rebuildSSA(Function &F) {
bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
   Function *SetjmpF = M.getFunction("setjmp");
   Function *LongjmpF = M.getFunction("longjmp");
   bool SetjmpUsed = SetjmpF && !SetjmpF->use_empty();
   bool LongjmpUsed = LongjmpF && !LongjmpF->use_empty();
   bool DoSjLj = EnableSjLj && (SetjmpUsed || LongjmpUsed);
 
   // 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));
+  ThrewGV = createGlobalVariable(M, IRB, "__THREW__");
+  ThrewValueGV = createGlobalVariable(M, IRB, "__threwValue");
+  TempRet0GV = createGlobalVariable(M, IRB, "__tempRet0");
 
   bool Changed = false;
 
   // Exception handling
   if (EnableEH) {
     // Register __resumeException function
     FunctionType *ResumeFTy =
         FunctionType::get(IRB.getVoidTy(), IRB.getInt8PtrTy(), false);
for (Function &F : M) {
         continue;
       Changed |= runEHOnFunction(F);
     }
   }
 
   // Setjmp/longjmp handling
   if (DoSjLj) {
     Changed = true; // We have setjmp or longjmp somewhere
 
-    Function *MallocF = M.getFunction("malloc");
-    Function *FreeF = M.getFunction("free");
-    if (!MallocF || !FreeF)
-      report_fatal_error(
-          "malloc and free must be linked into the module if setjmp is used");
-
     // Register saveSetjmp function
     FunctionType *SetjmpFTy = SetjmpF->getFunctionType();
     SmallVector<Type *, 4> Params = {SetjmpFTy->getParamType(0),
                                      IRB.getInt32Ty(), Type::getInt32PtrTy(C),
                                      IRB.getInt32Ty()};
     FunctionType *FTy =
         FunctionType::get(Type::getInt32PtrTy(C), Params, false);
     SaveSetjmpF = Function::Create(FTy, GlobalValue::ExternalLinkage,

test/CodeGen/WebAssembly/lower-em-exceptions.ll

 ; RUN: opt < %s -wasm-lower-em-ehsjlj -S | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 @_ZTIi = external constant i8*
 @_ZTIc = external constant i8*
-; CHECK-DAG: @[[__THREW__:__THREW__.*]] = global i32 0
-; CHECK-DAG: @[[THREWVALUE:__threwValue.*]] = global i32 0
-; CHECK-DAG: @[[TEMPRET0:__tempRet0.*]] = global i32 0
+; CHECK-DAG: @[[__THREW__:__THREW__.*]] = weak_odr global i32 0
+; CHECK-DAG: @[[THREWVALUE:__threwValue.*]] = weak_odr global i32 0
+; CHECK-DAG: @[[TEMPRET0:__tempRet0.*]] = weak_odr global i32 0
 
 ; Test invoke instruction with clauses (try-catch block)
 define void @clause() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 ; CHECK-LABEL: @clause(
 entry:
   invoke void @foo(i32 3)
           to label %invoke.cont unwind label %lpad
 ; CHECK: entry:

 declare void @__cxa_call_unexpected(i8*)
 
 ; JS glue functions and invoke wrappers declaration
 ; CHECK-DAG: declare void @__resumeException(i8*)
 ; CHECK-DAG: declare void @__invoke_void_i32(void (i32)*, i32)
 ; CHECK-DAG: declare i8* @__cxa_find_matching_catch_4(i8*, i8*)
 
 ; setThrew function creation
-; CHECK-LABEL: define void @setThrew(i32 %threw, i32 %value) {
+; CHECK-LABEL: define weak_odr void @setThrew(i32 %threw, i32 %value) {
 ; CHECK: entry:
 ; CHECK-NEXT: %[[__THREW__]].val = load i32, i32* @[[__THREW__]]
 ; CHECK-NEXT: %cmp = icmp eq i32 %[[__THREW__]].val, 0
 ; CHECK-NEXT: br i1 %cmp, label %if.then, label %if.end
 ; CHECK: if.then:
 ; CHECK-NEXT: store i32 %threw, i32* @[[__THREW__]]
 ; CHECK-NEXT: store i32 %value, i32* @[[THREWVALUE]]
 ; CHECK-NEXT: br label %if.end
 ; CHECK: if.end:
 ; CHECK-NEXT: ret void
 ; CHECK: }
 
 ; setTempRet0 function creation
-; CHECK-LABEL: define void @setTempRet0(i32 %value) {
+; CHECK-LABEL: define weak_odr void @setTempRet0(i32 %value) {
 ; CHECK: entry:
 ; CHECK-NEXT: store i32 %value, i32* @[[TEMPRET0]]
 ; CHECK-NEXT: ret void
 ; CHECK: }

test/CodeGen/WebAssembly/lower-em-sjlj.ll

 ; RUN: opt < %s -wasm-lower-em-ehsjlj -S | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 %struct.__jmp_buf_tag = type { [6 x i32], i32, [32 x i32] }
 
 @global_var = hidden global i32 0, align 4
-; CHECK-DAG: @[[__THREW__:__THREW__.*]] = global i32 0
-; CHECK-DAG: @[[THREWVALUE:__threwValue.*]] = global i32 0
-; CHECK-DAG: @[[TEMPRET0:__tempRet0.*]] = global i32 0
+; CHECK-DAG: @[[__THREW__:__THREW__.*]] = weak_odr global i32 0
+; CHECK-DAG: @[[THREWVALUE:__threwValue.*]] = weak_odr global i32 0
+; CHECK-DAG: @[[TEMPRET0:__tempRet0.*]] = weak_odr global i32 0
 
 ; Test a simple setjmp - longjmp sequence
 define hidden void @setjmp_longjmp() {
 ; CHECK-LABEL: @setjmp_longjmp
 entry:
   %buf = alloca [1 x %struct.__jmp_buf_tag], align 16
   %arraydecay = getelementptr inbounds [1 x %struct.__jmp_buf_tag], [1 x %struct.__jmp_buf_tag]* %buf, i32 0, i32 0
   %call = call i32 @setjmp(%struct.__jmp_buf_tag* %arraydecay) #0