Duplicate Import Elimination (#2292)

This is both an optimization and a workaround for the problem that emscripten-core/emscripten#7641 uncovered and had to be reverted because of.

What's going on there is that wasm-emscripten-finalize turns emscripten_longjmp_jmpbuf into emscripten_longjmp (for some LLVM internal reason - there's a long comment in the source that I didn't fully follow). There are two such imports already, one for each name, and before that PR, we ended up with just one. After that PR, we end up with two. And with two, the minification of import names gets confused - we have two imports with the same name, and the code there ends up ignoring one of them.

I'm not sure why that PR changed things - I guess the wasm-emscripten-finalize code looks at the name, and that PR changed what name appears? @sbc100 maybe #2285 is related?

Anyhow, it's not trivial to make import minification code support two identical imports, but I don't think we should - we should avoid having such duplication anyhow. And we should add an assert that they don't exist (I'll open a PR for that later when it's possible).

This fixes the duplication by adding a useful pass to remove duplicate imports (just functions, for now). Pretty simple, but we didn't do it yet. Even if there is a wasm-emscripten-finalize bug we need to fix with those duplicate imports, I think this pass is still a good thing to add.

I confirmed that this fixes the issue caused by that PR.

1 files changed, 2 insertions, 46 deletions

diff --git a/src/passes/DuplicateFunctionElimination.cpp b/src/passes/DuplicateFunctionElimination.cpp
index 445a024e4..12da32806 100644
--- a/src/passes/DuplicateFunctionElimination.cpp
+++ b/src/passes/DuplicateFunctionElimination.cpp
@@ -24,32 +24,12 @@
 #include "ir/hashed.h"
 #include "ir/module-utils.h"
 #include "ir/utils.h"
+#include "opt-utils.h"
 #include "pass.h"
 #include "wasm.h"
 
 namespace wasm {
 
-struct FunctionReplacer : public WalkerPass<PostWalker<FunctionReplacer>> {
-  bool isFunctionParallel() override { return true; }
-
-  FunctionReplacer(std::map<Name, Name>* replacements)
-    : replacements(replacements) {}
-
-  FunctionReplacer* create() override {
-    return new FunctionReplacer(replacements);
-  }
-
-  void visitCall(Call* curr) {
-    auto iter = replacements->find(curr->target);
-    if (iter != replacements->end()) {
-      curr->target = iter->second;
-    }
-  }
-
-private:
-  std::map<Name, Name>* replacements;
-};
-
 struct DuplicateFunctionElimination : public Pass {
   void run(PassRunner* runner, Module* module) override {
     // Multiple iterations may be necessary: A and B may be identical only after
@@ -117,31 +97,7 @@ struct DuplicateFunctionElimination : public Pass {
                                }),
                 v.end());
         module->updateMaps();
-        // replace direct calls
-        FunctionReplacer(&replacements).run(runner, module);
-        // replace in table
-        for (auto& segment : module->table.segments) {
-          for (auto& name : segment.data) {
-            auto iter = replacements.find(name);
-            if (iter != replacements.end()) {
-              name = iter->second;
-            }
-          }
-        }
-        // replace in start
-        if (module->start.is()) {
-          auto iter = replacements.find(module->start);
-          if (iter != replacements.end()) {
-            module->start = iter->second;
-          }
-        }
-        // replace in exports
-        for (auto& exp : module->exports) {
-          auto iter = replacements.find(exp->value);
-          if (iter != replacements.end()) {
-            exp->value = iter->second;
-          }
-        }
+        OptUtils::replaceFunctions(runner, *module, replacements);
       } else {
         break;
       }