Skip to content

Commit f70c5cf

Browse files
kripkenkripken
authored
[JSInterop] Be aware of configureAll (#8046)
This magic import refers to functions, which we must handle, and not modify their signature or contents. This fixes things optimally for RUME, but for all the passes that look at public/private heap types, it just marks those functions as public, which inhibits optimizations. This unblocks work on JS Interop for now (and maybe those optimizations are not necessary, but if we do want them, it would mean changes to each such pass). Spec: https://github.com/WebAssembly/custom-descriptors/blob/main/proposals/custom-descriptors/Overview.md#configuration-api
1 parent 9c67021 commit f70c5cf

7 files changed

Lines changed: 419 additions & 7 deletions

‎src/ir/intrinsics.cpp‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
namespace wasm {
2121

2222
static Name BinaryenIntrinsicsModule("binaryen-intrinsics"),
23-
CallWithoutEffects("call.without.effects");
23+
CallWithoutEffects("call.without.effects"),
24+
JSPrototypesModule("wasm:js-prototypes"), ConfigureAll("configureAll");
2425

2526
bool Intrinsics::isCallWithoutEffects(Function* func) {
2627
if (func->module != BinaryenIntrinsicsModule) {
@@ -45,4 +46,58 @@ Call* Intrinsics::isCallWithoutEffects(Expression* curr) {
4546
return nullptr;
4647
}
4748

49+
bool Intrinsics::isConfigureAll(Function* func) {
50+
return func->module == JSPrototypesModule && func->base == ConfigureAll;
51+
}
52+
53+
Call* Intrinsics::isConfigureAll(Expression* curr) {
54+
if (auto* call = curr->dynCast<Call>()) {
55+
if (auto* func = module.getFunctionOrNull(call->target)) {
56+
if (isConfigureAll(func)) {
57+
return call;
58+
}
59+
}
60+
}
61+
return nullptr;
62+
}
63+
64+
std::vector<Name> Intrinsics::getConfigureAllFunctions(Call* call) {
65+
assert(isConfigureAll(call));
66+
67+
auto error = [&](const char* msg) {
68+
Fatal() << "Invalid configureAll( " << msg << "): " << *call;
69+
};
70+
71+
// The second operand is an array of signature-called function refs.
72+
auto& operands = call->operands;
73+
if (operands.size() <= 2) {
74+
error("insufficient operands");
75+
}
76+
auto* arrayNew = operands[1]->dynCast<ArrayNewElem>();
77+
if (!arrayNew) {
78+
error("not array.new_elem");
79+
}
80+
auto start = arrayNew->offset->dynCast<Const>();
81+
if (!start || start->value.geti32() != 0) {
82+
error("start != 0");
83+
}
84+
auto size = arrayNew->size->dynCast<Const>();
85+
if (!size) {
86+
error("size not const");
87+
}
88+
auto* seg = module.getElementSegment(arrayNew->segment);
89+
if (seg->data.size() != size->value.getUnsigned()) {
90+
error("wrong seg size");
91+
}
92+
std::vector<Name> ret;
93+
for (auto* curr : seg->data) {
94+
if (auto* refFunc = curr->dynCast<RefFunc>()) {
95+
ret.push_back(refFunc->func);
96+
} else {
97+
error("non-function ref");
98+
}
99+
}
100+
return ret;
101+
}
102+
48103
} // namespace wasm

‎src/ir/intrinsics.h‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
#include "wasm-traversal.h"
2222

2323
//
24-
// See the README.md for background on intrinsic functions.
24+
// Intrinsics include Binaryen intrinsics, and Wasm spec builtins. See the
25+
// README.md for more.
2526
//
2627
// Intrinsics can be recognized by Intrinsics::isFoo() methods, that check if a
2728
// function is a particular intrinsic, or if a call to a function is so. The
@@ -37,8 +38,7 @@ class Intrinsics {
3738
public:
3839
Intrinsics(Module& module) : module(module) {}
3940

40-
//
41-
// Check if an instruction is the call.without.effects intrinsic.
41+
// Check if an instruction is the Binaryen call.without.effects intrinsic.
4242
//
4343
// (import "binaryen-intrinsics" "call.without.effects"
4444
// (func (..params..) (param $target funcref) (..results..)))
@@ -85,9 +85,30 @@ class Intrinsics {
8585
//
8686
// Later passes will then turn that into a direct call and further optimize
8787
// things.
88-
//
8988
bool isCallWithoutEffects(Function* func);
9089
Call* isCallWithoutEffects(Expression* curr);
90+
91+
// Check if an instruction is the wasm/JS interop configureAll builtin. Such
92+
// calls have a second parameter which is an array of function references,
93+
// which must be assumed to be "signature-called": Called from outside the
94+
// module, using that signature. "Signature-called" is less powerful than the
95+
// reference to the function generally escaping, as we do not care about the
96+
// heap type of the function (no call_refs or casts will happen), all we know
97+
// is that a JS-style call of the methods can occur (and only the signature
98+
// matters then).
99+
bool isConfigureAll(Function* func);
100+
Call* isConfigureAll(Expression* curr);
101+
// Given a configureAll, return all the functions it refers to. We error if it
102+
// is not in the canonical form
103+
//
104+
// (call $configureAll
105+
// ..arg0..
106+
// (array.new_elem $seg (0) (N)
107+
// ..
108+
// )
109+
//
110+
// where the segment $seg is of size N.
111+
std::vector<Name> getConfigureAllFunctions(Call* call);
91112
};
92113

93114
} // namespace wasm

‎src/ir/module-utils.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include "module-utils.h"
18+
#include "ir/find_all.h"
1819
#include "ir/intrinsics.h"
1920
#include "ir/manipulation.h"
2021
#include "ir/metadata.h"
@@ -692,6 +693,26 @@ std::vector<HeapType> getPublicHeapTypes(Module& wasm) {
692693
WASM_UNREACHABLE("unexpected export kind");
693694
}
694695

696+
// ConfigureAll in a start function makes its functions callable. They are
697+
// only signature-called, so the heap type does not need to be public - nor
698+
// types referred to - but for now we mark them as public to avoid breakage in
699+
// several passes.
700+
// TODO Specific fixes in those passes could replace this, and allow better
701+
// optimization.
702+
if (wasm.start) {
703+
auto* start = wasm.getFunction(wasm.start);
704+
if (!start->imported()) {
705+
Intrinsics intrinsics(wasm);
706+
for (auto* call : FindAll<Call>(start->body).list) {
707+
if (intrinsics.isConfigureAll(call)) {
708+
for (auto func : intrinsics.getConfigureAllFunctions(call)) {
709+
notePublic(wasm.getFunction(func)->type.getHeapType());
710+
}
711+
}
712+
}
713+
}
714+
}
715+
695716
// Ignorable public types are public.
696717
for (auto type : getIgnorablePublicTypes()) {
697718
notePublic(type);

‎src/passes/RemoveUnusedModuleElements.cpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ struct Noter : public PostWalker<Noter, UnifiedExpressionVisitor<Noter>> {
136136
void visitCall(Call* curr) {
137137
use({ModuleElementKind::Function, curr->target});
138138

139-
if (Intrinsics(*getModule()).isCallWithoutEffects(curr)) {
139+
Intrinsics intrinsics(*getModule());
140+
if (intrinsics.isCallWithoutEffects(curr)) {
140141
// A call-without-effects receives a function reference and calls it, the
141142
// same as a CallRef. When we have a flag for non-closed-world, we should
142143
// handle this automatically by the reference flowing out to an import,
@@ -157,6 +158,12 @@ struct Noter : public PostWalker<Noter, UnifiedExpressionVisitor<Noter>> {
157158
callRef.target = target;
158159
visitCallRef(&callRef);
159160
}
161+
} else if (intrinsics.isConfigureAll(curr)) {
162+
// Every function that configureAll refers to is signature-called. Mark
163+
// them all as called, as JS can call them.
164+
for (auto func : intrinsics.getConfigureAllFunctions(curr)) {
165+
use({ModuleElementKind::Function, func});
166+
}
160167
}
161168
}
162169

‎test/lit/passes/remove-unused-module-elements-configureAll.wast‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
3+
;; RUN: foreach %s %t wasm-opt --remove-unused-module-elements --closed-world -all -S -o - | filecheck %s
4+
;; RUN: foreach %s %t wasm-opt --remove-unused-module-elements -all -S -o - | filecheck %s --check-prefix OPEN_WORLD
5+
6+
;; Test that configureAll is respected: referred functions are not optimized
7+
;; away or emptied out. This is so even in closed world.
8+
9+
(module
10+
;; CHECK: (type $externs (array (mut externref)))
11+
;; OPEN_WORLD: (type $externs (array (mut externref)))
12+
(type $externs (array (mut externref)))
13+
14+
;; CHECK: (type $funcs (array (mut funcref)))
15+
;; OPEN_WORLD: (type $funcs (array (mut funcref)))
16+
(type $funcs (array (mut funcref)))
17+
18+
;; CHECK: (type $bytes (array (mut i8)))
19+
;; OPEN_WORLD: (type $bytes (array (mut i8)))
20+
(type $bytes (array (mut i8)))
21+
22+
;; CHECK: (type $configureAll (func (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
23+
;; OPEN_WORLD: (type $configureAll (func (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
24+
(type $configureAll (func (param (ref null $externs)) (param (ref null $funcs)) (param (ref null $bytes)) (param externref)))
25+
26+
;; CHECK: (type $4 (func))
27+
28+
;; CHECK: (type $5 (func (result i32)))
29+
30+
;; CHECK: (type $6 (func (param i32) (result i32)))
31+
32+
;; CHECK: (import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll) (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
33+
;; OPEN_WORLD: (type $4 (func))
34+
35+
;; OPEN_WORLD: (type $5 (func (result i32)))
36+
37+
;; OPEN_WORLD: (type $6 (func (param i32) (result i32)))
38+
39+
;; OPEN_WORLD: (import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll) (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
40+
(import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll)))
41+
42+
;; CHECK: (data $bytes "12345678")
43+
44+
;; CHECK: (elem $externs externref (item (ref.null noextern)))
45+
;; OPEN_WORLD: (data $bytes "12345678")
46+
47+
;; OPEN_WORLD: (elem $externs externref (item (ref.null noextern)))
48+
(elem $externs externref
49+
(ref.null extern)
50+
)
51+
52+
;; CHECK: (elem $funcs func $foo $bar)
53+
;; OPEN_WORLD: (elem $funcs func $foo $bar)
54+
(elem $funcs funcref
55+
(ref.func $foo)
56+
(ref.func $bar)
57+
)
58+
59+
(data $bytes "12345678")
60+
61+
;; CHECK: (start $start)
62+
;; OPEN_WORLD: (start $start)
63+
(start $start)
64+
65+
;; CHECK: (func $start (type $4)
66+
;; CHECK-NEXT: (call $configureAll
67+
;; CHECK-NEXT: (array.new_elem $externs $externs
68+
;; CHECK-NEXT: (i32.const 0)
69+
;; CHECK-NEXT: (i32.const 1)
70+
;; CHECK-NEXT: )
71+
;; CHECK-NEXT: (array.new_elem $funcs $funcs
72+
;; CHECK-NEXT: (i32.const 0)
73+
;; CHECK-NEXT: (i32.const 2)
74+
;; CHECK-NEXT: )
75+
;; CHECK-NEXT: (array.new_data $bytes $bytes
76+
;; CHECK-NEXT: (i32.const 0)
77+
;; CHECK-NEXT: (i32.const 8)
78+
;; CHECK-NEXT: )
79+
;; CHECK-NEXT: (ref.null noextern)
80+
;; CHECK-NEXT: )
81+
;; CHECK-NEXT: )
82+
;; OPEN_WORLD: (func $start (type $4)
83+
;; OPEN_WORLD-NEXT: (call $configureAll
84+
;; OPEN_WORLD-NEXT: (array.new_elem $externs $externs
85+
;; OPEN_WORLD-NEXT: (i32.const 0)
86+
;; OPEN_WORLD-NEXT: (i32.const 1)
87+
;; OPEN_WORLD-NEXT: )
88+
;; OPEN_WORLD-NEXT: (array.new_elem $funcs $funcs
89+
;; OPEN_WORLD-NEXT: (i32.const 0)
90+
;; OPEN_WORLD-NEXT: (i32.const 2)
91+
;; OPEN_WORLD-NEXT: )
92+
;; OPEN_WORLD-NEXT: (array.new_data $bytes $bytes
93+
;; OPEN_WORLD-NEXT: (i32.const 0)
94+
;; OPEN_WORLD-NEXT: (i32.const 8)
95+
;; OPEN_WORLD-NEXT: )
96+
;; OPEN_WORLD-NEXT: (ref.null noextern)
97+
;; OPEN_WORLD-NEXT: )
98+
;; OPEN_WORLD-NEXT: )
99+
(func $start
100+
(call $configureAll
101+
(array.new_elem $externs $externs
102+
(i32.const 0) (i32.const 1))
103+
(array.new_elem $funcs $funcs
104+
(i32.const 0) (i32.const 2))
105+
(array.new_data $bytes $bytes
106+
(i32.const 0) (i32.const 8))
107+
(ref.null extern)
108+
)
109+
)
110+
111+
;; CHECK: (func $foo (type $5) (result i32)
112+
;; CHECK-NEXT: (i32.const 42)
113+
;; CHECK-NEXT: )
114+
;; OPEN_WORLD: (func $foo (type $5) (result i32)
115+
;; OPEN_WORLD-NEXT: (i32.const 42)
116+
;; OPEN_WORLD-NEXT: )
117+
(func $foo (result i32)
118+
;; configureAll keeps this from being modified.
119+
(i32.const 42)
120+
)
121+
122+
;; CHECK: (func $bar (type $6) (param $x i32) (result i32)
123+
;; CHECK-NEXT: (local.get $x)
124+
;; CHECK-NEXT: )
125+
;; OPEN_WORLD: (func $bar (type $6) (param $x i32) (result i32)
126+
;; OPEN_WORLD-NEXT: (local.get $x)
127+
;; OPEN_WORLD-NEXT: )
128+
(func $bar (param $x i32) (result i32)
129+
;; This too.
130+
(local.get $x)
131+
)
132+
133+
(func $unconfigured (result i32)
134+
;; This is not referred to by configureAll, and can be removed.
135+
(i32.const 1337)
136+
)
137+
)

0 commit comments

Comments
 (0)