Skip to content

Commit 706b3f6

Browse files
alexcrichtonkripken
authored andcommitted
wasm2asm: Fix and enable a large number of spec tests (#1558)
* Import `abort` from the environment * Add passing spec tests * Bind the abort function * wasm2asm: Fix name collisions Currently function names and local names can collide in namespaces, causing buggy results when a function intends to call another function but ends up using a local value as the target! This fix was required to enable the `fac` spec test * wasm2asm: Get multiple modules in one file working The spec tests seem to have multiple modules defined in some tests and the invocations all use the most recently defined module. This commit updates the `--allow-asserts` mode of wasm2asm to work with this mode of tests, enabling us to enable more spec tests for wasm2asm. * wasm2asm: Enable the float_literals spec test This needed to be modified to account for how JS engines don't work with NaN bits the same way, but it's otherwise largely the same test. Additionally it turns out that asm.js doesn't accept either `Infinity` or `NaN` ambient globals so they needed to get imported through the `global` variable rather than defined as literals in code * wasm2asm: Fix function pointer invocations This commit fixes invocations of functions through function pointers as previously the table names on lookup and definition were mismatched. Both tables now go through signature-based namification rather than athe name of the type itself. Overall this enables a slew of spec tests * wasm2asm: Enable the left-to-right spec test There were two small bugs in the order of evaluation of operators with wasm2asm. The `select` instruction would sometimes evaluate the condition first when it was supposed to be last. Similarly a `call_indirect` instruction would evaluate the function pointer first when it was supposed to be evaluated last. The `select` instruction case was a relatively small fix but the one for `call_indirect` was a bit more pessimized to generate some temporaries. Hopefully if this becomes up a problem it can be tightened up. * wasm2asm: Fix signed load promotions of 64-bit ints This commit enables the `endianness` spec test which revealed a bug in 64-bit loads from smaller sizes which were signed. Previously the upper bits of the 64-bit number were all set to zero but the fix was for signed loads to have all the upper bits match the highest bit of the low 32 bits that we load. * wasm2asm: Enable the `stack` spec test Internally the spec test uses a mixture of the s-expression syntax and the wat syntax, so this is copied over into the `wasm2asm` folder after going through `wat2wasm` to ensure it's consistent for binaryen. * wasm2asm: Fix unaligned loads/stores of floats Replace these operations in `RemoveNonJSOps` by using reinterpretation to translate floats to integers and then use the existing code for unaligned loads/stores of integers. * wasm2asm: Fix a tricky grow_memory codegen bug This commit fixes a tricky codegen bug found in the `grow_memory` instruction. Specifically if you stored the result of `grow_memory` immediately into memory it would look like: HEAP32[..] = __wasm_grow_memory(..); Here though it looks like JS evaluates the destination *before* the grow function is called, but the grow function will invalidate the destination! Furthermore this is actually generalizable to all function calls: HEAP32[..] = foo(..); Because any function could transitively call `grow_memory`. This commit fixes the issue by ensuring that store instructions are always considered statements, unconditionally evaluating the value into a temporary and then storing that into the destination. While a bit of a pessmimization for now it should hopefully fix the bug here. * wasm2asm: Handle offsets in tables This commit fixes initializing tables whose elements have an initial offset. This should hopefully help fix some more Rust code which has all function pointers offset by default! * Update tests * Tweak * location on types * Rename entries of NameScope and document fromName * Comment on lowercase names * Update compiled JS * Update js test output expectation * Rename NameScope::Global to NameScope::Top * Switch to `enum class` * Switch to `Fatal()` * Add TODO for when asm.js is no longer generated
1 parent 91b90b7 commit 706b3f6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+61891
-930
lines changed

bin/binaryen.js

Lines changed: 20 additions & 20 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin/wasm.js

Lines changed: 21 additions & 21 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/emscripten-optimizer/simple_ast.h

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -836,17 +836,25 @@ struct JSPrinter {
836836
}
837837

838838
static char* numToString(double d, bool finalize=true) {
839+
// If this number is NaN or infinite then things are a bit tricky. In JS we
840+
// want to eventually use `NaN` and/or `Infinity`, but neither of those
841+
// identifiers are valid in asm.js. Instead we have to explicitly import
842+
// `NaN` and `Infinity` from the global environment, and those names are
843+
// bound locally in an asm function as `nan` and `infinity`.
844+
//
845+
// TODO: the JS names of `NaN` and `Infinity` should be used once literal
846+
// asm.js code isn't generated any more
839847
if (std::isnan(d)) {
840848
if (std::signbit(d)) {
841-
return (char*) "-NaN";
849+
return (char*) "-nan";
842850
} else {
843-
return (char*) "NaN";
851+
return (char*) "nan";
844852
}
845853
} else if (!std::isfinite(d)) {
846854
if (std::signbit(d)) {
847-
return (char*) "-Infinity";
855+
return (char*) "-infinity";
848856
} else {
849-
return (char*) "Infinity";
857+
return (char*) "infinity";
850858
}
851859
}
852860
bool neg = d < 0;
@@ -1059,8 +1067,8 @@ struct JSPrinter {
10591067
ensure(1); // we temporarily append a 0
10601068
char *curr = buffer + last; // ensure might invalidate
10611069
buffer[used] = 0;
1062-
if (strstr(curr, "Infinity")) return;
1063-
if (strstr(curr, "NaN")) return;
1070+
if (strstr(curr, "infinity")) return;
1071+
if (strstr(curr, "nan")) return;
10641072
if (strchr(curr, '.')) return; // already a decimal point, all good
10651073
char *e = strchr(curr, 'e');
10661074
if (!e) {

src/passes/I64ToI32Lowering.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
449449
void visitLoad(Load* curr) {
450450
if (curr->type != i64) return;
451451
assert(!curr->isAtomic && "atomic load not implemented");
452+
TempVar lowBits = getTemp();
452453
TempVar highBits = getTemp();
453454
TempVar ptrTemp = getTemp();
454455
SetLocal* setPtr = builder->makeSetLocal(ptrTemp, curr->ptr);
@@ -465,6 +466,15 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
465466
i32
466467
)
467468
);
469+
} else if (curr->signed_) {
470+
loadHigh = builder->makeSetLocal(
471+
highBits,
472+
builder->makeBinary(
473+
ShrSInt32,
474+
builder->makeGetLocal(lowBits, i32),
475+
builder->makeConst(Literal(int32_t(31)))
476+
)
477+
);
468478
} else {
469479
loadHigh = builder->makeSetLocal(
470480
highBits,
@@ -475,7 +485,12 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
475485
curr->bytes = std::min(curr->bytes, uint8_t(4));
476486
curr->align = std::min(uint32_t(curr->align), uint32_t(4));
477487
curr->ptr = builder->makeGetLocal(ptrTemp, i32);
478-
Block* result = builder->blockify(setPtr, loadHigh, curr);
488+
Block* result = builder->blockify(
489+
setPtr,
490+
builder->makeSetLocal(lowBits, curr),
491+
loadHigh,
492+
builder->makeGetLocal(lowBits, i32)
493+
);
479494
replaceCurrent(result);
480495
setOutParam(result, std::move(highBits));
481496
}

src/passes/RemoveNonJSOps.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,50 @@ struct RemoveNonJSOpsPass : public WalkerPass<PostWalker<RemoveNonJSOpsPass>> {
113113
PostWalker<RemoveNonJSOpsPass>::doWalkFunction(func);
114114
}
115115

116+
void visitLoad(Load *curr) {
117+
if (curr->align == 0 || curr->align >= curr->bytes) {
118+
return;
119+
}
120+
121+
// Switch unaligned loads of floats to unaligned loads of integers (which we
122+
// can actually implement) and then use reinterpretation to get the float
123+
// back out.
124+
switch (curr->type) {
125+
case f32:
126+
curr->type = i32;
127+
replaceCurrent(builder->makeUnary(ReinterpretInt32, curr));
128+
break;
129+
case f64:
130+
curr->type = i64;
131+
replaceCurrent(builder->makeUnary(ReinterpretInt64, curr));
132+
break;
133+
default:
134+
break;
135+
}
136+
}
137+
138+
void visitStore(Store *curr) {
139+
if (curr->align == 0 || curr->align >= curr->bytes) {
140+
return;
141+
}
142+
143+
// Switch unaligned stores of floats to unaligned stores of integers (which
144+
// we can actually implement) and then use reinterpretation to store the
145+
// right value.
146+
switch (curr->valueType) {
147+
case f32:
148+
curr->valueType = i32;
149+
curr->value = builder->makeUnary(ReinterpretFloat32, curr->value);
150+
break;
151+
case f64:
152+
curr->valueType = i64;
153+
curr->value = builder->makeUnary(ReinterpretFloat64, curr->value);
154+
break;
155+
default:
156+
break;
157+
}
158+
}
159+
116160
void visitBinary(Binary *curr) {
117161
Name name;
118162
switch (curr->op) {

src/tools/wasm2asm.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ int main(int argc, const char *argv[]) {
7676

7777
if (options.extra["asserts"] == "1") {
7878
if (options.debug) std::cerr << "asserting..." << std::endl;
79-
flattenAppend(asmjs, wasm2asm.processAsserts(*root, builder));
79+
flattenAppend(asmjs, wasm2asm.processAsserts(&wasm, *root, builder));
8080
}
8181
} catch (ParseException& p) {
8282
p.dump(std::cerr);

0 commit comments

Comments
 (0)