From aed3d6ce85ce5eb3d53f2c1eb948b7ae916cc4b7 Mon Sep 17 00:00:00 2001 From: Thomas Lively <7121787+tlively@users.noreply.github.com> Date: Fri, 19 Jul 2019 23:56:11 -0700 Subject: Revert "Remove bulk memory instructions refering to active segments (#2235)" (#2244) This reverts commit 72c52ea7d4eb61b95cf8a5164947cb760fe42e9c, which was causing test failures after it merged. --- src/ir/manipulation.h | 5 ----- src/passes/MemoryPacking.cpp | 44 ++++---------------------------------------- src/wasm/wasm-validator.cpp | 22 ++++++++-------------- 3 files changed, 12 insertions(+), 59 deletions(-) (limited to 'src') diff --git a/src/ir/manipulation.h b/src/ir/manipulation.h index ec137d372..d363fc547 100644 --- a/src/ir/manipulation.h +++ b/src/ir/manipulation.h @@ -38,11 +38,6 @@ template inline Nop* nop(InputType* target) { return convert(target); } -template -inline Unreachable* unreachable(InputType* target) { - return convert(target); -} - // Convert a node that allocates template inline OutputType* convert(InputType* input, MixedArena& allocator) { diff --git a/src/passes/MemoryPacking.cpp b/src/passes/MemoryPacking.cpp index f98c25096..0b24379e5 100644 --- a/src/passes/MemoryPacking.cpp +++ b/src/passes/MemoryPacking.cpp @@ -14,8 +14,6 @@ * limitations under the License. */ -#include "ir/manipulation.h" -#include "ir/utils.h" #include "pass.h" #include "wasm-binary.h" #include "wasm-builder.h" @@ -34,13 +32,11 @@ struct MemoryPacking : public Pass { return; } + // Conservatively refuse to change segments if any are passive to avoid + // invalidating segment indices or segment contents referenced from + // memory.init instructions. + // TODO: optimize in the presence of memory.init instructions if (module->features.hasBulkMemory()) { - // Remove any references to active segments that might be invalidated. - optimizeTrappingBulkMemoryOps(runner, module); - // Conservatively refuse to change segments if any are passive to avoid - // invalidating segment indices or segment contents referenced from - // memory.init and data.drop instructions. - // TODO: optimize in the presence of memory.init and data.drop for (auto segment : module->memory.segments) { if (segment.isPassive) { return; @@ -124,38 +120,6 @@ struct MemoryPacking : public Pass { } module->memory.segments.swap(packed); } - - void optimizeTrappingBulkMemoryOps(PassRunner* runner, Module* module) { - struct Trapper : WalkerPass> { - bool changed; - void visitMemoryInit(MemoryInit* curr) { - if (!getModule()->memory.segments[curr->segment].isPassive) { - Builder builder(*getModule()); - replaceCurrent(builder.blockify(builder.makeDrop(curr->dest), - builder.makeDrop(curr->offset), - builder.makeDrop(curr->size), - builder.makeUnreachable())); - changed = true; - } - } - void visitDataDrop(DataDrop* curr) { - if (!getModule()->memory.segments[curr->segment].isPassive) { - ExpressionManipulator::unreachable(curr); - changed = true; - } - } - void walkFunction(Function* func) { - changed = false; - PostWalker::walkFunction(func); - if (changed) { - ReFinalize().walkFunctionInModule(func, getModule()); - } - } - bool isFunctionParallel() override { return true; } - Pass* create() override { return new Trapper; } - } trapper; - trapper.run(runner, module); - } }; Pass* createMemoryPackingPass() { return new MemoryPacking(); } diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 8098116be..24f301110 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -970,6 +970,8 @@ void FunctionValidator::visitSIMDShift(SIMDShift* curr) { } void FunctionValidator::visitMemoryInit(MemoryInit* curr) { + shouldBeTrue( + getModule()->memory.exists, curr, "Memory operations require a memory"); shouldBeTrue(getModule()->features.hasBulkMemory(), curr, "Bulk memory operation (bulk memory is disabled)"); @@ -981,33 +983,27 @@ void FunctionValidator::visitMemoryInit(MemoryInit* curr) { curr->offset->type, i32, curr, "memory.init offset must be an i32"); shouldBeEqualOrFirstIsUnreachable( curr->size->type, i32, curr, "memory.init size must be an i32"); - if (!shouldBeTrue(getModule()->memory.exists, - curr, - "Memory operations require a memory")) { - return; - } shouldBeTrue(curr->segment < getModule()->memory.segments.size(), curr, "memory.init segment index out of bounds"); } void FunctionValidator::visitDataDrop(DataDrop* curr) { + shouldBeTrue( + getModule()->memory.exists, curr, "Memory operations require a memory"); shouldBeTrue(getModule()->features.hasBulkMemory(), curr, "Bulk memory operation (bulk memory is disabled)"); shouldBeEqualOrFirstIsUnreachable( curr->type, none, curr, "data.drop must have type none"); - if (!shouldBeTrue(getModule()->memory.exists, - curr, - "Memory operations require a memory")) { - return; - } shouldBeTrue(curr->segment < getModule()->memory.segments.size(), curr, "data.drop segment index out of bounds"); } void FunctionValidator::visitMemoryCopy(MemoryCopy* curr) { + shouldBeTrue( + getModule()->memory.exists, curr, "Memory operations require a memory"); shouldBeTrue(getModule()->features.hasBulkMemory(), curr, "Bulk memory operation (bulk memory is disabled)"); @@ -1019,11 +1015,11 @@ void FunctionValidator::visitMemoryCopy(MemoryCopy* curr) { curr->source->type, i32, curr, "memory.copy source must be an i32"); shouldBeEqualOrFirstIsUnreachable( curr->size->type, i32, curr, "memory.copy size must be an i32"); - shouldBeTrue( - getModule()->memory.exists, curr, "Memory operations require a memory"); } void FunctionValidator::visitMemoryFill(MemoryFill* curr) { + shouldBeTrue( + getModule()->memory.exists, curr, "Memory operations require a memory"); shouldBeTrue(getModule()->features.hasBulkMemory(), curr, "Bulk memory operation (bulk memory is disabled)"); @@ -1035,8 +1031,6 @@ void FunctionValidator::visitMemoryFill(MemoryFill* curr) { curr->value->type, i32, curr, "memory.fill value must be an i32"); shouldBeEqualOrFirstIsUnreachable( curr->size->type, i32, curr, "memory.fill size must be an i32"); - shouldBeTrue( - getModule()->memory.exists, curr, "Memory operations require a memory"); } void FunctionValidator::validateMemBytes(uint8_t bytes, -- cgit v1.2.3