Skip to content

Commit 9628a03

Browse files
kripkenkripken
authored
legalize invokes even when doing minimal legalization, as js needs them (#1883)
See [emscripten-core/emscripten#7679
1 parent 5bd9908 commit 9628a03

File tree

3 files changed

+71
-43
lines changed

3 files changed

+71
-43
lines changed

‎src/passes/LegalizeJSInterface.cpp‎

Lines changed: 47 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -63,58 +63,57 @@ struct LegalizeJSInterface : public Pass {
6363
}
6464
}
6565
}
66-
if (full) {
67-
// Avoid iterator invalidation later.
68-
std::vector<Function*> originalFunctions;
69-
for (auto& func : module->functions) {
70-
originalFunctions.push_back(func.get());
71-
}
72-
// for each illegal import, we must call a legalized stub instead
73-
for (auto* im : originalFunctions) {
74-
if (im->imported() && isIllegal(module->getFunctionType(im->type))) {
75-
auto funcName = makeLegalStubForCalledImport(im, module);
76-
illegalImportsToLegal[im->name] = funcName;
77-
// we need to use the legalized version in the table, as the import from JS
78-
// is legal for JS. Our stub makes it look like a native wasm function.
79-
for (auto& segment : module->table.segments) {
80-
for (auto& name : segment.data) {
81-
if (name == im->name) {
82-
name = funcName;
83-
}
66+
// Avoid iterator invalidation later.
67+
std::vector<Function*> originalFunctions;
68+
for (auto& func : module->functions) {
69+
originalFunctions.push_back(func.get());
70+
}
71+
// for each illegal import, we must call a legalized stub instead
72+
for (auto* im : originalFunctions) {
73+
if (im->imported() && isIllegal(module->getFunctionType(im->type))
74+
&& shouldBeLegalized(im)) {
75+
auto funcName = makeLegalStubForCalledImport(im, module);
76+
illegalImportsToLegal[im->name] = funcName;
77+
// we need to use the legalized version in the table, as the import from JS
78+
// is legal for JS. Our stub makes it look like a native wasm function.
79+
for (auto& segment : module->table.segments) {
80+
for (auto& name : segment.data) {
81+
if (name == im->name) {
82+
name = funcName;
8483
}
8584
}
8685
}
8786
}
88-
if (illegalImportsToLegal.size() > 0) {
89-
for (auto& pair : illegalImportsToLegal) {
90-
module->removeFunction(pair.first);
91-
}
87+
}
88+
if (illegalImportsToLegal.size() > 0) {
89+
for (auto& pair : illegalImportsToLegal) {
90+
module->removeFunction(pair.first);
91+
}
9292

93-
// fix up imports: call_import of an illegal must be turned to a call of a legal
93+
// fix up imports: call_import of an illegal must be turned to a call of a legal
9494

95-
struct FixImports : public WalkerPass<PostWalker<FixImports>> {
96-
bool isFunctionParallel() override { return true; }
95+
struct FixImports : public WalkerPass<PostWalker<FixImports>> {
96+
bool isFunctionParallel() override { return true; }
9797

98-
Pass* create() override { return new FixImports(illegalImportsToLegal); }
98+
Pass* create() override { return new FixImports(illegalImportsToLegal); }
9999

100-
std::map<Name, Name>* illegalImportsToLegal;
100+
std::map<Name, Name>* illegalImportsToLegal;
101101

102-
FixImports(std::map<Name, Name>* illegalImportsToLegal) : illegalImportsToLegal(illegalImportsToLegal) {}
102+
FixImports(std::map<Name, Name>* illegalImportsToLegal) : illegalImportsToLegal(illegalImportsToLegal) {}
103103

104-
void visitCall(Call* curr) {
105-
auto iter = illegalImportsToLegal->find(curr->target);
106-
if (iter == illegalImportsToLegal->end()) return;
104+
void visitCall(Call* curr) {
105+
auto iter = illegalImportsToLegal->find(curr->target);
106+
if (iter == illegalImportsToLegal->end()) return;
107107

108-
if (iter->second == getFunction()->name) return; // inside the stub function itself, is the one safe place to do the call
109-
replaceCurrent(Builder(*getModule()).makeCall(iter->second, curr->operands, curr->type));
110-
}
111-
};
108+
if (iter->second == getFunction()->name) return; // inside the stub function itself, is the one safe place to do the call
109+
replaceCurrent(Builder(*getModule()).makeCall(iter->second, curr->operands, curr->type));
110+
}
111+
};
112112

113-
PassRunner passRunner(module);
114-
passRunner.setIsNested(true);
115-
passRunner.add<FixImports>(&illegalImportsToLegal);
116-
passRunner.run();
117-
}
113+
PassRunner passRunner(module);
114+
passRunner.setIsNested(true);
115+
passRunner.add<FixImports>(&illegalImportsToLegal);
116+
passRunner.run();
118117
}
119118
}
120119

@@ -131,12 +130,20 @@ struct LegalizeJSInterface : public Pass {
131130
return false;
132131
}
133132

133+
// Check if an export should be legalized.
134134
bool shouldBeLegalized(Export* ex, Function* func) {
135135
if (full) return true;
136136
// We are doing minimal legalization - just what JS needs.
137137
return ex->name.startsWith("dynCall_");
138138
}
139139

140+
// Check if an import should be legalized.
141+
bool shouldBeLegalized(Function* im) {
142+
if (full) return true;
143+
// We are doing minimal legalization - just what JS needs.
144+
return im->module == ENV && im->base.startsWith("invoke_");
145+
}
146+
140147
// JS calls the export, so it must call a legal stub that calls the actual wasm function
141148
Name makeLegalStub(Function* func, Module* module) {
142149
Builder builder(*module);

‎test/passes/legalize-js-interface-minimally.txt‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,29 @@
11
(module
22
(type $FUNCSIG$j (func (result i64)))
3+
(type $FUNCSIG$vj (func (param i64)))
34
(type $FUNCSIG$vi (func (param i32)))
5+
(type $legaltype$invoke_vj (func (param i32 i32)))
46
(import "env" "imported" (func $imported (result i64)))
57
(import "env" "setTempRet0" (func $setTempRet0 (param i32)))
8+
(import "env" "invoke_vj" (func $legalimport$invoke_vj (param i32 i32)))
69
(export "func" (func $func))
710
(export "dynCall_foo" (func $legalstub$dyn))
8-
(func $func (; 2 ;) (type $FUNCSIG$j) (result i64)
11+
(func $func (; 3 ;) (type $FUNCSIG$j) (result i64)
912
(drop
1013
(call $imported)
1114
)
15+
(call $legalfunc$invoke_vj
16+
(i64.const 0)
17+
)
1218
(unreachable)
1319
)
14-
(func $dyn (; 3 ;) (type $FUNCSIG$j) (result i64)
20+
(func $dyn (; 4 ;) (type $FUNCSIG$j) (result i64)
1521
(drop
1622
(call $imported)
1723
)
1824
(unreachable)
1925
)
20-
(func $legalstub$dyn (; 4 ;) (result i32)
26+
(func $legalstub$dyn (; 5 ;) (result i32)
2127
(local $0 i64)
2228
(local.set $0
2329
(call $dyn)
@@ -34,6 +40,19 @@
3440
(local.get $0)
3541
)
3642
)
43+
(func $legalfunc$invoke_vj (; 6 ;) (param $0 i64)
44+
(call $legalimport$invoke_vj
45+
(i32.wrap_i64
46+
(local.get $0)
47+
)
48+
(i32.wrap_i64
49+
(i64.shr_u
50+
(local.get $0)
51+
(i64.const 32)
52+
)
53+
)
54+
)
55+
)
3756
)
3857
(module
3958
)

‎test/passes/legalize-js-interface-minimally.wast‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
(module
22
(import "env" "imported" (func $imported (result i64)))
3+
(import "env" "invoke_vj" (func $invoke_vj (param i64)))
34
(export "func" (func $func))
45
(export "dynCall_foo" (func $dyn))
56
(func $func (result i64)
67
(drop (call $imported))
8+
(call $invoke_vj (i64.const 0))
79
(unreachable)
810
)
911
(func $dyn (result i64)

0 commit comments

Comments
 (0)