Remove now-redundant stack pointer manipulation passes (#3251)

The use of these passes was removed on the emscripten side
in https://github.com/emscripten-core/emscripten/pull/12536.

15 files changed, 14 insertions, 167 deletions

diff --git a/src/passes/CMakeLists.txt b/src/passes/CMakeLists.txt
index d352e841b..1551ba256 100644
--- a/src/passes/CMakeLists.txt
+++ b/src/passes/CMakeLists.txt
@@ -65,7 +65,6 @@ set(passes_SOURCES
   RemoveImports.cpp
   RemoveMemory.cpp
   RemoveNonJSOps.cpp
-  ReplaceStackPointer.cpp
   RemoveUnusedBrs.cpp
   RemoveUnusedNames.cpp
   RemoveUnusedModuleElements.cpp
diff --git a/src/passes/ReplaceStackPointer.cpp b/src/passes/ReplaceStackPointer.cpp
deleted file mode 100644
index da57f8c3d..000000000
--- a/src/passes/ReplaceStackPointer.cpp
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- * Copyright 2020 WebAssembly Community Group participants
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-//
-// Convert llvm's stack pointer usage from from global.get/global.set into
-// cals to external stackSave/stackRestore.
-// This is needed to emscripten SIDE_MODULE code where mutable global imports
-// are not yet permitted by default.
-//
-
-#include "abi/js.h"
-#include "ir/import-utils.h"
-#include "pass.h"
-#include "support/debug.h"
-#include "wasm-emscripten.h"
-
-#define DEBUG_TYPE "replace-stack-pointer"
-
-namespace wasm {
-
-static Name STACK_SAVE("stackSave");
-static Name STACK_RESTORE("stackRestore");
-
-struct ReplaceStackPointer
-  : public WalkerPass<PostWalker<ReplaceStackPointer>> {
-  void visitGlobalGet(GlobalGet* curr) {
-    if (getModule()->getGlobalOrNull(curr->name) == stackPointer) {
-      needStackSave = true;
-      if (!builder) {
-        builder = make_unique<Builder>(*getModule());
-      }
-      replaceCurrent(builder->makeCall(STACK_SAVE, {}, Type::i32));
-    }
-  }
-
-  void visitGlobalSet(GlobalSet* curr) {
-    if (getModule()->getGlobalOrNull(curr->name) == stackPointer) {
-      needStackRestore = true;
-      if (!builder) {
-        builder = make_unique<Builder>(*getModule());
-      }
-      replaceCurrent(
-        builder->makeCall(STACK_RESTORE, {curr->value}, Type::none));
-    }
-  }
-
-  void doWalkModule(Module* module) {
-    stackPointer = getStackPointerGlobal(*module);
-    if (!stackPointer) {
-      BYN_DEBUG(std::cerr << "no stack pointer found\n");
-      return;
-    }
-    BYN_DEBUG(std::cerr << "stack pointer found\n");
-    super::doWalkModule(module);
-    if (needStackSave) {
-      ensureFunctionImport(
-        module, STACK_SAVE, Signature(Type::none, Type::i32));
-    }
-    if (needStackRestore) {
-      ensureFunctionImport(
-        module, STACK_RESTORE, Signature(Type::i32, Type::none));
-    }
-    // Finally remove the stack pointer global itself. This avoids importing
-    // a mutable global.
-    module->removeGlobal(stackPointer->name);
-  }
-
-  void ensureFunctionImport(Module* module, Name name, Signature sig) {
-    ImportInfo info(*module);
-    if (info.getImportedFunction(ENV, name)) {
-      return;
-    }
-    auto import = new Function;
-    import->name = name;
-    import->module = ENV;
-    import->base = name;
-    import->sig = sig;
-    module->addFunction(import);
-  }
-
-private:
-  std::unique_ptr<Builder> builder;
-  Global* stackPointer = nullptr;
-  bool needStackSave = false;
-  bool needStackRestore = false;
-};
-
-Pass* createReplaceStackPointerPass() { return new ReplaceStackPointer; }
-
-} // namespace wasm
diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp
index 9b56fa54d..67e1ca53b 100644
--- a/src/passes/pass.cpp
+++ b/src/passes/pass.cpp
@@ -303,10 +303,6 @@ void PassRegistry::registerPasses() {
   registerPass("rereloop",
                "re-optimize control flow using the relooper algorithm",
                createReReloopPass);
-  registerPass("replace-stack-pointer",
-               "Replace llvm-generated stack pointer global with calls with "
-               "imported functions.",
-               createReplaceStackPointerPass);
   registerPass(
     "rse", "remove redundant local.sets", createRedundantSetEliminationPass);
   registerPass("roundtrip",
diff --git a/src/passes/passes.h b/src/passes/passes.h
index 434b6e382..a36aeb7c3 100644
--- a/src/passes/passes.h
+++ b/src/passes/passes.h
@@ -100,7 +100,6 @@ Pass* createRemoveUnusedNonFunctionModuleElementsPass();
 Pass* createRemoveUnusedNamesPass();
 Pass* createReorderFunctionsPass();
 Pass* createReorderLocalsPass();
-Pass* createReplaceStackPointerPass();
 Pass* createReReloopPass();
 Pass* createRedundantSetEliminationPass();
 Pass* createRoundTripPass();
diff --git a/src/tools/wasm-emscripten-finalize.cpp b/src/tools/wasm-emscripten-finalize.cpp
index e79bafa32..2c21d49ca 100644
--- a/src/tools/wasm-emscripten-finalize.cpp
+++ b/src/tools/wasm-emscripten-finalize.cpp
@@ -289,10 +289,6 @@ int main(int argc, const char* argv[]) {
     passRunner.add("stack-check");
   }
 
-  if (sideModule && !mutableSP) {
-    passRunner.add("replace-stack-pointer");
-  }
-
   if (legacyPIC) {
     if (sideModule) {
       passRunner.add("emscripten-pic");
@@ -334,9 +330,6 @@ int main(int argc, const char* argv[]) {
   } else {
     BYN_TRACE("finalizing as regular module\n");
     if (legacyPIC) {
-      if (!mutableSP) {
-        generator.internalizeStackPointerGlobal();
-      }
       // For side modules these gets called via __post_instantiate
       if (Function* F = wasm.getFunctionOrNull(ASSIGN_GOT_ENTRIES)) {
         auto* ex = new Export();
diff --git a/src/wasm-emscripten.h b/src/wasm-emscripten.h
index dd5c2009f..8275f7bc8 100644
--- a/src/wasm-emscripten.h
+++ b/src/wasm-emscripten.h
@@ -36,10 +36,6 @@ public:
   Function* generateAssignGOTEntriesFunction();
   void generatePostInstantiateFunction();
 
-  // Remove the import of a mutable __stack_pointer and instead initialize the
-  // stack pointer from an immutable import.
-  void internalizeStackPointerGlobal();
-
   std::string
   generateEmscriptenMetadata(Address staticBump,
                              std::vector<Name> const& initializerFunctions);
diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index dec824331..7f7b227cf 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -116,32 +116,6 @@ void EmscriptenGlueGenerator::generatePostInstantiateFunction() {
   wasm.addExport(ex);
 }
 
-// lld can sometimes produce a build with an imported mutable __stack_pointer
-// (i.e.  when linking with -fpie).  This method internalizes the
-// __stack_pointer and initializes it from an immutable global instead.
-// For -shared builds we instead call replaceStackPointerGlobal.
-void EmscriptenGlueGenerator::internalizeStackPointerGlobal() {
-  Global* stackPointer = getStackPointerGlobal(wasm);
-  if (!stackPointer || !stackPointer->imported() || !stackPointer->mutable_) {
-    return;
-  }
-
-  Name internalName = stackPointer->name;
-  Name externalName = internalName.c_str() + std::string("_import");
-
-  // Rename the imported global, and make it immutable
-  stackPointer->name = externalName;
-  stackPointer->mutable_ = false;
-  wasm.updateMaps();
-
-  // Create a new global with the old name that is not imported.
-  Builder builder(wasm);
-  auto* init = builder.makeGlobalGet(externalName, stackPointer->type);
-  auto* sp = builder.makeGlobal(
-    internalName, stackPointer->type, init, Builder::Mutable);
-  wasm.addGlobal(sp);
-}
-
 const Address UNKNOWN_OFFSET(uint32_t(-1));
 
 std::vector<Address> getSegmentOffsets(Module& wasm) {
diff --git a/test/lld/em_asm_shared.wat.out b/test/lld/em_asm_shared.wat.out
index cf5e9f65f..ddf87cf9b 100644
--- a/test/lld/em_asm_shared.wat.out
+++ b/test/lld/em_asm_shared.wat.out
@@ -1,17 +1,15 @@
 (module
- (type $none_=>_i32 (func (result i32)))
  (type $none_=>_none (func))
- (type $i32_=>_none (func (param i32)))
+ (type $none_=>_i32 (func (result i32)))
  (type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
  (type $i32_i32_i32_=>_i32 (func (param i32 i32 i32) (result i32)))
  (import "env" "memory" (memory $mimport$0 0))
  (data (global.get $gimport$3) "{ Module.print(\"Hello world\"); }\00\00{ return $0 + $1; }\00ii\00{ Module.print(\"Got \" + $0); }\00i\00")
  (import "env" "__indirect_function_table" (table $timport$1 0 funcref))
+ (import "env" "__stack_pointer" (global $gimport$2 (mut i32)))
  (import "env" "__memory_base" (global $gimport$3 i32))
  (import "env" "__table_base" (global $gimport$4 i32))
  (import "env" "emscripten_asm_const_int" (func $emscripten_asm_const_int (param i32 i32 i32) (result i32)))
- (import "env" "stackSave" (func $stackSave (result i32)))
- (import "env" "stackRestore" (func $stackRestore (param i32)))
  (import "env" "g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJEEE6bufferE" (func $g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJEEE6bufferE (result i32)))
  (import "env" "g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJiiEEE6bufferE" (func $g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJiiEEE6bufferE (result i32)))
  (import "env" "g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJiEEE6bufferE" (func $g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJiEEE6bufferE (result i32)))
@@ -36,10 +34,10 @@
  (func $__original_main (result i32)
   (local $0 i32)
   (local $1 i32)
-  (call $stackRestore
+  (global.set $gimport$2
    (local.tee $0
     (i32.sub
-     (call $stackSave)
+     (global.get $gimport$2)
      (i32.const 32)
     )
    )
@@ -84,7 +82,7 @@
     (local.get $0)
    )
   )
-  (call $stackRestore
+  (global.set $gimport$2
    (i32.add
     (local.get $0)
     (i32.const 32)
@@ -123,13 +121,12 @@
   "tableSize": 0,
   "declares": [
     "emscripten_asm_const_int",
-    "stackSave",
-    "stackRestore",
     "g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJEEE6bufferE",
     "g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJiiEEE6bufferE",
     "g$_ZN20__em_asm_sig_builderI19__em_asm_type_tupleIJiEEE6bufferE"
   ],
   "externs": [
+    "___stack_pointer",
     "___memory_base",
     "___table_base"
   ],
diff --git a/test/lld/main_module.wat.out b/test/lld/main_module.wat.out
index 290e52d06..98db00719 100644
--- a/test/lld/main_module.wat.out
+++ b/test/lld/main_module.wat.out
@@ -5,7 +5,7 @@
  (import "env" "memory" (memory $0 0))
  (data (global.get $gimport$2) "Hello, world\00\00\00\00\00\00\00\00\00\00\00\00")
  (import "env" "__indirect_function_table" (table $timport$1 0 funcref))
- (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
  (import "env" "__memory_base" (global $gimport$2 i32))
  (import "env" "__table_base" (global $gimport$3 i32))
  (import "env" "puts" (func $puts (param i32) (result i32)))
@@ -18,7 +18,6 @@
  (global $global$0 i32 (i32.const 16))
  (global $global$1 i32 (i32.const 20))
  (global $global i32 (i32.const 42))
- (global $sp (mut i32) (global.get $sp_import))
  (export "__wasm_call_ctors" (func $__wasm_call_ctors))
  (export "_Z13print_messagev" (func $print_message\28\29))
  (export "ptr_puts" (global $global$0))
diff --git a/test/lld/main_module_table.wat.out b/test/lld/main_module_table.wat.out
index 740ab5317..62ba29010 100644
--- a/test/lld/main_module_table.wat.out
+++ b/test/lld/main_module_table.wat.out
@@ -1,11 +1,10 @@
 (module
  (type $none_=>_none (func))
  (type $none_=>_i32 (func (result i32)))
- (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
  (import "env" "fp$__stdio_write$v" (func $fp$__stdio_write$v (result i32)))
  (global $gimport$9 (mut i32) (i32.const 0))
  (global $global i32 (i32.const 42))
- (global $sp (mut i32) (global.get $sp_import))
  (export "__stdio_write" (func $__stdio_write))
  (export "__data_end" (global $global))
  (export "__assign_got_enties" (func $__assign_got_enties))
diff --git a/test/lld/main_module_table_2.wat.out b/test/lld/main_module_table_2.wat.out
index 201a58fa4..1a0cceed6 100644
--- a/test/lld/main_module_table_2.wat.out
+++ b/test/lld/main_module_table_2.wat.out
@@ -2,11 +2,10 @@
  (type $none_=>_none (func))
  (type $none_=>_i32 (func (result i32)))
  (import "env" "table" (table $timport$9 1 funcref))
- (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
  (import "env" "fp$__stdio_write$v" (func $fp$__stdio_write$v (result i32)))
  (global $gimport$9 (mut i32) (i32.const 0))
  (global $global i32 (i32.const 42))
- (global $sp (mut i32) (global.get $sp_import))
  (export "__stdio_write" (func $__stdio_write))
  (export "__data_end" (global $global))
  (export "__assign_got_enties" (func $__assign_got_enties))
diff --git a/test/lld/main_module_table_3.wat.out b/test/lld/main_module_table_3.wat.out
index 201a58fa4..1a0cceed6 100644
--- a/test/lld/main_module_table_3.wat.out
+++ b/test/lld/main_module_table_3.wat.out
@@ -2,11 +2,10 @@
  (type $none_=>_none (func))
  (type $none_=>_i32 (func (result i32)))
  (import "env" "table" (table $timport$9 1 funcref))
- (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
  (import "env" "fp$__stdio_write$v" (func $fp$__stdio_write$v (result i32)))
  (global $gimport$9 (mut i32) (i32.const 0))
  (global $global i32 (i32.const 42))
- (global $sp (mut i32) (global.get $sp_import))
  (export "__stdio_write" (func $__stdio_write))
  (export "__data_end" (global $global))
  (export "__assign_got_enties" (func $__assign_got_enties))
diff --git a/test/lld/main_module_table_4.wat.out b/test/lld/main_module_table_4.wat.out
index ab25bc110..35f5e2ea1 100644
--- a/test/lld/main_module_table_4.wat.out
+++ b/test/lld/main_module_table_4.wat.out
@@ -2,12 +2,11 @@
  (type $none_=>_none (func))
  (type $none_=>_i32 (func (result i32)))
  (import "env" "table" (table $timport$9 1 funcref))
- (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
  (import "env" "__table_base" (global $tb i32))
  (import "env" "fp$__stdio_write$v" (func $fp$__stdio_write$v (result i32)))
  (global $gimport$9 (mut i32) (i32.const 0))
  (global $global i32 (i32.const 42))
- (global $sp (mut i32) (global.get $sp_import))
  (export "__stdio_write" (func $__stdio_write))
  (export "__data_end" (global $global))
  (export "__assign_got_enties" (func $__assign_got_enties))
diff --git a/test/lld/main_module_table_5.wat.out b/test/lld/main_module_table_5.wat.out
index bdc75b825..a2526fc48 100644
--- a/test/lld/main_module_table_5.wat.out
+++ b/test/lld/main_module_table_5.wat.out
@@ -4,12 +4,11 @@
  (type $none_=>_i32 (func (result i32)))
  (import "env" "table" (table $timport$9 1 funcref))
  (elem (global.get $tb) $other $stuff)
- (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
  (import "env" "__table_base" (global $tb i32))
  (import "env" "fp$__stdio_write$v" (func $fp$__stdio_write$v (result i32)))
  (global $gimport$9 (mut i32) (i32.const 0))
  (global $global i32 (i32.const 42))
- (global $sp (mut i32) (global.get $sp_import))
  (export "__stdio_write" (func $__stdio_write))
  (export "__data_end" (global $global))
  (export "dynCall_v" (func $dynCall_v))
diff --git a/test/lld/shared_add_to_table.wasm.out b/test/lld/shared_add_to_table.wasm.out
index c7a39b1bf..22a608f1b 100644
--- a/test/lld/shared_add_to_table.wasm.out
+++ b/test/lld/shared_add_to_table.wasm.out
@@ -6,6 +6,7 @@
  (import "env" "memory" (memory $mimport$0 0))
  (data (global.get $gimport$1) "*\00\00\00")
  (import "env" "__indirect_function_table" (table $timport$0 0 funcref))
+ (import "env" "__stack_pointer" (global $gimport$0 (mut i32)))
  (import "env" "__memory_base" (global $gimport$1 i32))
  (import "env" "__table_base" (global $gimport$2 i32))
  (import "env" "_Z16waka_func_theirsi" (func $waka_func_theirs\28int\29 (param i32) (result i32)))
@@ -102,6 +103,7 @@
     "fp$_Z14waka_func_minei$ii"
   ],
   "externs": [
+    "___stack_pointer",
     "___memory_base",
     "___table_base"
   ],