Skip to content

Commit 883d14d

Browse files
kripkenkripken
authored
Wasm2js memory fixes (WebAssembly#2003)
* I64ToI32Lowering - don't assume address 0 is a hardcoded location for scratch memory. Import __tempMemory__ for that. * RemoveNonJSOps - also use __tempMemory__. Oddly here the address was a hardcoded 1024 (perhaps where the rust program put a static global?). * Support imported ints in wasm2js, coercing them as needed. * Add "env" import support in the tests, since now we emit imports from there. * Make wasm2js tests split out multi-module tests using split_wast which is more robust and avoids emitting multiple outputs in one file (which makes no sense for ES6 modules)
1 parent 53badfb commit 883d14d

25 files changed

Lines changed: 9017 additions & 489 deletions

‎scripts/test/env.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const __tempMemory__ = 0;

‎scripts/test/node-esm-loader.mjs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,16 @@ export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) {
1616
format: 'builtin'
1717
};
1818
}
19-
// Resolve the 'spectest' module to our special module which has some builtins
20-
if (specifier == 'spectest') {
21-
const resolved = new URL('./scripts/test/spectest.js', parentModuleURL);
19+
// Resolve the 'spectest' and 'env' modules to our custom implementations of
20+
// various builtins.
21+
if (specifier == 'spectest' || specifier == 'env') {
22+
const resolved = new URL('./scripts/test/' + specifier + '.js', parentModuleURL);
2223
return {
2324
url: resolved.href,
2425
format: 'esm'
2526
};
2627
}
28+
2729
const resolved = new URL(specifier, parentModuleURL);
2830
return {
2931
url: resolved.href,

‎scripts/test/wasm2js.py‎

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
import os
1818

19-
from support import run_command
19+
from support import run_command, split_wast
2020
from shared import (
2121
WASM2JS, MOZJS, NODEJS, fail_if_not_identical, options, tests,
2222
fail_if_not_identical_to_file
@@ -51,36 +51,46 @@ def test_wasm2js_output():
5151

5252
print '..', wasm
5353

54-
cmd = WASM2JS + [os.path.join(options.binaryen_test, wasm)]
55-
if 'emscripten' in wasm:
56-
cmd += ['--emscripten']
57-
out = run_command(cmd)
58-
fail_if_not_identical_to_file(out, expected_file)
54+
t = os.path.join(options.binaryen_test, wasm)
5955

60-
if not NODEJS and not MOZJS:
61-
print 'No JS interpreters. Skipping spec tests.'
62-
continue
56+
all_out = []
6357

64-
open('a.2asm.mjs', 'w').write(out)
58+
for module, asserts in split_wast(t):
59+
with open('split.wast', 'w') as o:
60+
o.write(module + '\n'.join(asserts))
6561

66-
cmd += ['--allow-asserts']
67-
out = run_command(cmd)
62+
cmd = WASM2JS + ['split.wast']
63+
if 'emscripten' in wasm:
64+
cmd += ['--emscripten']
65+
out = run_command(cmd)
66+
all_out.append(out)
6867

69-
open('a.2asm.asserts.mjs', 'w').write(out)
68+
if not NODEJS and not MOZJS:
69+
print 'No JS interpreters. Skipping spec tests.'
70+
continue
7071

71-
# verify asm.js is valid js, note that we're using --experimental-modules
72-
# to enable ESM syntax and we're also passing a custom loader to handle the
73-
# `spectest` module in our tests.
74-
if NODEJS:
75-
node = [NODEJS, '--experimental-modules', '--loader', './scripts/test/node-esm-loader.mjs']
76-
cmd = node[:]
77-
cmd.append('a.2asm.mjs')
72+
open('a.2asm.mjs', 'w').write(out)
73+
74+
cmd += ['--allow-asserts']
7875
out = run_command(cmd)
79-
fail_if_not_identical(out, '')
80-
cmd = node[:]
81-
cmd.append('a.2asm.asserts.mjs')
82-
out = run_command(cmd, expected_err='', err_ignore='The ESM module loader is experimental')
83-
fail_if_not_identical(out, '')
76+
77+
open('a.2asm.asserts.mjs', 'w').write(out)
78+
79+
# verify asm.js is valid js, note that we're using --experimental-modules
80+
# to enable ESM syntax and we're also passing a custom loader to handle the
81+
# `spectest` and `env` modules in our tests.
82+
if NODEJS:
83+
node = [NODEJS, '--experimental-modules', '--loader', './scripts/test/node-esm-loader.mjs']
84+
cmd = node[:]
85+
cmd.append('a.2asm.mjs')
86+
out = run_command(cmd)
87+
fail_if_not_identical(out, '')
88+
cmd = node[:]
89+
cmd.append('a.2asm.asserts.mjs')
90+
out = run_command(cmd, expected_err='', err_ignore='The ESM module loader is experimental')
91+
fail_if_not_identical(out, '')
92+
93+
fail_if_not_identical_to_file(''.join(all_out), expected_file)
8494

8595

8696
def test_asserts_output():
@@ -130,12 +140,22 @@ def update_wasm2js_tests():
130140

131141
print '..', wasm
132142

133-
cmd = WASM2JS + [os.path.join('test', wasm)]
134-
if 'emscripten' in wasm:
135-
cmd += ['--emscripten']
136-
out = run_command(cmd)
143+
t = os.path.join(options.binaryen_test, wasm)
144+
145+
all_out = []
146+
147+
for module, asserts in split_wast(t):
148+
with open('split.wast', 'w') as o:
149+
o.write(module + '\n'.join(asserts))
150+
151+
cmd = WASM2JS + ['split.wast']
152+
if 'emscripten' in wasm:
153+
cmd += ['--emscripten']
154+
out = run_command(cmd)
155+
all_out.append(out)
156+
137157
with open(expected_file, 'w') as o:
138-
o.write(out)
158+
o.write(''.join(all_out))
139159

140160
for wasm in assert_tests:
141161
print '..', wasm

‎src/passes/I64ToI32Lowering.cpp‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -600,35 +600,51 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
600600
// our f64 through memory at address 0
601601
TempVar highBits = getTemp();
602602
Block *result = builder->blockify(
603-
builder->makeStore(8, 0, 8, builder->makeConst(Literal(int32_t(0))), curr->value, f64),
603+
builder->makeStore(8, 0, 8, makeGetTempMemory(), curr->value, f64),
604604
builder->makeSetLocal(
605605
highBits,
606-
builder->makeLoad(4, true, 4, 4, builder->makeConst(Literal(int32_t(0))), i32)
606+
builder->makeLoad(4, true, 4, 4, makeGetTempMemory(), i32)
607607
),
608-
builder->makeLoad(4, true, 0, 4, builder->makeConst(Literal(int32_t(0))), i32)
608+
builder->makeLoad(4, true, 0, 4, makeGetTempMemory(), i32)
609609
);
610610
setOutParam(result, std::move(highBits));
611611
replaceCurrent(result);
612-
ensureMinimalMemory();
612+
MemoryUtils::ensureExists(getModule()->memory);
613+
ensureTempMemoryGlobal();
613614
}
614615

615616
void lowerReinterpretInt64(Unary* curr) {
616617
// Assume that the wasm file assumes the address 0 is invalid and roundtrip
617618
// our i64 through memory at address 0
618619
TempVar highBits = fetchOutParam(curr->value);
619620
Block *result = builder->blockify(
620-
builder->makeStore(4, 0, 4, builder->makeConst(Literal(int32_t(0))), curr->value, i32),
621-
builder->makeStore(4, 4, 4, builder->makeConst(Literal(int32_t(0))), builder->makeGetLocal(highBits, i32), i32),
622-
builder->makeLoad(8, true, 0, 8, builder->makeConst(Literal(int32_t(0))), f64)
621+
builder->makeStore(4, 0, 4, makeGetTempMemory(), curr->value, i32),
622+
builder->makeStore(4, 4, 4, makeGetTempMemory(), builder->makeGetLocal(highBits, i32), i32),
623+
builder->makeLoad(8, true, 0, 8, makeGetTempMemory(), f64)
623624
);
624625
replaceCurrent(result);
625-
ensureMinimalMemory();
626+
MemoryUtils::ensureExists(getModule()->memory);
627+
ensureTempMemoryGlobal();
626628
}
627629

628-
// Ensure memory exists with a minimal size, enough for round-tripping operations on
629-
// address 0, which we need for reinterpret operations.
630-
void ensureMinimalMemory() {
631-
MemoryUtils::ensureExists(getModule()->memory);
630+
Name tempMemory = "__tempMemory__";
631+
632+
void ensureTempMemoryGlobal() {
633+
// Ensure the existence of an imported global, __tempMemory__, which points to 8
634+
// bytes of scratch memory we can use for roundtrip purposes.
635+
if (!getModule()->getGlobalOrNull(tempMemory)) {
636+
auto global = make_unique<Global>();
637+
global->name = tempMemory;
638+
global->type = i32;
639+
global->mutable_ = false;
640+
global->module = ENV;
641+
global->base = tempMemory;
642+
getModule()->addGlobal(global.release());
643+
}
644+
}
645+
646+
Expression* makeGetTempMemory() {
647+
return builder->makeGetGlobal(tempMemory, i32);
632648
}
633649

634650
void lowerTruncFloatToInt(Unary *curr) {

‎src/passes/RemoveNonJSOps.cpp‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ namespace wasm {
4343
struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
4444
std::unique_ptr<Builder> builder;
4545
std::unordered_set<Name> neededIntrinsics;
46+
std::set<std::pair<Name, Type>> neededImportedGlobals;
4647

4748
bool isFunctionParallel() override { return false; }
4849

@@ -101,6 +102,21 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
101102

102103
// Intrinsics may use memory, so ensure the module has one.
103104
MemoryUtils::ensureExists(module->memory);
105+
106+
// Add missing globals
107+
for (auto& pair : neededImportedGlobals) {
108+
auto name = pair.first;
109+
auto type = pair.second;
110+
if (!getModule()->getGlobalOrNull(name)) {
111+
auto global = make_unique<Global>();
112+
global->name = name;
113+
global->type = type;
114+
global->mutable_ = false;
115+
global->module = ENV;
116+
global->base = name;
117+
module->addGlobal(global.release());
118+
}
119+
}
104120
}
105121

106122
void addNeededFunctions(Module &m, Name name, std::set<Name> &needed) {
@@ -121,7 +137,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
121137
PostWalker<RemoveNonJSOpsPass>::doWalkFunction(func);
122138
}
123139

124-
void visitLoad(Load *curr) {
140+
void visitLoad(Load* curr) {
125141
if (curr->align == 0 || curr->align >= curr->bytes) {
126142
return;
127143
}
@@ -143,7 +159,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
143159
}
144160
}
145161

146-
void visitStore(Store *curr) {
162+
void visitStore(Store* curr) {
147163
if (curr->align == 0 || curr->align >= curr->bytes) {
148164
return;
149165
}
@@ -165,7 +181,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
165181
}
166182
}
167183

168-
void visitBinary(Binary *curr) {
184+
void visitBinary(Binary* curr) {
169185
Name name;
170186
switch (curr->op) {
171187
case CopySignFloat32:
@@ -207,7 +223,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
207223
replaceCurrent(builder->makeCall(name, {curr->left, curr->right}, curr->type));
208224
}
209225

210-
void rewriteCopysign(Binary *curr) {
226+
void rewriteCopysign(Binary* curr) {
211227
Literal signBit, otherBits;
212228
UnaryOp int2float, float2int;
213229
BinaryOp bitAnd, bitOr;
@@ -259,7 +275,7 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
259275
);
260276
}
261277

262-
void visitUnary(Unary *curr) {
278+
void visitUnary(Unary* curr) {
263279
Name functionCall;
264280
switch (curr->op) {
265281
case NearestFloat32:
@@ -295,9 +311,13 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
295311
neededIntrinsics.insert(functionCall);
296312
replaceCurrent(builder->makeCall(functionCall, {curr->value}, curr->type));
297313
}
314+
315+
void visitGetGlobal(GetGlobal* curr) {
316+
neededImportedGlobals.insert(std::make_pair(curr->name, curr->type));
317+
}
298318
};
299319

300-
Pass *createRemoveNonJSOpsPass() {
320+
Pass* createRemoveNonJSOpsPass() {
301321
return new RemoveNonJSOpsPass();
302322
}
303323

‎src/passes/wasm-intrinsics.wast‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
;; these pretty early so they can continue to be optimized by further passes
66
;; (aka inlining and whatnot)
77
;;
8+
;; LOCAL MODS done by hand afterwards:
9+
;; * Remove hardcoded address 1024 (apparently a free memory location rustc
10+
;; thinks is ok to use?); add a global __tempMemory__ which is used for that
11+
;; purpose.
12+
;; * Fix function type of __wasm_ctz_i64, which was wrong somehow,
13+
;; i32, i32 => i32 instead of i64 => i64
14+
;;
815
;; [1]: https://gist.github.com/alexcrichton/e7ea67bcdd17ce4b6254e66f77165690
916

1017
(module
@@ -13,7 +20,9 @@
1320
(type $2 (func (param f64) (result f64)))
1421
(type $3 (func (param i32) (result i32)))
1522
(type $4 (func (param i32 i32) (result i32)))
23+
(type $5 (func (param i64) (result i64)))
1624
(import "env" "memory" (memory $0 17))
25+
(import "env" "__tempMemory__" (global $__tempMemory__ i32))
1726
(export "__wasm_i64_sdiv" (func $__wasm_i64_sdiv))
1827
(export "__wasm_i64_udiv" (func $__wasm_i64_udiv))
1928
(export "__wasm_i64_srem" (func $__wasm_i64_srem))
@@ -128,7 +137,7 @@
128137
)
129138
)
130139
(i64.load
131-
(i32.const 1024)
140+
(global.get $__tempMemory__)
132141
)
133142
)
134143
;; lowering of the i64.mul instruction, return $var0 * $var$1
@@ -192,7 +201,7 @@
192201
(i32.const 32)
193202
)
194203
;; lowering of the i64.ctz instruction, counting the number of zeros in $var$0
195-
(func $__wasm_ctz_i64 (; 8 ;) (type $4) (param $var$0 i64) (result i64)
204+
(func $__wasm_ctz_i64 (; 8 ;) (type $5) (param $var$0 i64) (result i64)
196205
(if
197206
(i32.eqz
198207
(i64.eqz
@@ -571,7 +580,7 @@
571580
)
572581
)
573582
(i64.store
574-
(i32.const 1024)
583+
(global.get $__tempMemory__)
575584
(i64.extend_i32_u
576585
(i32.sub
577586
(local.tee $var$2
@@ -633,7 +642,7 @@
633642
)
634643
)
635644
(i64.store
636-
(i32.const 1024)
645+
(global.get $__tempMemory__)
637646
(i64.or
638647
(i64.shl
639648
(i64.extend_i32_u
@@ -714,7 +723,7 @@
714723
(br $label$3)
715724
)
716725
(i64.store
717-
(i32.const 1024)
726+
(global.get $__tempMemory__)
718727
(i64.shl
719728
(i64.extend_i32_u
720729
(i32.sub
@@ -757,7 +766,7 @@
757766
(br $label$2)
758767
)
759768
(i64.store
760-
(i32.const 1024)
769+
(global.get $__tempMemory__)
761770
(i64.extend_i32_u
762771
(i32.and
763772
(local.get $var$4)
@@ -889,7 +898,7 @@
889898
)
890899
)
891900
(i64.store
892-
(i32.const 1024)
901+
(global.get $__tempMemory__)
893902
(local.get $var$5)
894903
)
895904
(return
@@ -903,7 +912,7 @@
903912
)
904913
)
905914
(i64.store
906-
(i32.const 1024)
915+
(global.get $__tempMemory__)
907916
(local.get $var$0)
908917
)
909918
(local.set $var$0

0 commit comments

Comments
 (0)