[WebAssembly] WebAssemblyLowerEmscriptenEHSjLj: use getter/setter for accessing tempRet0

sbc100 · sbc100 · commit 4791a668f573 · 2018-11-20T19:25:07.000Z
Rather than assuming that `tempRet0` exists in linear memory only assume the getter/setter functions exist. This avoids conflicting with binaryen which declares a wasm global for this purpose and defines it's own getter and setter for that. The other advantage of doing things this way is that it leaving it up to the linker/finalizer to decide how to actually store this temporary. As it happens binaryen uses a wasm global which is more appropriate since it is thread safe. This also allows us to change the way this is stored in the future (memory, TLS memory, wasm global) without modifying LLVM. This is part of a 4 part change: LLVM: https://reviews.llvm.org/D53240 fastcomp: emscripten-core/emscripten-fastcomp#237 emscripten: emscripten-core/emscripten#7358 binaryen: WebAssembly/binaryen#1709 Differential Revision: https://reviews.llvm.org/D53240 llvm-svn: 347340
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -50,24 +50,19 @@
 ///
 /// In detail, this pass does following things:
 ///
-/// 1) Assumes the existence of global variables: __THREW__, __threwValue, and
-///    __tempRet0.
-///    __tempRet0 will be set within __cxa_find_matching_catch() function in
-///    JS library, and __THREW__ and __threwValue will be set in invoke wrappers
+/// 1) Assumes the existence of global variables: __THREW__, __threwValue
+///    __THREW__ and __threwValue will be set in invoke wrappers
 ///    in JS glue code. For what invoke wrappers are, refer to 3). These
 ///    variables are used for both exceptions and setjmp/longjmps.
 ///    __THREW__ indicates whether an exception or a longjmp occurred or not. 0
 ///    means nothing occurred, 1 means an exception occurred, and other numbers
 ///    mean a longjmp occurred. In the case of longjmp, __threwValue variable
 ///    indicates the corresponding setjmp buffer the longjmp corresponds to.
-///    In exception handling, __tempRet0 indicates the type of an exception
-///    caught, and in setjmp/longjmp, it means the second argument to longjmp
-///    function.
 ///
 /// * Exception handling
 ///
-/// 2) We assume the existence of setThrew and setTempRet0 functions at link
-///    time.
+/// 2) We assume the existence of setThrew and setTempRet0/getTempRet0 functions
+///    at link time.
 ///    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
@@ -80,10 +75,12 @@
 ///        __threwValue = value;
 ///      }
 ///    }
+//
+///    setTempRet0 is called from __cxa_find_matching_catch() in JS glue code.
 ///
-///    function setTempRet0(value) {
-///      __tempRet0 = value;
-///    }
+///    In exception handling, getTempRet0 indicates the type of an exception
+///    caught, and in setjmp/longjmp, it means the second argument to longjmp
+///    function.
 ///
 /// 3) Lower
 ///      invoke @func(arg1, arg2) to label %invoke.cont unwind label %lpad
@@ -120,11 +117,10 @@
 ///      ... use %val ...
 ///    into
 ///      %fmc = call @__cxa_find_matching_catch_N(c1, c2, c3, ...)
-///      %val = {%fmc, __tempRet0}
+///      %val = {%fmc, getTempRet0()}
 ///      ... use %val ...
 ///    Here N is a number calculated based on the number of clauses.
-///    Global variable __tempRet0 is set within __cxa_find_matching_catch() in
-///    JS glue code.
+///    setTempRet0 is called from __cxa_find_matching_catch() in JS glue code.
 ///
 /// 5) Lower
 ///      resume {%a, %b}
@@ -162,15 +158,15 @@
 ///      setjmp(buf)
 ///    into
 ///      setjmpTable = saveSetjmp(buf, label, setjmpTable, setjmpTableSize);
-///      setjmpTableSize = __tempRet0;
+///      setjmpTableSize = getTempRet0();
 ///    For each dynamic setjmp call, setjmpTable stores its ID (a number which
 ///    is incrementally assigned from 0) and its label (a unique number that
 ///    represents each callsite of setjmp). When we need more entries in
 ///    setjmpTable, it is reallocated in saveSetjmp() in JS code and it will
 ///    return the new table address, and assign the new table size in
-///    __tempRet0. saveSetjmp also stores the setjmp's ID into the buffer buf.
-///    A BB with setjmp is split into two after setjmp call in order to make the
-///    post-setjmp BB the possible destination of longjmp BB.
+///    setTempRet0(). saveSetjmp also stores the setjmp's ID into the buffer
+///    buf. A BB with setjmp is split into two after setjmp call in order to
+///    make the post-setjmp BB the possible destination of longjmp BB.
 ///
 ///
 /// 4) Lower every call that might longjmp into
@@ -183,11 +179,11 @@
 ///                            setjmpTableSize);
 ///        if (%label == 0)
 ///          emscripten_longjmp(%__THREW__.val, __threwValue);
-///        __tempRet0 = __threwValue;
+///        setTempRet0(__threwValue);
 ///      } else {
 ///        %label = -1;
 ///      }
-///      longjmp_result = __tempRet0;
+///      longjmp_result = getTempRet0();
 ///      switch label {
 ///        label 1: goto post-setjmp BB 1
 ///        label 2: goto post-setjmp BB 2
@@ -246,7 +242,8 @@ class WebAssemblyLowerEmscriptenEHSjLj final : public ModulePass {
 
   GlobalVariable *ThrewGV;
   GlobalVariable *ThrewValueGV;
-  GlobalVariable *TempRet0GV;
+  Function *GetTempRet0Func;
+  Function *SetTempRet0Func;
   Function *ResumeF;
   Function *EHTypeIDF;
   Function *EmLongjmpF;
@@ -286,9 +283,10 @@ class WebAssemblyLowerEmscriptenEHSjLj final : public ModulePass {
 
   WebAssemblyLowerEmscriptenEHSjLj(bool EnableEH = true, bool EnableSjLj = true)
       : ModulePass(ID), EnableEH(EnableEH), EnableSjLj(EnableSjLj),
-        ThrewGV(nullptr), ThrewValueGV(nullptr), TempRet0GV(nullptr),
-        ResumeF(nullptr), EHTypeIDF(nullptr), EmLongjmpF(nullptr),
-        EmLongjmpJmpbufF(nullptr), SaveSetjmpF(nullptr), TestSetjmpF(nullptr) {
+        ThrewGV(nullptr), ThrewValueGV(nullptr), GetTempRet0Func(nullptr),
+        SetTempRet0Func(nullptr), ResumeF(nullptr), EHTypeIDF(nullptr),
+        EmLongjmpF(nullptr), EmLongjmpJmpbufF(nullptr), SaveSetjmpF(nullptr),
+        TestSetjmpF(nullptr) {
     EHWhitelistSet.insert(EHWhitelist.begin(), EHWhitelist.end());
   }
   bool runOnModule(Module &M) override;
@@ -514,7 +512,8 @@ bool WebAssemblyLowerEmscriptenEHSjLj::canLongjmp(Module &M,
   Function *ThrowF = M.getFunction("__cxa_throw");
   Function *TerminateF = M.getFunction("__clang_call_terminate");
   if (Callee == BeginCatchF || Callee == EndCatchF ||
-      Callee == AllocExceptionF || Callee == ThrowF || Callee == TerminateF)
+      Callee == AllocExceptionF || Callee == ThrowF || Callee == TerminateF ||
+      Callee == GetTempRet0Func || Callee == SetTempRet0Func)
     return false;
 
   // Otherwise we don't know
@@ -527,11 +526,11 @@ bool WebAssemblyLowerEmscriptenEHSjLj::canLongjmp(Module &M,
 //   %label = _testSetjmp(mem[%__THREW__.val], setjmpTable, setjmpTableSize);
 //   if (%label == 0)
 //     emscripten_longjmp(%__THREW__.val, threwValue);
-//   __tempRet0 = threwValue;
+//   setTempRet0(threwValue);
 // } else {
 //   %label = -1;
 // }
-// %longjmp_result = __tempRet0;
+// %longjmp_result = getTempRet0();
 //
 // As output parameters. returns %label, %longjmp_result, and the BB the last
 // instruction (%longjmp_result = ...) is in.
@@ -575,15 +574,15 @@ void WebAssemblyLowerEmscriptenEHSjLj::wrapTestSetjmp(
   IRB.CreateCall(EmLongjmpF, {Threw, ThrewValue});
   IRB.CreateUnreachable();
 
-  // __tempRet0 = threwValue;
+  // setTempRet0(threwValue);
   IRB.SetInsertPoint(EndBB2);
-  IRB.CreateStore(ThrewValue, TempRet0GV);
+  IRB.CreateCall(SetTempRet0Func, ThrewValue);
   IRB.CreateBr(EndBB1);
 
   IRB.SetInsertPoint(ElseBB1);
   IRB.CreateBr(EndBB1);
 
-  // longjmp_result = __tempRet0;
+  // longjmp_result = getTempRet0();
   IRB.SetInsertPoint(EndBB1);
   PHINode *LabelPHI = IRB.CreatePHI(IRB.getInt32Ty(), 2, "label");
   LabelPHI->addIncoming(ThenLabel, EndBB2);
@@ -593,7 +592,7 @@ void WebAssemblyLowerEmscriptenEHSjLj::wrapTestSetjmp(
   // Output parameter assignment
   Label = LabelPHI;
   EndBB = EndBB1;
-  LongjmpResult = IRB.CreateLoad(TempRet0GV, "longjmp_result");
+  LongjmpResult = IRB.CreateCall(GetTempRet0Func, None, "longjmp_result");
 }
 
 void WebAssemblyLowerEmscriptenEHSjLj::rebuildSSA(Function &F) {
@@ -633,12 +632,19 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
   bool LongjmpUsed = LongjmpF && !LongjmpF->use_empty();
   bool DoSjLj = EnableSjLj && (SetjmpUsed || LongjmpUsed);
 
-  // Declare (or get) global variables __THREW__, __threwValue, and __tempRet0,
-  // which are used in common for both exception handling and setjmp/longjmp
-  // handling
+  // Declare (or get) global variables __THREW__, __threwValue, and
+  // getTempRet0/setTempRet0 function which are used in common for both
+  // exception handling and setjmp/longjmp handling
   ThrewGV = getGlobalVariableI32(M, IRB, "__THREW__");
   ThrewValueGV = getGlobalVariableI32(M, IRB, "__threwValue");
-  TempRet0GV = getGlobalVariableI32(M, IRB, "__tempRet0");
+  GetTempRet0Func =
+      Function::Create(FunctionType::get(IRB.getInt32Ty(), false),
+                       GlobalValue::ExternalLinkage, "getTempRet0", &M);
+  SetTempRet0Func = Function::Create(
+      FunctionType::get(IRB.getVoidTy(), IRB.getInt32Ty(), false),
+      GlobalValue::ExternalLinkage, "setTempRet0", &M);
+  GetTempRet0Func->setDoesNotThrow();
+  SetTempRet0Func->setDoesNotThrow();
 
   bool Changed = false;
 
@@ -848,8 +854,7 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runEHOnFunction(Function &F) {
     CallInst *FMCI = IRB.CreateCall(FMCF, FMCArgs, "fmc");
     Value *Undef = UndefValue::get(LPI->getType());
     Value *Pair0 = IRB.CreateInsertValue(Undef, FMCI, 0, "pair0");
-    Value *TempRet0 =
-        IRB.CreateLoad(TempRet0GV, TempRet0GV->getName() + ".val");
+    Value *TempRet0 = IRB.CreateCall(GetTempRet0Func, None, "tempret0");
     Value *Pair1 = IRB.CreateInsertValue(Pair0, TempRet0, 1, "pair1");
 
     LPI->replaceAllUsesWith(Pair1);
@@ -930,7 +935,7 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runSjLjOnFunction(Function &F) {
     Instruction *NewSetjmpTable =
         IRB.CreateCall(SaveSetjmpF, Args, "setjmpTable");
     Instruction *NewSetjmpTableSize =
-        IRB.CreateLoad(TempRet0GV, "setjmpTableSize");
+        IRB.CreateCall(GetTempRet0Func, None, "setjmpTableSize");
     SetjmpTableInsts.push_back(NewSetjmpTable);
     SetjmpTableSizeInsts.push_back(NewSetjmpTableSize);
     ToErase.push_back(CI);
@@ -1052,7 +1057,7 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runSjLjOnFunction(Function &F) {
   // ...
   // somebb:
   //   setjmpTable = saveSetjmp(buf, label, setjmpTable, setjmpTableSize);
-  //   setjmpTableSize = __tempRet0;
+  //   setjmpTableSize = getTempRet0();
   // So we need to make sure the SSA for these variables is valid so that every
   // saveSetjmp and testSetjmp calls have the correct arguments.
   SSAUpdater SetjmpTableSSA;
diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll b/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll
@@ -7,7 +7,6 @@ target triple = "wasm32-unknown-unknown"
 @_ZTIc = external constant i8*
 ; CHECK-DAG: __THREW__ = external global i32
 ; CHECK-DAG: __threwValue = external global i32
-; CHECK-DAG: __tempRet0 = external global i32
 
 ; Test invoke instruction with clauses (try-catch block)
 define void @clause() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
@@ -36,7 +35,7 @@ lpad:                                             ; preds = %entry
 ; CHECK: lpad:
 ; CHECK-NEXT: %[[FMC:.*]] = call i8* @__cxa_find_matching_catch_4(i8* bitcast (i8** @_ZTIi to i8*), i8* null)
 ; CHECK-NEXT: %[[IVI1:.*]] = insertvalue { i8*, i32 } undef, i8* %[[FMC]], 0
-; CHECK-NEXT: %[[TEMPRET0_VAL:.*]] = load i32, i32* @__tempRet0
+; CHECK-NEXT: %[[TEMPRET0_VAL:.*]] = call i32 @getTempRet0()
 ; CHECK-NEXT: %[[IVI2:.*]] = insertvalue { i8*, i32 } %[[IVI1]], i32 %[[TEMPRET0_VAL]], 1
 ; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 0
 ; CHECK-NEXT: %[[CDR:.*]] = extractvalue { i8*, i32 } %[[IVI2]], 1
@@ -91,7 +90,7 @@ lpad:                                             ; preds = %entry
 ; CHECK: lpad:
 ; CHECK-NEXT: %[[FMC:.*]] = call i8* @__cxa_find_matching_catch_4(i8* bitcast (i8** @_ZTIi to i8*), i8* bitcast (i8** @_ZTIc to i8*))
 ; CHECK-NEXT: %[[IVI1:.*]] = insertvalue { i8*, i32 } undef, i8* %[[FMC]], 0
-; CHECK-NEXT: %[[TEMPRET0_VAL:.*]] = load i32, i32* @__tempRet0
+; CHECK-NEXT: %[[TEMPRET0_VAL:.*]] = call i32 @getTempRet0()
 ; CHECK-NEXT: %[[IVI2:.*]] = insertvalue { i8*, i32 } %[[IVI1]], i32 %[[TEMPRET0_VAL]], 1
 ; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 0
 ; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 1
@@ -168,6 +167,8 @@ declare void @__cxa_end_catch()
 declare void @__cxa_call_unexpected(i8*)
 
 ; JS glue functions and invoke wrappers declaration
+; CHECK-DAG: declare i32 @getTempRet0()
+; CHECK-DAG: declare void @setTempRet0(i32)
 ; 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*)
diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll b/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
@@ -8,7 +8,6 @@ target triple = "wasm32-unknown-unknown"
 @global_var = hidden global i32 0, align 4
 ; CHECK-DAG: __THREW__ = external global i32
 ; CHECK-DAG: __threwValue = external global i32
-; CHECK-DAG: __tempRet0 = external global i32
 
 ; Test a simple setjmp - longjmp sequence
 define hidden void @setjmp_longjmp() {
@@ -28,7 +27,7 @@ entry:
 ; CHECK-NEXT: %[[BUF:.*]] = alloca [1 x %struct.__jmp_buf_tag]
 ; CHECK-NEXT: %[[ARRAYDECAY:.*]] = getelementptr inbounds [1 x %struct.__jmp_buf_tag], [1 x %struct.__jmp_buf_tag]* %[[BUF]], i32 0, i32 0
 ; CHECK-NEXT: %[[SETJMP_TABLE1:.*]] = call i32* @saveSetjmp(%struct.__jmp_buf_tag* %[[ARRAYDECAY]], i32 1, i32* %[[SETJMP_TABLE]], i32 %[[SETJMP_TABLE_SIZE]])
-; CHECK-NEXT: %[[SETJMP_TABLE_SIZE1:.*]] = load i32, i32* @__tempRet0
+; CHECK-NEXT: %[[SETJMP_TABLE_SIZE1:.*]] = call i32 @getTempRet0()
 ; CHECK-NEXT: br label %entry.split
 
 ; CHECK: entry.split:
@@ -59,7 +58,7 @@ entry:
 
 ; CHECK: if.end:
 ; CHECK-NEXT: %[[LABEL_PHI:.*]] = phi i32 [ %[[LABEL:.*]], %if.end2 ], [ -1, %if.else1 ]
-; CHECK-NEXT: %[[LONGJMP_RESULT]] = load i32, i32* @__tempRet0
+; CHECK-NEXT: %[[LONGJMP_RESULT]] = call i32 @getTempRet0()
 ; CHECK-NEXT: switch i32 %[[LABEL_PHI]], label %entry.split.split [
 ; CHECK-NEXT:   i32 1, label %entry.split
 ; CHECK-NEXT: ]
@@ -69,7 +68,7 @@ entry:
 ; CHECK-NEXT: unreachable
 
 ; CHECK: if.end2:
-; CHECK-NEXT: store i32 %[[THREWVALUE_VAL]], i32* @__tempRet0
+; CHECK-NEXT: call void  @setTempRet0(i32 %[[THREWVALUE_VAL]])
 ; CHECK-NEXT: br label %if.end
 }
 
@@ -152,7 +151,7 @@ if.then:                                          ; preds = %entry
 ; CHECK: if.then:
 ; CHECK: %[[VAR0:.*]] = load i32, i32* @global_var, align 4
 ; CHECK: %[[SETJMP_TABLE1:.*]] = call i32* @saveSetjmp(
-; CHECK-NEXT: %[[SETJMP_TABLE_SIZE1:.*]] = load i32, i32* @__tempRet0
+; CHECK-NEXT: %[[SETJMP_TABLE_SIZE1:.*]] = call i32 @getTempRet0()
 
 ; CHECK: if.then.split:
 ; CHECK: %[[VAR1:.*]] = phi i32 [ %[[VAR0]], %if.then ], [ %[[VAR2:.*]], %if.end3 ]
@@ -201,6 +200,8 @@ declare i8* @malloc(i32)
 declare void @free(i8*)
 
 ; JS glue functions and invoke wrappers declaration
+; CHECK-DAG: declare i32 @getTempRet0()
+; CHECK-DAG: declare void @setTempRet0(i32)
 ; CHECK-DAG: declare i32* @saveSetjmp(%struct.__jmp_buf_tag*, i32, i32*, i32)
 ; CHECK-DAG: declare i32 @testSetjmp(i32, i32*, i32)
 ; CHECK-DAG: declare void @emscripten_longjmp_jmpbuf(%struct.__jmp_buf_tag*, i32)
