Skip to content

Commit 15f94c2

Browse files
kripkenkripken
authored
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 parent 12add6f commit 15f94c2

File tree

9 files changed

+158
-46
lines changed

9 files changed

+158
-46
lines changed

‎build-js.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ mkdir -p ${OUT}
103103
$BINARYEN_SRC/passes/DataFlowOpts.cpp \
104104
$BINARYEN_SRC/passes/DeadCodeElimination.cpp \
105105
$BINARYEN_SRC/passes/Directize.cpp \
106+
$BINARYEN_SRC/passes/DuplicateImportElimination.cpp \
106107
$BINARYEN_SRC/passes/DuplicateFunctionElimination.cpp \
107108
$BINARYEN_SRC/passes/ExtractFunction.cpp \
108109
$BINARYEN_SRC/passes/Flatten.cpp \

‎src/passes/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ SET(passes_SOURCES
1616
DeadArgumentElimination.cpp
1717
DeadCodeElimination.cpp
1818
Directize.cpp
19+
DuplicateImportElimination.cpp
1920
DuplicateFunctionElimination.cpp
2021
ExtractFunction.cpp
2122
Flatten.cpp

‎src/passes/DuplicateFunctionElimination.cpp‎

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -24,32 +24,12 @@
2424
#include "ir/hashed.h"
2525
#include "ir/module-utils.h"
2626
#include "ir/utils.h"
27+
#include "opt-utils.h"
2728
#include "pass.h"
2829
#include "wasm.h"
2930

3031
namespace wasm {
3132

32-
struct FunctionReplacer : public WalkerPass<PostWalker<FunctionReplacer>> {
33-
bool isFunctionParallel() override { return true; }
34-
35-
FunctionReplacer(std::map<Name, Name>* replacements)
36-
: replacements(replacements) {}
37-
38-
FunctionReplacer* create() override {
39-
return new FunctionReplacer(replacements);
40-
}
41-
42-
void visitCall(Call* curr) {
43-
auto iter = replacements->find(curr->target);
44-
if (iter != replacements->end()) {
45-
curr->target = iter->second;
46-
}
47-
}
48-
49-
private:
50-
std::map<Name, Name>* replacements;
51-
};
52-
5333
struct DuplicateFunctionElimination : public Pass {
5434
void run(PassRunner* runner, Module* module) override {
5535
// Multiple iterations may be necessary: A and B may be identical only after
@@ -117,31 +97,7 @@ struct DuplicateFunctionElimination : public Pass {
11797
}),
11898
v.end());
11999
module->updateMaps();
120-
// replace direct calls
121-
FunctionReplacer(&replacements).run(runner, module);
122-
// replace in table
123-
for (auto& segment : module->table.segments) {
124-
for (auto& name : segment.data) {
125-
auto iter = replacements.find(name);
126-
if (iter != replacements.end()) {
127-
name = iter->second;
128-
}
129-
}
130-
}
131-
// replace in start
132-
if (module->start.is()) {
133-
auto iter = replacements.find(module->start);
134-
if (iter != replacements.end()) {
135-
module->start = iter->second;
136-
}
137-
}
138-
// replace in exports
139-
for (auto& exp : module->exports) {
140-
auto iter = replacements.find(exp->value);
141-
if (iter != replacements.end()) {
142-
exp->value = iter->second;
143-
}
144-
}
100+
OptUtils::replaceFunctions(runner, *module, replacements);
145101
} else {
146102
break;
147103
}

‎src/passes/DuplicateImportElimination.cpp‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright 2019 WebAssembly Community Group participants
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
//
18+
// Removes duplicate imports.
19+
//
20+
// TODO: non-function imports too
21+
//
22+
23+
#include "asm_v_wasm.h"
24+
#include "ir/import-utils.h"
25+
#include "opt-utils.h"
26+
#include "pass.h"
27+
#include "wasm.h"
28+
29+
namespace wasm {
30+
31+
struct DuplicateImportElimination : public Pass {
32+
void run(PassRunner* runner, Module* module) override {
33+
ImportInfo imports(*module);
34+
std::map<Name, Name> replacements;
35+
std::map<std::pair<Name, Name>, Name> seen;
36+
std::vector<Name> toRemove;
37+
for (auto* func : imports.importedFunctions) {
38+
auto pair = std::make_pair(func->module, func->base);
39+
auto iter = seen.find(pair);
40+
if (iter != seen.end()) {
41+
auto previousName = iter->second;
42+
auto previousFunc = module->getFunction(previousName);
43+
// It is ok to import the same thing with multiple types; we can only
44+
// merge if the types match, of course.
45+
if (getSig(previousFunc) == getSig(func)) {
46+
replacements[func->name] = previousName;
47+
toRemove.push_back(func->name);
48+
continue;
49+
}
50+
}
51+
seen[pair] = func->name;
52+
}
53+
if (!replacements.empty()) {
54+
module->updateMaps();
55+
OptUtils::replaceFunctions(runner, *module, replacements);
56+
for (auto name : toRemove) {
57+
module->removeFunction(name);
58+
}
59+
}
60+
}
61+
};
62+
63+
Pass* createDuplicateImportEliminationPass() {
64+
return new DuplicateImportElimination();
65+
}
66+
67+
} // namespace wasm

‎src/passes/opt-utils.h‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,57 @@ inline void optimizeAfterInlining(std::unordered_set<Function*>& funcs,
5353
module->updateMaps();
5454
}
5555

56+
struct CallTargetReplacer : public WalkerPass<PostWalker<CallTargetReplacer>> {
57+
bool isFunctionParallel() override { return true; }
58+
59+
CallTargetReplacer(std::map<Name, Name>* replacements)
60+
: replacements(replacements) {}
61+
62+
CallTargetReplacer* create() override {
63+
return new CallTargetReplacer(replacements);
64+
}
65+
66+
void visitCall(Call* curr) {
67+
auto iter = replacements->find(curr->target);
68+
if (iter != replacements->end()) {
69+
curr->target = iter->second;
70+
}
71+
}
72+
73+
private:
74+
std::map<Name, Name>* replacements;
75+
};
76+
77+
inline void replaceFunctions(PassRunner* runner,
78+
Module& module,
79+
std::map<Name, Name>& replacements) {
80+
// replace direct calls
81+
CallTargetReplacer(&replacements).run(runner, &module);
82+
// replace in table
83+
for (auto& segment : module.table.segments) {
84+
for (auto& name : segment.data) {
85+
auto iter = replacements.find(name);
86+
if (iter != replacements.end()) {
87+
name = iter->second;
88+
}
89+
}
90+
}
91+
// replace in start
92+
if (module.start.is()) {
93+
auto iter = replacements.find(module.start);
94+
if (iter != replacements.end()) {
95+
module.start = iter->second;
96+
}
97+
}
98+
// replace in exports
99+
for (auto& exp : module.exports) {
100+
auto iter = replacements.find(exp->value);
101+
if (iter != replacements.end()) {
102+
exp->value = iter->second;
103+
}
104+
}
105+
}
106+
56107
} // namespace OptUtils
57108
} // namespace wasm
58109

‎src/passes/pass.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ void PassRegistry::registerPasses() {
107107
"directize", "turns indirect calls into direct ones", createDirectizePass);
108108
registerPass(
109109
"dfo", "optimizes using the DataFlow SSA IR", createDataFlowOptsPass);
110+
registerPass("duplicate-import-elimination",
111+
"removes duplicate imports",
112+
createDuplicateImportEliminationPass);
110113
registerPass("duplicate-function-elimination",
111114
"removes duplicate functions",
112115
createDuplicateFunctionEliminationPass);
@@ -412,6 +415,7 @@ void PassRunner::addDefaultGlobalOptimizationPostPasses() {
412415
}
413416
// optimizations show more functions as duplicate
414417
add("duplicate-function-elimination");
418+
add("duplicate-import-elimination");
415419
add("simplify-globals");
416420
add("remove-unused-module-elements");
417421
add("memory-packing");

‎src/passes/passes.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Pass* createDAEOptimizingPass();
3535
Pass* createDataFlowOptsPass();
3636
Pass* createDeadCodeEliminationPass();
3737
Pass* createDirectizePass();
38+
Pass* createDuplicateImportEliminationPass();
3839
Pass* createDuplicateFunctionEliminationPass();
3940
Pass* createEmitTargetFeaturesPass();
4041
Pass* createExtractFunctionPass();

‎test/passes/duplicate-import-elimination.txt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
(module
2+
(type $FUNCSIG$v (func))
3+
(type $FUNCSIG$vi (func (param i32)))
4+
(import "env" "waka" (func $foo))
5+
(import "env" "waka" (func $wrong (param i32)))
6+
(table $0 2 2 funcref)
7+
(elem (i32.const 0) $foo $foo)
8+
(export "baz" (func $0))
9+
(start $foo)
10+
(func $0 (; 2 ;) (type $FUNCSIG$v)
11+
(call $foo)
12+
(call $foo)
13+
(call $wrong
14+
(i32.const 1)
15+
)
16+
)
17+
)

‎test/passes/duplicate-import-elimination.wast‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
(module
2+
(import "env" "waka" (func $foo))
3+
(import "env" "waka" (func $bar))
4+
(import "env" "waka" (func $wrong (param i32)))
5+
(table 2 2 funcref)
6+
(elem (i32.const 0) $foo $bar)
7+
(start $bar)
8+
(func "baz"
9+
(call $foo)
10+
(call $bar)
11+
(call $wrong (i32.const 1))
12+
)
13+
)
14+

0 commit comments

Comments
 (0)