From c6668c7c64e8c3539a52204bcdad429c39171bf5 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 14 Jul 2017 18:29:01 -0700 Subject: [PATCH 1/6] Optimizer support for atomic instructions --- src/ast/ExpressionManipulator.cpp | 16 +++++ src/ast/cost.h | 10 ++- src/ast/effects.h | 29 ++++++++- src/ast_utils.h | 2 + src/passes/DeadCodeElimination.cpp | 97 ++++++++++++------------------ src/passes/InstrumentMemory.cpp | 1 + src/passes/MergeBlocks.cpp | 16 +++++ src/passes/Precompute.cpp | 6 ++ src/wasm-builder.h | 35 +++++++++++ 9 files changed, 148 insertions(+), 64 deletions(-) diff --git a/src/ast/ExpressionManipulator.cpp b/src/ast/ExpressionManipulator.cpp index 3868ca316b1..cca799e10f6 100644 --- a/src/ast/ExpressionManipulator.cpp +++ b/src/ast/ExpressionManipulator.cpp @@ -96,11 +96,27 @@ Expression* flexibleCopy(Expression* original, Module& wasm, CustomCopier custom return builder.makeSetGlobal(curr->name, copy(curr->value)); } Expression* visitLoad(Load *curr) { + if (curr->isAtomic) { + return builder.makeAtomicLoad(curr->bytes, curr->signed_, curr->offset, + copy(curr->ptr), curr->type); + } return builder.makeLoad(curr->bytes, curr->signed_, curr->offset, curr->align, copy(curr->ptr), curr->type); } Expression* visitStore(Store *curr) { + if (curr->isAtomic) { + return builder.makeAtomicStore(curr->bytes, curr->offset, copy(curr->ptr), copy(curr->value), curr->valueType); + } return builder.makeStore(curr->bytes, curr->offset, curr->align, copy(curr->ptr), copy(curr->value), curr->valueType); } + Expression* visitAtomicRMW(AtomicRMW* curr) { + return builder.makeAtomicRMW(curr->op, curr->bytes, curr->offset, + copy(curr->ptr), copy(curr->value), curr->type); + } + Expression* visitAtomicCmpxchg(AtomicCmpxchg* curr) { + return builder.makeAtomicCmpxchg(curr->bytes, curr->offset, + copy(curr->ptr), copy(curr->expected), copy(curr->replacement), + curr->type); + } Expression* visitConst(Const *curr) { return builder.makeConst(curr->value); } diff --git a/src/ast/cost.h b/src/ast/cost.h index 1514686504f..56050b1896c 100644 --- a/src/ast/cost.h +++ b/src/ast/cost.h @@ -78,10 +78,16 @@ struct CostAnalyzer : public Visitor { return 2; } Index visitLoad(Load *curr) { - return 1 + visit(curr->ptr); + return 1 + visit(curr->ptr) + 10 * curr->isAtomic; } Index visitStore(Store *curr) { - return 2 + visit(curr->ptr) + visit(curr->value); + return 2 + visit(curr->ptr) + visit(curr->value) + 10 * curr->isAtomic; + } + Index visitAtomicRMW(AtomicRMW *curr) { + return 100; + } + Index visitAtomicCmpxchg(AtomicCmpxchg* curr) { + return 100; } Index visitConst(Const *curr) { return 1; diff --git a/src/ast/effects.h b/src/ast/effects.h index 6e4bb617eec..b7fce3c5296 100644 --- a/src/ast/effects.h +++ b/src/ast/effects.h @@ -53,12 +53,14 @@ struct EffectAnalyzer : public PostWalker { // (so a trap may occur later or earlier, if it is // going to occur anyhow), but we can't remove them, // they count as side effects + bool isAtomic = false; // An atomic load/store/RMW/Cmpxchg or an operator that + // has a defined ordering wrt atomics (e.g. grow_memory) bool accessesLocal() { return localsRead.size() + localsWritten.size() > 0; } bool accessesGlobal() { return globalsRead.size() + globalsWritten.size() > 0; } bool accessesMemory() { return calls || readsMemory || writesMemory; } - bool hasSideEffects() { return calls || localsWritten.size() > 0 || writesMemory || branches || globalsWritten.size() > 0 || implicitTrap; } - bool hasAnything() { return branches || calls || accessesLocal() || readsMemory || writesMemory || accessesGlobal() || implicitTrap; } + bool hasSideEffects() { return calls || localsWritten.size() > 0 || writesMemory || branches || globalsWritten.size() > 0 || implicitTrap || isAtomic; } + bool hasAnything() { return branches || calls || accessesLocal() || readsMemory || writesMemory || accessesGlobal() || implicitTrap || isAtomic; } // checks if these effects would invalidate another set (e.g., if we write, we invalidate someone that reads, they can't be moved past us) bool invalidates(EffectAnalyzer& other) { @@ -67,6 +69,9 @@ struct EffectAnalyzer : public PostWalker { || (accessesMemory() && (other.writesMemory || other.calls))) { return true; } + // All atomics are sequentially consistent for now, but have no ordering + // constraints wrt non-atomics. + if (isAtomic && other.isAtomic) return true; for (auto local : localsWritten) { if (other.localsWritten.count(local) || other.localsRead.count(local)) { return true; @@ -176,10 +181,24 @@ struct EffectAnalyzer : public PostWalker { } void visitLoad(Load *curr) { readsMemory = true; + isAtomic = curr->isAtomic; if (!ignoreImplicitTraps) implicitTrap = true; } void visitStore(Store *curr) { writesMemory = true; + isAtomic = curr->isAtomic; + if (!ignoreImplicitTraps) implicitTrap = true; + } + void visitAtomicRMW(AtomicRMW* curr) { + readsMemory = true; + writesMemory = true; + isAtomic = true; + if (!ignoreImplicitTraps) implicitTrap = true; + } + void visitAtomicCmpxchg(AtomicCmpxchg* curr) { + readsMemory = true; + writesMemory = true; + isAtomic = true; if (!ignoreImplicitTraps) implicitTrap = true; } void visitUnary(Unary *curr) { @@ -219,7 +238,11 @@ struct EffectAnalyzer : public PostWalker { } } void visitReturn(Return *curr) { branches = true; } - void visitHost(Host *curr) { calls = true; } + void visitHost(Host *curr) { + calls = true; + // Atomics are also sequentially consistent with grow_memory. + isAtomic = true; + } void visitUnreachable(Unreachable *curr) { branches = true; } }; diff --git a/src/ast_utils.h b/src/ast_utils.h index 253da8050b4..1f781b87e50 100644 --- a/src/ast_utils.h +++ b/src/ast_utils.h @@ -154,6 +154,8 @@ struct ReFinalize : public WalkerPass> { void visitSetGlobal(SetGlobal *curr) { curr->finalize(); } void visitLoad(Load *curr) { curr->finalize(); } void visitStore(Store *curr) { curr->finalize(); } + void visitAtomicRMW(AtomicRMW *curr) { curr->finalize(); } + void visitAtomicCmpxchg(AtomicCmpxchg *curr) { curr->finalize(); } void visitConst(Const *curr) { curr->finalize(); } void visitUnary(Unary *curr) { curr->finalize(); } void visitBinary(Binary *curr) { curr->finalize(); } diff --git a/src/passes/DeadCodeElimination.cpp b/src/passes/DeadCodeElimination.cpp index 321bc0f9aae..3a79af6cfab 100644 --- a/src/passes/DeadCodeElimination.cpp +++ b/src/passes/DeadCodeElimination.cpp @@ -28,6 +28,7 @@ // have no side effects. // +#include #include #include #include @@ -321,84 +322,62 @@ struct DeadCodeElimination : public WalkerPass> } } - void visitSetLocal(SetLocal* curr) { - if (isUnreachable(curr->value)) { - replaceCurrent(curr->value); + // Append the reachable operands of the current node to a block, and replace + // it with the block + void BlockifyReachableOperands(std::vector list, WasmType type) { + for (size_t i = 0; i < list.size(); ++i) { + auto* elem = list[i]; + if (isUnreachable(elem)) { + auto* replacement = elem; + if (i > 0) { + auto* block = getModule()->allocator.alloc(); + for (size_t j = 0; j < i; ++j) { + block->list.push_back(drop(list[j])); + } + block->list.push_back(list[i]); + block->finalize(type); + replacement = block; + } + replaceCurrent(replacement); + return; + } } } + void visitSetLocal(SetLocal* curr) { + BlockifyReachableOperands({curr->value}, curr->type); + } + void visitLoad(Load* curr) { - if (isUnreachable(curr->ptr)) { - replaceCurrent(curr->ptr); - } + BlockifyReachableOperands({curr->ptr}, curr->type); } void visitStore(Store* curr) { - if (isUnreachable(curr->ptr)) { - replaceCurrent(curr->ptr); - return; - } - if (isUnreachable(curr->value)) { - auto* block = getModule()->allocator.alloc(); - block->list.resize(2); - block->list[0] = drop(curr->ptr); - block->list[1] = curr->value; - block->finalize(curr->type); - replaceCurrent(block); - } + BlockifyReachableOperands({curr->ptr, curr->value}, curr->type); + } + + void visitAtomicRMW(AtomicRMW* curr) { + BlockifyReachableOperands({curr->ptr, curr->value}, curr->type); + } + + void visitAtomicCmpxchg(AtomicCmpxchg* curr) { + BlockifyReachableOperands({curr->ptr, curr->expected, curr->replacement}, curr->type); } void visitUnary(Unary* curr) { - if (isUnreachable(curr->value)) { - replaceCurrent(curr->value); - } + BlockifyReachableOperands({curr->value}, curr->type); } void visitBinary(Binary* curr) { - if (isUnreachable(curr->left)) { - replaceCurrent(curr->left); - return; - } - if (isUnreachable(curr->right)) { - auto* block = getModule()->allocator.alloc(); - block->list.resize(2); - block->list[0] = drop(curr->left); - block->list[1] = curr->right; - block->finalize(curr->type); - replaceCurrent(block); - } + BlockifyReachableOperands({curr->left, curr->right}, curr->type); } void visitSelect(Select* curr) { - if (isUnreachable(curr->ifTrue)) { - replaceCurrent(curr->ifTrue); - return; - } - if (isUnreachable(curr->ifFalse)) { - auto* block = getModule()->allocator.alloc(); - block->list.resize(2); - block->list[0] = drop(curr->ifTrue); - block->list[1] = curr->ifFalse; - block->finalize(curr->type); - replaceCurrent(block); - return; - } - if (isUnreachable(curr->condition)) { - auto* block = getModule()->allocator.alloc(); - block->list.resize(3); - block->list[0] = drop(curr->ifTrue); - block->list[1] = drop(curr->ifFalse); - block->list[2] = curr->condition; - block->finalize(curr->type); - replaceCurrent(block); - return; - } + BlockifyReachableOperands({curr->ifTrue, curr->ifFalse, curr->condition}, curr->type); } void visitDrop(Drop* curr) { - if (isUnreachable(curr->value)) { - replaceCurrent(curr->value); - } + BlockifyReachableOperands({curr->value}, curr->type); } void visitHost(Host* curr) { diff --git a/src/passes/InstrumentMemory.cpp b/src/passes/InstrumentMemory.cpp index 536031064c2..d9a5a431664 100644 --- a/src/passes/InstrumentMemory.cpp +++ b/src/passes/InstrumentMemory.cpp @@ -66,6 +66,7 @@ namespace wasm { Name load("load"); Name store("store"); +// TODO: Add support for atomicRMW/cmpxchg struct InstrumentMemory : public WalkerPass> { void visitLoad(Load* curr) { diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index bc5fea6fb05..bc4a134f798 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp @@ -286,6 +286,22 @@ struct MergeBlocks : public WalkerPass> { void visitStore(Store* curr) { optimize(curr, curr->value, optimize(curr, curr->ptr), &curr->ptr); } + void visitAtomicRMW(AtomicRMW* curr) { + optimize(curr, curr->value, optimize(curr, curr->ptr), &curr->ptr); + } + // XXX TODO: why doesn't this work for select? + void optimizeTernary(Expression* curr, + Expression* first, Expression* second, Expression* third) { + Block* outer = nullptr; + outer = optimize(curr, first, outer); + if (EffectAnalyzer(getPassOptions(), first).hasSideEffects()) return; + outer = optimize(curr, second, outer); + if (EffectAnalyzer(getPassOptions(), second).hasSideEffects()) return; + optimize(curr, third, outer); + } + void visitAtomicCmpxchg(AtomicCmpxchg* curr) { + optimizeTernary(curr, curr->ptr, curr->expected, curr->replacement); + } void visitSelect(Select* curr) { // TODO: for now, just stop when we see any side effect. instead, we could diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 122501de941..c4702fdeb8a 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -67,6 +67,12 @@ class StandaloneExpressionRunner : public ExpressionRunnertype = type; return ret; } + Load* makeAtomicLoad(unsigned bytes, bool signed_, uint32_t offset, Expression* ptr, WasmType type) { + Load* load = makeLoad(bytes, signed_, offset, getWasmTypeSize(type), ptr, type); + load->isAtomic = true; + return load; + } Store* makeStore(unsigned bytes, uint32_t offset, unsigned align, Expression *ptr, Expression *value, WasmType type) { auto* ret = allocator.alloc(); ret->isAtomic = false; @@ -201,6 +206,36 @@ class Builder { assert(isConcreteWasmType(ret->value->type) ? ret->value->type == type : true); return ret; } + Store* makeAtomicStore(unsigned bytes, uint32_t offset, Expression* ptr, Expression* value, WasmType type) { + Store* store = makeStore(bytes, offset, getWasmTypeSize(type), ptr, value, type); + store->isAtomic = true; + return store; + } + AtomicRMW* makeAtomicRMW(AtomicRMWOp op, unsigned bytes, uint32_t offset, + Expression* ptr, Expression* value, WasmType type) { + auto* ret = allocator.alloc(); + ret->op = op; + ret->bytes = bytes; + ret->offset = offset; + ret->ptr = ptr; + ret->value = value; + ret->type = type; + ret->finalize(); + return ret; + } + AtomicCmpxchg* makeAtomicCmpxchg(unsigned bytes, uint32_t offset, + Expression* ptr, Expression* expected, Expression* replacement, + WasmType type) { + auto* ret = allocator.alloc(); + ret->bytes = bytes; + ret->offset = offset; + ret->ptr = ptr; + ret->expected = expected; + ret->replacement = replacement; + ret->type = type; + ret->finalize(); + return ret; + } Const* makeConst(Literal value) { assert(isConcreteWasmType(value.type)); auto* ret = allocator.alloc(); From a4e11af93ecbca6f234da3f0cf81b42cc078685a Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 19 Jul 2017 13:13:57 -0700 Subject: [PATCH 2/6] try for select --- src/passes/MergeBlocks.cpp | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index bc4a134f798..86d009e0445 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp @@ -292,27 +292,22 @@ struct MergeBlocks : public WalkerPass> { // XXX TODO: why doesn't this work for select? void optimizeTernary(Expression* curr, Expression* first, Expression* second, Expression* third) { + // TODO: for now, just stop when we see any side effect. instead, we could + // check effects carefully for reordering Block* outer = nullptr; - outer = optimize(curr, first, outer); if (EffectAnalyzer(getPassOptions(), first).hasSideEffects()) return; - outer = optimize(curr, second, outer); + outer = optimize(curr, first, outer); if (EffectAnalyzer(getPassOptions(), second).hasSideEffects()) return; - optimize(curr, third, outer); + outer = optimize(curr, second, outer); + if (EffectAnalyzer(getPassOptions(), third).hasSideEffects()) return; + optimize(curr, third, outer); } void visitAtomicCmpxchg(AtomicCmpxchg* curr) { optimizeTernary(curr, curr->ptr, curr->expected, curr->replacement); } void visitSelect(Select* curr) { - // TODO: for now, just stop when we see any side effect. instead, we could - // check effects carefully for reordering - Block* outer = nullptr; - if (EffectAnalyzer(getPassOptions(), curr->ifTrue).hasSideEffects()) return; - outer = optimize(curr, curr->ifTrue, outer); - if (EffectAnalyzer(getPassOptions(), curr->ifFalse).hasSideEffects()) return; - outer = optimize(curr, curr->ifFalse, outer); - if (EffectAnalyzer(getPassOptions(), curr->condition).hasSideEffects()) return; - optimize(curr, curr->condition, outer); + optimizeTernary(curr, curr->ifTrue, curr->ifFalse, curr->condition); } void visitDrop(Drop* curr) { @@ -360,4 +355,3 @@ Pass *createMergeBlocksPass() { } } // namespace wasm - From 8c56cd347a26578a2ca2dd7eafd981b30d0eee07 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 19 Jul 2017 15:21:21 -0700 Subject: [PATCH 3/6] add mergeblocks test --- src/passes/MergeBlocks.cpp | 3 +- .../remove-unused-names_merge-blocks.txt | 29 ++++++++++++++- .../remove-unused-names_merge-blocks.wast | 37 ++++++++++++++++++- 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index 86d009e0445..455e5497191 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp @@ -289,9 +289,8 @@ struct MergeBlocks : public WalkerPass> { void visitAtomicRMW(AtomicRMW* curr) { optimize(curr, curr->value, optimize(curr, curr->ptr), &curr->ptr); } - // XXX TODO: why doesn't this work for select? void optimizeTernary(Expression* curr, - Expression* first, Expression* second, Expression* third) { + Expression*& first, Expression*& second, Expression*& third) { // TODO: for now, just stop when we see any side effect. instead, we could // check effects carefully for reordering Block* outer = nullptr; diff --git a/test/passes/remove-unused-names_merge-blocks.txt b/test/passes/remove-unused-names_merge-blocks.txt index bc42f27663d..6bd9d348ed9 100644 --- a/test/passes/remove-unused-names_merge-blocks.txt +++ b/test/passes/remove-unused-names_merge-blocks.txt @@ -7,7 +7,7 @@ (type $5 (func (result f64))) (table 1 1 anyfunc) (elem (i32.const 0) $call-i) - (memory $0 256 256) + (memory $0 256 256 shared) (func $call-i (type $i) (param $0 i32) (nop) ) @@ -735,6 +735,33 @@ (unreachable) ) ) + (func $atomics (type $3) + (drop + (i32.const 10) + ) + (drop + (i32.const 30) + ) + (drop + (i32.const 50) + ) + (drop + (i32.atomic.rmw.cmpxchg + (i32.const 20) + (i32.const 40) + (i32.const 60) + ) + ) + (drop + (i32.const 10) + ) + (drop + (i32.atomic.rmw.add + (i32.const 20) + (i32.const 30) + ) + ) + ) (func $mix-select (type $i) (param $x i32) (drop (select diff --git a/test/passes/remove-unused-names_merge-blocks.wast b/test/passes/remove-unused-names_merge-blocks.wast index 346dc78f88f..c249a34dd5e 100644 --- a/test/passes/remove-unused-names_merge-blocks.wast +++ b/test/passes/remove-unused-names_merge-blocks.wast @@ -1,5 +1,5 @@ (module - (memory 256 256) + (memory 256 256 shared) (type $i (func (param i32))) (type $ii (func (param i32 i32))) (type $iii (func (param i32 i32 i32))) @@ -885,6 +885,41 @@ (unreachable) ) ) + (func $atomics (type $3) + (drop + (i32.atomic.rmw.cmpxchg ;; mergeblock logic should be same as select + (block $block0 (result i32) + (drop + (i32.const 10) + ) + (i32.const 20) + ) + (block $block1 (result i32) + (drop + (i32.const 30) + ) + (i32.const 40) + ) + (block $block2 (result i32) + (drop + (i32.const 50) + ) + (i32.const 60) + ) + ) + ) + (drop + (i32.atomic.rmw.add ;; atomicrmw is like a binary + (block $block1 (result i32) + (drop + (i32.const 10) + ) + (i32.const 20) + ) + (i32.const 30) + ) + ) + ) (func $mix-select (param $x i32) (drop (select From fd310f579bad99c5bac812ecf5d68596fc3be32e Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 19 Jul 2017 15:44:22 -0700 Subject: [PATCH 4/6] formatting and rvalue --- src/passes/DeadCodeElimination.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/passes/DeadCodeElimination.cpp b/src/passes/DeadCodeElimination.cpp index 3a79af6cfab..5017569aac9 100644 --- a/src/passes/DeadCodeElimination.cpp +++ b/src/passes/DeadCodeElimination.cpp @@ -324,7 +324,7 @@ struct DeadCodeElimination : public WalkerPass> // Append the reachable operands of the current node to a block, and replace // it with the block - void BlockifyReachableOperands(std::vector list, WasmType type) { + void blockifyReachableOperands(std::vector&& list, WasmType type) { for (size_t i = 0; i < list.size(); ++i) { auto* elem = list[i]; if (isUnreachable(elem)) { @@ -345,39 +345,39 @@ struct DeadCodeElimination : public WalkerPass> } void visitSetLocal(SetLocal* curr) { - BlockifyReachableOperands({curr->value}, curr->type); + blockifyReachableOperands({ curr->value }, curr->type); } void visitLoad(Load* curr) { - BlockifyReachableOperands({curr->ptr}, curr->type); + blockifyReachableOperands({ curr->ptr}, curr->type); } void visitStore(Store* curr) { - BlockifyReachableOperands({curr->ptr, curr->value}, curr->type); + blockifyReachableOperands({ curr->ptr, curr->value }, curr->type); } void visitAtomicRMW(AtomicRMW* curr) { - BlockifyReachableOperands({curr->ptr, curr->value}, curr->type); + blockifyReachableOperands({ curr->ptr, curr->value }, curr->type); } void visitAtomicCmpxchg(AtomicCmpxchg* curr) { - BlockifyReachableOperands({curr->ptr, curr->expected, curr->replacement}, curr->type); + blockifyReachableOperands({ curr->ptr, curr->expected, curr->replacement }, curr->type); } void visitUnary(Unary* curr) { - BlockifyReachableOperands({curr->value}, curr->type); + blockifyReachableOperands({ curr->value }, curr->type); } void visitBinary(Binary* curr) { - BlockifyReachableOperands({curr->left, curr->right}, curr->type); + blockifyReachableOperands({ curr->left, curr->right}, curr->type); } void visitSelect(Select* curr) { - BlockifyReachableOperands({curr->ifTrue, curr->ifFalse, curr->condition}, curr->type); + blockifyReachableOperands({ curr->ifTrue, curr->ifFalse, curr->condition}, curr->type); } void visitDrop(Drop* curr) { - BlockifyReachableOperands({curr->value}, curr->type); + blockifyReachableOperands({ curr->value }, curr->type); } void visitHost(Host* curr) { @@ -394,4 +394,3 @@ Pass *createDeadCodeEliminationPass() { } } // namespace wasm - From 9540c04c560b4898aa38b3fdeb5c936e72d6ff33 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 20 Jul 2017 11:25:17 -0700 Subject: [PATCH 5/6] only set atomic property, don't remove it --- src/ast/effects.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ast/effects.h b/src/ast/effects.h index b7fce3c5296..3c7683dbf6f 100644 --- a/src/ast/effects.h +++ b/src/ast/effects.h @@ -181,12 +181,12 @@ struct EffectAnalyzer : public PostWalker { } void visitLoad(Load *curr) { readsMemory = true; - isAtomic = curr->isAtomic; + isAtomic |= curr->isAtomic; if (!ignoreImplicitTraps) implicitTrap = true; } void visitStore(Store *curr) { writesMemory = true; - isAtomic = curr->isAtomic; + isAtomic |= curr->isAtomic; if (!ignoreImplicitTraps) implicitTrap = true; } void visitAtomicRMW(AtomicRMW* curr) { @@ -249,4 +249,3 @@ struct EffectAnalyzer : public PostWalker { } // namespace wasm #endif // wasm_ast_effects_h - From c61935672fb36cd9d064203faa0282d01d2a5630 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 20 Jul 2017 18:03:20 -0700 Subject: [PATCH 6/6] order with all memory, add a few tests --- src/ast/effects.h | 11 ++- test/passes/simplify-locals.txt | 137 +++++++++++++++++++++++++++++++ test/passes/simplify-locals.wast | 58 +++++++++++++ 3 files changed, 203 insertions(+), 3 deletions(-) diff --git a/src/ast/effects.h b/src/ast/effects.h index 3c7683dbf6f..5392c0e500e 100644 --- a/src/ast/effects.h +++ b/src/ast/effects.h @@ -69,9 +69,12 @@ struct EffectAnalyzer : public PostWalker { || (accessesMemory() && (other.writesMemory || other.calls))) { return true; } - // All atomics are sequentially consistent for now, but have no ordering - // constraints wrt non-atomics. - if (isAtomic && other.isAtomic) return true; + // All atomics are sequentially consistent for now, and ordered wrt other + // memory references. + if ((isAtomic && other.accessesMemory()) || + (other.isAtomic && accessesMemory())) { + return true; + } for (auto local : localsWritten) { if (other.localsWritten.count(local) || other.localsRead.count(local)) { return true; @@ -240,6 +243,8 @@ struct EffectAnalyzer : public PostWalker { void visitReturn(Return *curr) { branches = true; } void visitHost(Host *curr) { calls = true; + // grow_memory modifies the set of valid addresses, and thus can be modeled as modifying memory + writesMemory = true; // Atomics are also sequentially consistent with grow_memory. isAtomic = true; } diff --git a/test/passes/simplify-locals.txt b/test/passes/simplify-locals.txt index 2be81e8c6de..a46f74c6967 100644 --- a/test/passes/simplify-locals.txt +++ b/test/passes/simplify-locals.txt @@ -876,3 +876,140 @@ ) ) ) +(module + (type $FUNCSIG$v (func)) + (type $FUNCSIG$i (func (result i32))) + (type $FUNCSIG$iiiii (func (param i32 i32 i32 i32) (result i32))) + (type $FUNCSIG$iiiiii (func (param i32 i32 i32 i32 i32) (result i32))) + (type $4 (func (param i32))) + (type $5 (func (param i32) (result i32))) + (type $6 (func (param i32 i32 i32 i32 i32 i32))) + (memory $0 256 256 shared) + (func $nonatomics (type $FUNCSIG$i) (result i32) + (local $x i32) + (nop) + (drop + (i32.load + (i32.const 1028) + ) + ) + (i32.load + (i32.const 1024) + ) + ) + (func $nonatomic-growmem (type $FUNCSIG$i) (result i32) + (local $x i32) + (set_local $x + (i32.load + (grow_memory + (i32.const 1) + ) + ) + ) + (drop + (i32.load + (i32.const 1028) + ) + ) + (get_local $x) + ) + (func $atomics (type $FUNCSIG$v) + (local $x i32) + (set_local $x + (i32.atomic.load + (i32.const 1024) + ) + ) + (drop + (i32.atomic.load + (i32.const 1028) + ) + ) + (drop + (get_local $x) + ) + ) + (func $one-atomic (type $FUNCSIG$v) + (local $x i32) + (set_local $x + (i32.load + (i32.const 1024) + ) + ) + (drop + (i32.atomic.load + (i32.const 1028) + ) + ) + (drop + (get_local $x) + ) + ) + (func $other-atomic (type $FUNCSIG$v) + (local $x i32) + (set_local $x + (i32.atomic.load + (i32.const 1024) + ) + ) + (drop + (i32.load + (i32.const 1028) + ) + ) + (drop + (get_local $x) + ) + ) + (func $atomic-growmem (type $FUNCSIG$i) (result i32) + (local $x i32) + (set_local $x + (i32.load + (grow_memory + (i32.const 1) + ) + ) + ) + (drop + (i32.atomic.load + (i32.const 1028) + ) + ) + (get_local $x) + ) + (func $atomicrmw (type $FUNCSIG$v) + (local $x i32) + (set_local $x + (i32.atomic.rmw.add + (i32.const 1024) + (i32.const 1) + ) + ) + (drop + (i32.atomic.load + (i32.const 1028) + ) + ) + (drop + (get_local $x) + ) + ) + (func $atomic-cmpxchg (type $FUNCSIG$v) + (local $x i32) + (set_local $x + (i32.atomic.rmw.cmpxchg + (i32.const 1024) + (i32.const 1) + (i32.const 2) + ) + ) + (drop + (i32.atomic.load + (i32.const 1028) + ) + ) + (drop + (get_local $x) + ) + ) +) diff --git a/test/passes/simplify-locals.wast b/test/passes/simplify-locals.wast index 534bd8883d6..344e0934e1a 100644 --- a/test/passes/simplify-locals.wast +++ b/test/passes/simplify-locals.wast @@ -872,3 +872,61 @@ ) ) ) +(module + (memory 256 256 shared) + (type $FUNCSIG$v (func)) + (type $FUNCSIG$i (func (result i32))) + (type $FUNCSIG$iiiii (func (param i32 i32 i32 i32) (result i32))) + (type $FUNCSIG$iiiiii (func (param i32 i32 i32 i32 i32) (result i32))) + (type $4 (func (param i32))) + (type $5 (func (param i32) (result i32))) + (type $6 (func (param i32 i32 i32 i32 i32 i32))) + (func $nonatomics (result i32) ;; loads are reordered + (local $x i32) + (set_local $x (i32.load (i32.const 1024))) + (drop (i32.load (i32.const 1028))) + (get_local $x) + ) + (func $nonatomic-growmem (result i32) ;; grow_memory is modeled as modifying memory + (local $x i32) + (set_local $x (i32.load (grow_memory (i32.const 1)))) + (drop (i32.load (i32.const 1028))) + (get_local $x) + ) + (func $atomics ;; atomic loads don't pass each other + (local $x i32) + (set_local $x (i32.atomic.load (i32.const 1024))) + (drop (i32.atomic.load (i32.const 1028))) + (drop (get_local $x)) + ) + (func $one-atomic ;; atomic loads don't pass other loads + (local $x i32) + (set_local $x (i32.load (i32.const 1024))) + (drop (i32.atomic.load (i32.const 1028))) + (drop (get_local $x)) + ) + (func $other-atomic ;; atomic loads don't pass other loads + (local $x i32) + (set_local $x (i32.atomic.load (i32.const 1024))) + (drop (i32.load (i32.const 1028))) + (drop (get_local $x)) + ) + (func $atomic-growmem (result i32) ;; grow_memory is modeled as modifying memory + (local $x i32) + (set_local $x (i32.load (grow_memory (i32.const 1)))) + (drop (i32.atomic.load (i32.const 1028))) + (get_local $x) + ) + (func $atomicrmw ;; atomic rmw don't pass loads + (local $x i32) + (set_local $x (i32.atomic.rmw.add (i32.const 1024) (i32.const 1))) + (drop (i32.atomic.load (i32.const 1028))) + (drop (get_local $x)) + ) + (func $atomic-cmpxchg ;; cmpxchg don't pass loads + (local $x i32) + (set_local $x (i32.atomic.rmw.cmpxchg (i32.const 1024) (i32.const 1) (i32.const 2))) + (drop (i32.atomic.load (i32.const 1028))) + (drop (get_local $x)) + ) +) \ No newline at end of file