From dad5c602e3b56ac8522572241ba2fa69e6e429d1 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 6 Dec 2019 05:34:25 -0800 Subject: [PATCH 01/11] Make local.tee's type its local's type According to the current spec, `local.tee`'s return type should be the same as its local's type. (Discussions on whether we should change this rule is going on in WebAssembly/reference-types#55, but here I will assume this spec does not change. If this changes, we should change many parts of Binaryen transformation anyway...) But currently in Binaryen `local.tee`'s type is computed from its value's type. This didn't make any difference in the MVP, but after we have subtype relationship in #2451, this can become a problem. For example: ``` (func $test (result funcref) (local $0 anyref) (local.tee $0 (ref.func $test) ) ) ``` This shouldn't validate in the spec, but this will pass Binaryen validation with the current `local.tee` implementation. This makes `local.tee`'s type computed from the local's type, and makes `LocalSet::makeTee` get a type parameter, to which we should pass the its corresponding local's type. We don't embed the local type in the class `LocalSet` because it may increase memory size. This also fixes the type of `local.get` to be the local type where `local.get` and `local.set` pair is created from `local.tee`. --- src/asm2wasm.h | 6 +++--- src/binaryen-c.cpp | 9 ++++---- src/binaryen-c.h | 6 ++++-- src/ir/ExpressionManipulator.cpp | 2 +- src/ir/localize.h | 2 +- src/js/binaryen.js-post.js | 4 ++-- src/passes/Flatten.cpp | 5 +++-- src/passes/LocalCSE.cpp | 3 ++- src/passes/MergeLocals.cpp | 2 +- src/passes/RemoveUnusedBrs.cpp | 4 ++-- src/passes/SSAify.cpp | 2 +- src/passes/SimplifyLocals.cpp | 9 ++++---- src/passes/Untee.cpp | 7 ++++--- src/passes/Vacuum.cpp | 2 +- src/tools/fuzzing.h | 2 +- src/wasm-builder.h | 6 ++++-- src/wasm.h | 5 +++-- src/wasm/wasm-binary.cpp | 7 +++++-- src/wasm/wasm-emscripten.cpp | 4 ++-- src/wasm/wasm-s-parser.cpp | 4 ++-- src/wasm/wasm-validator.cpp | 8 ++++++- src/wasm/wasm.cpp | 21 +++++++++---------- test/binaryen.js/kitchen-sink.js | 2 +- test/example/c-api-kitchen-sink.c | 4 +++- test/example/c-api-kitchen-sink.txt | 2 +- .../example/c-api-relooper-unreachable-if.cpp | 9 +++++--- 26 files changed, 80 insertions(+), 57 deletions(-) diff --git a/src/asm2wasm.h b/src/asm2wasm.h index 048d363ef16..4ae51025c69 100644 --- a/src/asm2wasm.h +++ b/src/asm2wasm.h @@ -1907,7 +1907,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) { auto ret = allocator.alloc(); ret->index = function->getLocalIndex(assign->target()); ret->value = process(assign->value()); - ret->setTee(false); + ret->makeSet(); ret->finalize(); return ret; } @@ -2160,7 +2160,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) { auto set = allocator.alloc(); set->index = function->getLocalIndex(I32_TEMP); set->value = value; - set->setTee(false); + set->makeSet(); set->finalize(); auto get = [&]() { auto ret = allocator.alloc(); @@ -2266,7 +2266,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) { view.bytes, 0, processUnshifted(ast[2][1], view.bytes), - builder.makeLocalTee(temp, process(ast[2][2])), + builder.makeLocalTee(temp, process(ast[2][2]), type), type), builder.makeLocalGet(temp, type)); } else if (name == Atomics_exchange) { diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index 87e1fdb0b05..bb81ac5faa0 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -1261,22 +1261,23 @@ BinaryenExpressionRef BinaryenLocalSet(BinaryenModuleRef module, ret->index = index; ret->value = (Expression*)value; - ret->setTee(false); + ret->makeSet(); ret->finalize(); return static_cast(ret); } BinaryenExpressionRef BinaryenLocalTee(BinaryenModuleRef module, BinaryenIndex index, - BinaryenExpressionRef value) { + BinaryenExpressionRef value, + BinaryenType type) { auto* ret = ((Module*)module)->allocator.alloc(); if (tracing) { - traceExpression(ret, "BinaryenLocalTee", index, value); + traceExpression(ret, "BinaryenLocalTee", index, value, type); } ret->index = index; ret->value = (Expression*)value; - ret->setTee(true); + ret->makeTee(Type(type)); ret->finalize(); return static_cast(ret); } diff --git a/src/binaryen-c.h b/src/binaryen-c.h index 046a5636371..8a303f18151 100644 --- a/src/binaryen-c.h +++ b/src/binaryen-c.h @@ -665,8 +665,10 @@ BINARYEN_API BinaryenExpressionRef BinaryenLocalGet(BinaryenModuleRef module, BinaryenType type); BINARYEN_API BinaryenExpressionRef BinaryenLocalSet( BinaryenModuleRef module, BinaryenIndex index, BinaryenExpressionRef value); -BINARYEN_API BinaryenExpressionRef BinaryenLocalTee( - BinaryenModuleRef module, BinaryenIndex index, BinaryenExpressionRef value); +BINARYEN_API BinaryenExpressionRef BinaryenLocalTee(BinaryenModuleRef module, + BinaryenIndex index, + BinaryenExpressionRef value, + BinaryenType type); BINARYEN_API BinaryenExpressionRef BinaryenGlobalGet(BinaryenModuleRef module, const char* name, BinaryenType type); diff --git a/src/ir/ExpressionManipulator.cpp b/src/ir/ExpressionManipulator.cpp index 2542e574355..ccb189db612 100644 --- a/src/ir/ExpressionManipulator.cpp +++ b/src/ir/ExpressionManipulator.cpp @@ -91,7 +91,7 @@ flexibleCopy(Expression* original, Module& wasm, CustomCopier custom) { } Expression* visitLocalSet(LocalSet* curr) { if (curr->isTee()) { - return builder.makeLocalTee(curr->index, copy(curr->value)); + return builder.makeLocalTee(curr->index, copy(curr->value), curr->type); } else { return builder.makeLocalSet(curr->index, copy(curr->value)); } diff --git a/src/ir/localize.h b/src/ir/localize.h index ff454382dc1..733e7bdecd3 100644 --- a/src/ir/localize.h +++ b/src/ir/localize.h @@ -36,7 +36,7 @@ struct Localizer { index = set->index; } else { index = Builder::addVar(func, expr->type); - expr = Builder(*wasm).makeLocalTee(index, expr); + expr = Builder(*wasm).makeLocalTee(index, expr, expr->type); } } }; diff --git a/src/js/binaryen.js-post.js b/src/js/binaryen.js-post.js index d5943176e0a..d4fdbf37f65 100644 --- a/src/js/binaryen.js-post.js +++ b/src/js/binaryen.js-post.js @@ -541,8 +541,8 @@ function wrapModule(module, self) { 'set': function(index, value) { return Module['_BinaryenLocalSet'](module, index, value); }, - 'tee': function(index, value) { - return Module['_BinaryenLocalTee'](module, index, value); + 'tee': function(index, value, type) { + return Module['_BinaryenLocalTee'](module, index, value, type); } } diff --git a/src/passes/Flatten.cpp b/src/passes/Flatten.cpp index 6f698367f00..33438158483 100644 --- a/src/passes/Flatten.cpp +++ b/src/passes/Flatten.cpp @@ -172,9 +172,10 @@ struct Flatten replaceCurrent(set->value); // trivial, no set happens } else { // use a set in a prelude + a get - set->setTee(false); + set->makeSet(); ourPreludes.push_back(set); - replaceCurrent(builder.makeLocalGet(set->index, set->value->type)); + replaceCurrent(builder.makeLocalGet( + set->index, getFunction()->getLocalType(set->index))); } } } else if (auto* br = curr->dynCast()) { diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp index afd30c040f5..0816bf6ea8e 100644 --- a/src/passes/LocalCSE.cpp +++ b/src/passes/LocalCSE.cpp @@ -184,8 +184,9 @@ struct LocalCSE : public WalkerPass> { if (iter != usables.end()) { // already exists in the table, this is good to reuse auto& info = iter->second; + Type localType = getFunction()->getLocalType(info.index); set->value = - Builder(*getModule()).makeLocalGet(info.index, value->type); + Builder(*getModule()).makeLocalGet(info.index, localType); anotherPass = true; } else { // not in table, add this, maybe we can help others later diff --git a/src/passes/MergeLocals.cpp b/src/passes/MergeLocals.cpp index c20105621c5..0116753f158 100644 --- a/src/passes/MergeLocals.cpp +++ b/src/passes/MergeLocals.cpp @@ -88,7 +88,7 @@ struct MergeLocals if (auto* get = curr->value->dynCast()) { if (get->index != curr->index) { Builder builder(*getModule()); - auto* trivial = builder.makeLocalTee(get->index, get); + auto* trivial = builder.makeLocalTee(get->index, get, get->type); curr->value = trivial; copies.push_back(curr); } diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp index 55f57302d19..e0174934aac 100644 --- a/src/passes/RemoveUnusedBrs.cpp +++ b/src/passes/RemoveUnusedBrs.cpp @@ -287,7 +287,7 @@ struct RemoveUnusedBrs : public WalkerPass> { Expression* z; replaceCurrent( z = builder.makeIf( - builder.makeLocalTee(temp, curr->condition), + builder.makeLocalTee(temp, curr->condition, i32), builder.makeIf(builder.makeBinary(EqInt32, builder.makeLocalGet(temp, i32), builder.makeConst(Literal(int32_t( @@ -1074,7 +1074,7 @@ struct RemoveUnusedBrs : public WalkerPass> { iff->finalize(); Expression* replacement = iff; if (tee) { - set->setTee(false); + set->makeSet(); // We need a block too. replacement = builder.makeSequence(iff, get // reuse the get diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index bcafb2784bd..df32fc77bee 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -154,7 +154,7 @@ struct SSAify : public Pass { if (set) { // a set exists, just add a tee of its value auto* value = set->value; - auto* tee = builder.makeLocalTee(new_, value); + auto* tee = builder.makeLocalTee(new_, value, get->type); set->value = tee; // the value may have been something we tracked the location // of. if so, update that, since we moved it into the tee diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index 6b76faed32e..a3fa4a34d87 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp @@ -256,7 +256,7 @@ struct SimplifyLocals } else { this->replaceCurrent(set); assert(!set->isTee()); - set->setTee(true); + set->makeTee(this->getFunction()->getLocalType(set->index)); } // reuse the local.get that is dying *found->second.item = curr; @@ -271,7 +271,7 @@ struct SimplifyLocals auto* set = curr->value->dynCast(); if (set) { assert(set->isTee()); - set->setTee(false); + set->makeSet(); this->replaceCurrent(set); } } @@ -559,7 +559,7 @@ struct SimplifyLocals auto* set = (*breakLocalSetPointer)->template cast(); if (br->condition) { br->value = set; - set->setTee(true); + set->makeTee(this->getFunction()->getLocalType(set->index)); *breakLocalSetPointer = this->getModule()->allocator.template alloc(); // in addition, as this is a conditional br that now has a value, it now @@ -728,7 +728,8 @@ struct SimplifyLocals ifTrueBlock->finalize(); assert(ifTrueBlock->type != none); // Update the ifFalse side. - iff->ifFalse = builder.makeLocalGet(set->index, set->value->type); + iff->ifFalse = builder.makeLocalGet( + set->index, this->getFunction()->getLocalType(set->index)); iff->finalize(); // update type // Update the get count. getCounter.num[set->index]++; diff --git a/src/passes/Untee.cpp b/src/passes/Untee.cpp index 79c76b988fe..6e5fb489b4c 100644 --- a/src/passes/Untee.cpp +++ b/src/passes/Untee.cpp @@ -41,9 +41,10 @@ struct Untee : public WalkerPass> { } else { // a normal tee. replace with set and get Builder builder(*getModule()); - replaceCurrent(builder.makeSequence( - curr, builder.makeLocalGet(curr->index, curr->value->type))); - curr->setTee(false); + LocalGet* get = builder.makeLocalGet( + curr->index, getFunction()->getLocalType(curr->index)); + replaceCurrent(builder.makeSequence(curr, get)); + curr->makeSet(); } } } diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index c2911f2b2bb..3850368124c 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -346,7 +346,7 @@ struct Vacuum : public WalkerPass> { // a drop of a tee is a set if (auto* set = curr->value->dynCast()) { assert(set->isTee()); - set->setTee(false); + set->makeSet(); replaceCurrent(set); return; } diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index 8de47900c1d..b9ff169480b 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -1282,7 +1282,7 @@ class TranslateToFuzzReader { } auto* value = make(valueType); if (tee) { - return builder.makeLocalTee(pick(locals), value); + return builder.makeLocalTee(pick(locals), value, valueType); } else { return builder.makeLocalSet(pick(locals), value); } diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 533f83d94d0..3459ebeed8e 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -241,14 +241,16 @@ class Builder { auto* ret = allocator.alloc(); ret->index = index; ret->value = value; + ret->makeSet(); ret->finalize(); return ret; } - LocalSet* makeLocalTee(Index index, Expression* value) { + LocalSet* makeLocalTee(Index index, Expression* value, Type type) { auto* ret = allocator.alloc(); ret->index = index; ret->value = value; - ret->setTee(true); + ret->makeTee(type); + ret->finalize(); return ret; } GlobalGet* makeGlobalGet(Name name, Type type) { diff --git a/src/wasm.h b/src/wasm.h index d9efe446aad..b65ada95315 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -725,8 +725,9 @@ class LocalSet : public SpecificExpression { Index index; Expression* value; - bool isTee(); - void setTee(bool is); + bool isTee() const; + void makeTee(Type type); + void makeSet(); }; class GlobalGet : public SpecificExpression { diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index b8b001927b1..f9952cc473e 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2527,8 +2527,11 @@ void WasmBinaryBuilder::visitLocalSet(LocalSet* curr, uint8_t code) { throwError("bad local.set index"); } curr->value = popNonVoidExpression(); - curr->type = curr->value->type; - curr->setTee(code == BinaryConsts::LocalTee); + if (code == BinaryConsts::LocalTee) { + curr->makeTee(currFunction->getLocalType(curr->index)); + } else { + curr->makeSet(); + } curr->finalize(); } diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 695561f68d4..a503d798c70 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -113,7 +113,7 @@ inline Expression* stackBoundsCheck(Builder& builder, auto check = builder.makeIf(builder.makeBinary( BinaryOp::LtUInt32, - builder.makeLocalTee(newSP, value), + builder.makeLocalTee(newSP, value, stackPointer->type), builder.makeGlobalGet(stackLimit->name, stackLimit->type)), builder.makeCall(handler, {}, none)); // (global.set $__stack_pointer (local.get $newSP)) @@ -173,7 +173,7 @@ void EmscriptenGlueGenerator::generateStackAllocFunction() { const static uint32_t bitMask = bitAlignment - 1; Const* subConst = builder.makeConst(Literal(~bitMask)); Binary* maskedSub = builder.makeBinary(AndInt32, sub, subConst); - LocalSet* teeStackLocal = builder.makeLocalTee(1, maskedSub); + LocalSet* teeStackLocal = builder.makeLocalTee(1, maskedSub, i32); Expression* storeStack = generateStoreStackPointer(function, teeStackLocal); Block* block = builder.makeBlock(); diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 24a4fcb3562..b820c696da0 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1040,7 +1040,7 @@ Expression* SExpressionWasmBuilder::makeLocalTee(Element& s) { auto ret = allocator.alloc(); ret->index = getLocalIndex(*s[1]); ret->value = parseExpression(s[2]); - ret->setTee(true); + ret->makeTee(currFunction->getLocalType(ret->index)); ret->finalize(); return ret; } @@ -1049,7 +1049,7 @@ Expression* SExpressionWasmBuilder::makeLocalSet(Element& s) { auto ret = allocator.alloc(); ret->index = getLocalIndex(*s[1]); ret->value = parseExpression(s[2]); - ret->setTee(false); + ret->makeSet(); ret->finalize(); return ret; } diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 75121d4f4c6..b2764c279c4 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -720,7 +720,13 @@ void FunctionValidator::visitLocalSet(LocalSet* curr) { if (curr->value->type != unreachable) { if (curr->type != none) { // tee is ok anyhow shouldBeEqualOrFirstIsUnreachable( - curr->value->type, curr->type, curr, "local.set type must be correct"); + curr->value->type, + curr->type, + curr, + "local.set's value type must be correct"); + shouldBeTrue(curr->type == getFunction()->getLocalType(curr->index), + curr, + "local.set's tpye must be correct"); } shouldBeEqual(getFunction()->getLocalType(curr->index), curr->value->type, diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index 5a722e600a5..7dea15d1d45 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -469,24 +469,23 @@ bool FunctionType::operator==(FunctionType& b) { } bool FunctionType::operator!=(FunctionType& b) { return !(*this == b); } -bool LocalSet::isTee() { return type != none; } +bool LocalSet::isTee() const { return type != none; } -void LocalSet::setTee(bool is) { - if (is) { - type = value->type; - } else { - type = none; - } +// Changes to local.tee. The type of the local should be given. +void LocalSet::makeTee(Type type_) { + type = type_; + finalize(); // type may need to be unreachable +} + +// Changes to local.set. +void LocalSet::makeSet() { + type = none; finalize(); // type may need to be unreachable } void LocalSet::finalize() { if (value->type == unreachable) { type = unreachable; - } else if (isTee()) { - type = value->type; - } else { - type = none; } } diff --git a/test/binaryen.js/kitchen-sink.js b/test/binaryen.js/kitchen-sink.js index 8c2ebc7832d..b1843cca507 100644 --- a/test/binaryen.js/kitchen-sink.js +++ b/test/binaryen.js/kitchen-sink.js @@ -469,7 +469,7 @@ function test_core() { ), module.drop(module.local.get(0, Binaryen.i32)), module.local.set(0, makeInt32(101)), - module.drop(module.local.tee(0, makeInt32(102))), + module.drop(module.local.tee(0, makeInt32(102), Binaryen.i32)), module.i32.load(0, 0, makeInt32(1)), module.i64.load16_s(2, 1, makeInt32(8)), module.f32.load(0, 0, makeInt32(2)), diff --git a/test/example/c-api-kitchen-sink.c b/test/example/c-api-kitchen-sink.c index f438015b8a7..c3c09425b03 100644 --- a/test/example/c-api-kitchen-sink.c +++ b/test/example/c-api-kitchen-sink.c @@ -642,7 +642,9 @@ void test_core() { module, makeInt32(module, 2449), callOperands4b, 4, "iiIfF")), BinaryenDrop(module, BinaryenLocalGet(module, 0, BinaryenTypeInt32())), BinaryenLocalSet(module, 0, makeInt32(module, 101)), - BinaryenDrop(module, BinaryenLocalTee(module, 0, makeInt32(module, 102))), + BinaryenDrop( + module, + BinaryenLocalTee(module, 0, makeInt32(module, 102), BinaryenTypeInt32())), BinaryenLoad(module, 4, 0, 0, 0, BinaryenTypeInt32(), makeInt32(module, 1)), BinaryenLoad(module, 2, 1, 2, 1, BinaryenTypeInt64(), makeInt32(module, 8)), BinaryenLoad( diff --git a/test/example/c-api-kitchen-sink.txt b/test/example/c-api-kitchen-sink.txt index 44f40ac4c00..caa3a65bec2 100644 --- a/test/example/c-api-kitchen-sink.txt +++ b/test/example/c-api-kitchen-sink.txt @@ -3835,7 +3835,7 @@ int main() { expressions[747] = BinaryenConst(the_module, BinaryenLiteralInt32(101)); expressions[748] = BinaryenLocalSet(the_module, 0, expressions[747]); expressions[749] = BinaryenConst(the_module, BinaryenLiteralInt32(102)); - expressions[750] = BinaryenLocalTee(the_module, 0, expressions[749]); + expressions[750] = BinaryenLocalTee(the_module, 0, expressions[749], 2); expressions[751] = BinaryenDrop(the_module, expressions[750]); expressions[752] = BinaryenConst(the_module, BinaryenLiteralInt32(1)); expressions[753] = BinaryenLoad(the_module, 4, 0, 0, 0, 2, expressions[752]); diff --git a/test/example/c-api-relooper-unreachable-if.cpp b/test/example/c-api-relooper-unreachable-if.cpp index 0cd4004c531..b468a9188dc 100644 --- a/test/example/c-api-relooper-unreachable-if.cpp +++ b/test/example/c-api-relooper-unreachable-if.cpp @@ -218,7 +218,8 @@ int main() { expressions[76] = BinaryenLoad(the_module, 4, 0, 0, 0, BinaryenTypeInt32(), expressions[75]); expressions[77] = BinaryenConst(the_module, BinaryenLiteralInt32(128)); expressions[78] = BinaryenBinary(the_module, 1, expressions[76], expressions[77]); - expressions[79] = BinaryenLocalTee(the_module, 3, expressions[78]); + expressions[79] = + BinaryenLocalTee(the_module, 3, expressions[78], BinaryenTypeInt32()); expressions[80] = BinaryenStore( the_module, 4, 0, 0, expressions[75], expressions[79], BinaryenTypeInt32()); expressions[81] = BinaryenLocalGet(the_module, 3, BinaryenTypeInt32()); @@ -307,7 +308,8 @@ int main() { expressions[123] = BinaryenLoad(the_module, 4, 0, 0, 0, BinaryenTypeInt32(), expressions[122]); expressions[124] = BinaryenConst(the_module, BinaryenLiteralInt32(128)); expressions[125] = BinaryenBinary(the_module, 1, expressions[123], expressions[124]); - expressions[126] = BinaryenLocalTee(the_module, 5, expressions[125]); + expressions[126] = + BinaryenLocalTee(the_module, 5, expressions[125], BinaryenTypeInt32()); expressions[127] = BinaryenStore(the_module, 4, 0, @@ -465,7 +467,8 @@ int main() { expressions[186] = BinaryenLoad(the_module, 4, 0, 0, 0, BinaryenTypeInt32(), expressions[185]); expressions[187] = BinaryenConst(the_module, BinaryenLiteralInt32(128)); expressions[188] = BinaryenBinary(the_module, 1, expressions[186], expressions[187]); - expressions[189] = BinaryenLocalTee(the_module, 6, expressions[188]); + expressions[189] = + BinaryenLocalTee(the_module, 6, expressions[188], BinaryenTypeInt32()); expressions[190] = BinaryenStore(the_module, 4, 0, From 28d9600fd1f51f8405cf9d7f5acae8cb96adeacd Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 6 Dec 2019 07:11:32 -0800 Subject: [PATCH 02/11] binaryen.js fix --- test/binaryen.js/kitchen-sink.js.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/binaryen.js/kitchen-sink.js.txt b/test/binaryen.js/kitchen-sink.js.txt index 601f08b4cdf..91480988b57 100644 --- a/test/binaryen.js/kitchen-sink.js.txt +++ b/test/binaryen.js/kitchen-sink.js.txt @@ -5545,7 +5545,7 @@ int main() { expressions[736] = BinaryenConst(the_module, BinaryenLiteralInt32(101)); expressions[737] = BinaryenLocalSet(the_module, 0, expressions[736]); expressions[738] = BinaryenConst(the_module, BinaryenLiteralInt32(102)); - expressions[739] = BinaryenLocalTee(the_module, 0, expressions[738]); + expressions[739] = BinaryenLocalTee(the_module, 0, expressions[738], 2); expressions[740] = BinaryenDrop(the_module, expressions[739]); expressions[741] = BinaryenConst(the_module, BinaryenLiteralInt32(1)); expressions[742] = BinaryenLoad(the_module, 4, 1, 0, 0, 2, expressions[741]); From e93ec17e93f4a56b9f6187f76fe59f56ec578a61 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Sun, 8 Dec 2019 00:21:20 -0800 Subject: [PATCH 03/11] Update src/wasm/wasm-validator.cpp Co-Authored-By: Thomas Lively <7121787+tlively@users.noreply.github.com> --- src/wasm/wasm-validator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index b2764c279c4..4d3f51af316 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -726,7 +726,7 @@ void FunctionValidator::visitLocalSet(LocalSet* curr) { "local.set's value type must be correct"); shouldBeTrue(curr->type == getFunction()->getLocalType(curr->index), curr, - "local.set's tpye must be correct"); + "local.set's type must be correct"); } shouldBeEqual(getFunction()->getLocalType(curr->index), curr->value->type, From 320c3130bd757f7d847ba1c2611f8f41c62200be Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Sun, 8 Dec 2019 00:25:32 -0800 Subject: [PATCH 04/11] Remove ret->finalize() --- src/wasm-builder.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 3459ebeed8e..6644609f873 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -250,7 +250,6 @@ class Builder { ret->index = index; ret->value = value; ret->makeTee(type); - ret->finalize(); return ret; } GlobalGet* makeGlobalGet(Name name, Type type) { From 2341ab1d02ee8499d7e523ff9e6ec739ebddd752 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Sun, 8 Dec 2019 00:55:33 -0800 Subject: [PATCH 05/11] Add to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dc5eedf045..fc339a5cc6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ full changeset diff at the end of each section. Current Trunk ------------- +- `local.tee`'s C/Binaryen.js API now takes an additional type parameter for its + local type, like `local.get`. This is required to handle subtypes. - Added load_splat SIMD instructions - Binaryen.js instruction API changes: - `notify` -> `atomic.notify` From 4ad6110c143a2172d0d7c8fc59b4bd09c9e07303 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 12 Dec 2019 04:32:43 -0800 Subject: [PATCH 06/11] Error out if local.tee type is undefined --- src/js/binaryen.js-post.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/js/binaryen.js-post.js b/src/js/binaryen.js-post.js index a9355e97db9..918d205cf2e 100644 --- a/src/js/binaryen.js-post.js +++ b/src/js/binaryen.js-post.js @@ -541,6 +541,9 @@ function wrapModule(module, self) { return Module['_BinaryenLocalSet'](module, index, value); }, 'tee': function(index, value, type) { + if (typeof type === 'undefined') { + throw new Error("local.tee's type should be defined"); + } return Module['_BinaryenLocalTee'](module, index, value, type); } } From d9c4973e9fb2afa72afef469b1dddc9017d93f32 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 12 Dec 2019 14:41:59 -0800 Subject: [PATCH 07/11] Binaryen.js test result fix --- test/binaryen.js/kitchen-sink.js.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/binaryen.js/kitchen-sink.js.txt b/test/binaryen.js/kitchen-sink.js.txt index 8db664c15a7..6878393da9e 100644 --- a/test/binaryen.js/kitchen-sink.js.txt +++ b/test/binaryen.js/kitchen-sink.js.txt @@ -10107,7 +10107,7 @@ module loaded from binary form: ) ) -[wasm-validator error in function func] i32 != i64: local.set type must match function, on +[wasm-validator error in function func] i32 != i64: local.set value type must be correct, on [none] (local.set $0 [i64] (i64.const 1234) ) From 94966e266fcf1a82bc98e1d07551916f3ea772ba Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 12 Dec 2019 15:41:47 -0800 Subject: [PATCH 08/11] aaaaarg --- test/binaryen.js/kitchen-sink.js.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/binaryen.js/kitchen-sink.js.txt b/test/binaryen.js/kitchen-sink.js.txt index 6878393da9e..74a11d48c50 100644 --- a/test/binaryen.js/kitchen-sink.js.txt +++ b/test/binaryen.js/kitchen-sink.js.txt @@ -10107,7 +10107,7 @@ module loaded from binary form: ) ) -[wasm-validator error in function func] i32 != i64: local.set value type must be correct, on +[wasm-validator error in function func] i64 != i32: local.set's value type must be correct, on [none] (local.set $0 [i64] (i64.const 1234) ) From 556ddf43b181a94f806a6f1feca03cfc5af2cbee Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 12 Dec 2019 16:22:39 -0800 Subject: [PATCH 09/11] Change to trigger CI --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3f9cf4ad4a..ff1717e0110 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Current Trunk ------------- - `local.tee`'s C/Binaryen.js API now takes an additional type parameter for its - local type, like `local.get`. This is required to handle subtypes. + local type, like `local.get`. This is required to handle subtypes - Added load_splat SIMD instructions - Binaryen.js instruction API changes: - `notify` -> `atomic.notify` From a937cae46499d892566fb7536a2f2e633af4df71 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 12 Dec 2019 16:23:00 -0800 Subject: [PATCH 10/11] Change to trigger CI --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff1717e0110..d3f9cf4ad4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Current Trunk ------------- - `local.tee`'s C/Binaryen.js API now takes an additional type parameter for its - local type, like `local.get`. This is required to handle subtypes + local type, like `local.get`. This is required to handle subtypes. - Added load_splat SIMD instructions - Binaryen.js instruction API changes: - `notify` -> `atomic.notify` From 7a77d506b7cd80b3531b9880f27ff406f370e519 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 12 Dec 2019 16:41:00 -0800 Subject: [PATCH 11/11] Extract localType as a variable --- src/passes/Flatten.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/passes/Flatten.cpp b/src/passes/Flatten.cpp index 33438158483..74788afb520 100644 --- a/src/passes/Flatten.cpp +++ b/src/passes/Flatten.cpp @@ -174,8 +174,8 @@ struct Flatten // use a set in a prelude + a get set->makeSet(); ourPreludes.push_back(set); - replaceCurrent(builder.makeLocalGet( - set->index, getFunction()->getLocalType(set->index))); + Type localType = getFunction()->getLocalType(set->index); + replaceCurrent(builder.makeLocalGet(set->index, localType)); } } } else if (auto* br = curr->dynCast()) {