From 8f897c7d5543fd1529b8279801ddbb59ffff7f1b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Sep 2016 15:14:34 -0700 Subject: autoDrop fix --- src/asm2wasm.h | 8 +++- src/ast_utils.h | 132 +++++++++++++++++++++++++++++++++++--------------------- 2 files changed, 90 insertions(+), 50 deletions(-) (limited to 'src') diff --git a/src/asm2wasm.h b/src/asm2wasm.h index 1c528d7f3..e926e381f 100644 --- a/src/asm2wasm.h +++ b/src/asm2wasm.h @@ -797,13 +797,17 @@ void Asm2WasmBuilder::processAsm(Ref ast) { switch (curr->type) { case i32: replaceCurrent(parent->builder.makeUnary(TruncSFloat64ToInt32, curr)); break; case f32: replaceCurrent(parent->builder.makeUnary(DemoteFloat64, curr)); break; - case none: replaceCurrent(parent->builder.makeDrop(curr)); break; + case none: { + // this function returns a value, but we are not using it, so it must be dropped. + // autodrop will do that for us. + break; + } default: WASM_UNREACHABLE(); } } else { assert(curr->type == none); // we don't want a return value here, but the import does provide one - replaceCurrent(parent->builder.makeDrop(curr)); + // autodrop will do that for us. } curr->type = importResult; } diff --git a/src/ast_utils.h b/src/ast_utils.h index ff5d93e02..9741faa25 100644 --- a/src/ast_utils.h +++ b/src/ast_utils.h @@ -422,6 +422,32 @@ struct ExpressionAnalyzer { return func->result != none; } + // Checks if a value is dropped. + static bool isResultDropped(std::vector stack) { + for (int i = int(stack.size()) - 2; i >= 0; i--) { + auto* curr = stack[i]; + auto* above = stack[i + 1]; + if (curr->is()) { + auto* block = curr->cast(); + for (size_t j = 0; j < block->list.size() - 1; j++) { + if (block->list[j] == above) return false; + } + assert(block->list.back() == above); + // continue down + } else if (curr->is()) { + auto* iff = curr->cast(); + if (above == iff->condition) return false; + if (!iff->ifFalse) return false; + assert(above == iff->ifTrue || above == iff->ifFalse); + // continue down + } else { + if (curr->is()) return true; // dropped + return false; // all other node types use the result + } + } + return false; + } + // Checks if a break is a simple - no condition, no value, just a plain branching static bool isSimple(Break* curr) { return !curr->condition && !curr->value; @@ -856,6 +882,34 @@ struct ExpressionAnalyzer { } }; +// Finalizes a node + +struct ReFinalize : public WalkerPass>> { + void visitBlock(Block *curr) { curr->finalize(); } + void visitIf(If *curr) { curr->finalize(); } + void visitLoop(Loop *curr) { curr->finalize(); } + void visitBreak(Break *curr) { curr->finalize(); } + void visitSwitch(Switch *curr) { curr->finalize(); } + void visitCall(Call *curr) { curr->finalize(); } + void visitCallImport(CallImport *curr) { curr->finalize(); } + void visitCallIndirect(CallIndirect *curr) { curr->finalize(); } + void visitGetLocal(GetLocal *curr) { curr->finalize(); } + void visitSetLocal(SetLocal *curr) { curr->finalize(); } + void visitGetGlobal(GetGlobal *curr) { curr->finalize(); } + void visitSetGlobal(SetGlobal *curr) { curr->finalize(); } + void visitLoad(Load *curr) { curr->finalize(); } + void visitStore(Store *curr) { curr->finalize(); } + void visitConst(Const *curr) { curr->finalize(); } + void visitUnary(Unary *curr) { curr->finalize(); } + void visitBinary(Binary *curr) { curr->finalize(); } + void visitSelect(Select *curr) { curr->finalize(); } + void visitDrop(Drop *curr) { curr->finalize(); } + void visitReturn(Return *curr) { curr->finalize(); } + void visitHost(Host *curr) { curr->finalize(); } + void visitNop(Nop *curr) { curr->finalize(); } + void visitUnreachable(Unreachable *curr) { curr->finalize(); } +}; + // Adds drop() operations where necessary. This lets you not worry about adding drop when // generating code. struct AutoDrop : public WalkerPass>> { @@ -863,6 +917,26 @@ struct AutoDrop : public WalkerPasstype)) { + expressionStack.push_back(child); + if (!ExpressionAnalyzer::isResultUsed(expressionStack, getFunction()) && !ExpressionAnalyzer::isResultDropped(expressionStack)) { + child = Builder(*getModule()).makeDrop(child); + acted = true; + } + expressionStack.pop_back(); + } + return acted; + } + + void reFinalize() { + for (int i = int(expressionStack.size()) - 1; i >= 0; i--) { + auto* curr = expressionStack[i]; + ReFinalize().visit(curr); + } + } + void visitBlock(Block* curr) { if (curr->list.size() == 0) return; for (Index i = 0; i < curr->list.size() - 1; i++) { @@ -871,31 +945,21 @@ struct AutoDrop : public WalkerPasslist[i] = Builder(*getModule()).makeDrop(child); } } - auto* last = curr->list.back(); - expressionStack.push_back(last); - if (isConcreteWasmType(last->type) && !ExpressionAnalyzer::isResultUsed(expressionStack, getFunction())) { - curr->list.back() = Builder(*getModule()).makeDrop(last); + if (maybeDrop(curr->list.back())) { + reFinalize(); + assert(curr->type == none); } - expressionStack.pop_back(); - curr->finalize(); // we may have changed our type } void visitIf(If* curr) { + bool acted = false; + if (maybeDrop(curr->ifTrue)) acted = true; if (curr->ifFalse) { - if (!isConcreteWasmType(curr->type)) { - // if either side of an if-else not returning a value is concrete, drop it - if (isConcreteWasmType(curr->ifTrue->type)) { - curr->ifTrue = Builder(*getModule()).makeDrop(curr->ifTrue); - } - if (isConcreteWasmType(curr->ifFalse->type)) { - curr->ifFalse = Builder(*getModule()).makeDrop(curr->ifFalse); - } - } - } else { - // if without else does not return a value, so the body must be dropped if it is concrete - if (isConcreteWasmType(curr->ifTrue->type)) { - curr->ifTrue = Builder(*getModule()).makeDrop(curr->ifTrue); - } + if (maybeDrop(curr->ifFalse)) acted = true; + } + if (acted) { + reFinalize(); + assert(curr->type == none); } } @@ -906,34 +970,6 @@ struct AutoDrop : public WalkerPass>> { - void visitBlock(Block *curr) { curr->finalize(); } - void visitIf(If *curr) { curr->finalize(); } - void visitLoop(Loop *curr) { curr->finalize(); } - void visitBreak(Break *curr) { curr->finalize(); } - void visitSwitch(Switch *curr) { curr->finalize(); } - void visitCall(Call *curr) { curr->finalize(); } - void visitCallImport(CallImport *curr) { curr->finalize(); } - void visitCallIndirect(CallIndirect *curr) { curr->finalize(); } - void visitGetLocal(GetLocal *curr) { curr->finalize(); } - void visitSetLocal(SetLocal *curr) { curr->finalize(); } - void visitGetGlobal(GetGlobal *curr) { curr->finalize(); } - void visitSetGlobal(SetGlobal *curr) { curr->finalize(); } - void visitLoad(Load *curr) { curr->finalize(); } - void visitStore(Store *curr) { curr->finalize(); } - void visitConst(Const *curr) { curr->finalize(); } - void visitUnary(Unary *curr) { curr->finalize(); } - void visitBinary(Binary *curr) { curr->finalize(); } - void visitSelect(Select *curr) { curr->finalize(); } - void visitDrop(Drop *curr) { curr->finalize(); } - void visitReturn(Return *curr) { curr->finalize(); } - void visitHost(Host *curr) { curr->finalize(); } - void visitNop(Nop *curr) { curr->finalize(); } - void visitUnreachable(Unreachable *curr) { curr->finalize(); } -}; - } // namespace wasm #endif // wasm_ast_utils_h -- cgit v1.2.3 From 8ef9aafab1a2c47f9a09c8e64636da570870dc00 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Sep 2016 15:35:58 -0700 Subject: replace two drops in an if-else with one on the if --- src/passes/Vacuum.cpp | 9 +++++++-- test/passes/vacuum.txt | 10 ++++++++++ test/passes/vacuum.wast | 12 ++++++++++++ test/unit.asm.js | 14 ++++++++++++++ test/unit.fromasm | 13 +++++++++++++ test/unit.fromasm.imprecise | 13 +++++++++++++ test/unit.fromasm.imprecise.no-opts | 19 +++++++++++++++++++ test/unit.fromasm.no-opts | 19 +++++++++++++++++++ 8 files changed, 107 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index 0fdd12dec..03becb04c 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -193,9 +193,14 @@ struct Vacuum : public WalkerPass> curr->ifTrue = curr->ifFalse; curr->ifFalse = nullptr; curr->condition = Builder(*getModule()).makeUnary(EqZInt32, curr->condition); + } else if (curr->ifTrue->is() && curr->ifFalse->is()) { + // instead of dropping both sides, drop the if + curr->ifTrue = curr->ifTrue->cast()->value; + curr->ifFalse = curr->ifFalse->cast()->value; + curr->finalize(); + replaceCurrent(Builder(*getModule()).makeDrop(curr)); } - } - if (!curr->ifFalse) { + } else { // no else if (curr->ifTrue->is()) { // no nothing diff --git a/test/passes/vacuum.txt b/test/passes/vacuum.txt index c2fb9fc29..6b5d63b2a 100644 --- a/test/passes/vacuum.txt +++ b/test/passes/vacuum.txt @@ -207,4 +207,14 @@ (func $relooperJumpThreading3 (type $0) (nop) ) + (func $if2drops (type $3) (result i32) + (drop + (if + (i32.const 1) + (call $if2drops) + (call $if2drops) + ) + ) + (i32.const 2) + ) ) diff --git a/test/passes/vacuum.wast b/test/passes/vacuum.wast index ecc176385..3584df3ff 100644 --- a/test/passes/vacuum.wast +++ b/test/passes/vacuum.wast @@ -412,4 +412,16 @@ ) ) ) + (func $if2drops (result i32) + (if + (i32.const 1) + (drop + (call $if2drops) + ) + (drop + (call $if2drops) + ) + ) + (i32.const 2) + ) ) diff --git a/test/unit.asm.js b/test/unit.asm.js index 931a75191..7b4b2a96f 100644 --- a/test/unit.asm.js +++ b/test/unit.asm.js @@ -587,6 +587,20 @@ function asm(global, env, buffer) { return; } + function dropIgnoredImportsInIf($0,$1,$2) { + $0 = $0|0; + $1 = $1|0; + $2 = $2|0; + do { + if ($0) { + lb($1 | 0) | 0; + } else { + lb($2 | 0) | 0; + } + } while(0); + return; + } + var FUNCTION_TABLE_a = [ z, big_negative, z, z ]; var FUNCTION_TABLE_b = [ w, w, importedDoubles, w ]; var FUNCTION_TABLE_c = [ z, cneg ]; diff --git a/test/unit.fromasm b/test/unit.fromasm index c4af783eb..123e087e6 100644 --- a/test/unit.fromasm +++ b/test/unit.fromasm @@ -1031,4 +1031,17 @@ ) ) ) + (func $dropIgnoredImportsInIf (param $0 i32) (param $1 i32) (param $2 i32) + (drop + (if + (get_local $0) + (call $lb + (get_local $1) + ) + (call $lb + (get_local $2) + ) + ) + ) + ) ) diff --git a/test/unit.fromasm.imprecise b/test/unit.fromasm.imprecise index 91e75d5c6..b170c8b82 100644 --- a/test/unit.fromasm.imprecise +++ b/test/unit.fromasm.imprecise @@ -1012,4 +1012,17 @@ ) ) ) + (func $dropIgnoredImportsInIf (param $0 i32) (param $1 i32) (param $2 i32) + (drop + (if + (get_local $0) + (call $lb + (get_local $1) + ) + (call $lb + (get_local $2) + ) + ) + ) + ) ) diff --git a/test/unit.fromasm.imprecise.no-opts b/test/unit.fromasm.imprecise.no-opts index ffcd9f449..913920f8b 100644 --- a/test/unit.fromasm.imprecise.no-opts +++ b/test/unit.fromasm.imprecise.no-opts @@ -1644,4 +1644,23 @@ ) (return) ) + (func $dropIgnoredImportsInIf (param $$0 i32) (param $$1 i32) (param $$2 i32) + (block $do-once$0 + (if + (get_local $$0) + (drop + (call $lb + (get_local $$1) + ) + ) + (drop + (call $lb + (get_local $$2) + ) + ) + ) + (nop) + ) + (return) + ) ) diff --git a/test/unit.fromasm.no-opts b/test/unit.fromasm.no-opts index a8ca0e47a..21e7ea79e 100644 --- a/test/unit.fromasm.no-opts +++ b/test/unit.fromasm.no-opts @@ -1650,4 +1650,23 @@ ) (return) ) + (func $dropIgnoredImportsInIf (param $$0 i32) (param $$1 i32) (param $$2 i32) + (block $do-once$0 + (if + (get_local $$0) + (drop + (call $lb + (get_local $$1) + ) + ) + (drop + (call $lb + (get_local $$2) + ) + ) + ) + (nop) + ) + (return) + ) ) -- cgit v1.2.3 From 7d5180a5b3e369634dbc1442117068d332d71049 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 23 Sep 2016 16:23:20 -0700 Subject: fix a simplify-locals bug where we didn't notice and if already had a result value --- src/passes/SimplifyLocals.cpp | 5 ++--- test/passes/simplify-locals.txt | 29 +++++++++++++++++++++++++++++ test/passes/simplify-locals.wast | 26 ++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index e032acc77..acc6860be 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp @@ -351,9 +351,8 @@ struct SimplifyLocals : public WalkerPassifFalse); - // if this if already has a result that is used, we can't do anything - assert(expressionStack.back() == iff); - if (ExpressionAnalyzer::isResultUsed(expressionStack, getFunction())) return; + // if this if already has a result, we can't do anything + if (isConcreteWasmType(iff->type)) return; // We now have the sinkables from both sides of the if. Sinkables& ifFalse = sinkables; Index sharedIndex = -1; diff --git a/test/passes/simplify-locals.txt b/test/passes/simplify-locals.txt index f2aa8d0d9..f12054887 100644 --- a/test/passes/simplify-locals.txt +++ b/test/passes/simplify-locals.txt @@ -9,6 +9,7 @@ (type $6 (func (param i32 i32 i32 i32 i32 i32))) (type $FUNCSIG$iii (func (param i32 i32) (result i32))) (type $8 (func (param i32 i32))) + (type $9 (func (param i32 i32 i32) (result i32))) (import "env" "waka" (func $waka)) (import "env" "waka_int" (func $waka_int (result i32))) (import "env" "i64sub" (func $_i64Subtract (param i32 i32 i32 i32) (result i32))) @@ -735,4 +736,32 @@ ) (get_local $a) ) + (func $drop-if-value (type $9) (param $x i32) (param $y i32) (param $z i32) (result i32) + (local $temp i32) + (drop + (if + (get_local $x) + (block $block53 i32 + (nop) + (set_local $temp + (get_local $y) + ) + (get_local $z) + ) + (block $block54 i32 + (nop) + (set_local $temp + (get_local $y) + ) + (get_local $z) + ) + ) + ) + (drop + (get_local $temp) + ) + (return + (i32.const 0) + ) + ) ) diff --git a/test/passes/simplify-locals.wast b/test/passes/simplify-locals.wast index 4fa345770..a998daf5f 100644 --- a/test/passes/simplify-locals.wast +++ b/test/passes/simplify-locals.wast @@ -775,4 +775,30 @@ ) (get_local $a) ) + (func $drop-if-value (param $x i32) (param $y i32) (param $z i32) (result i32) + (local $temp i32) + (drop + (if + (get_local $x) + (block $block53 i32 + (nop) + (set_local $temp + (get_local $y) + ) + (get_local $z) + ) + (block $block54 i32 + (nop) + (set_local $temp + (get_local $y) + ) + (get_local $z) + ) + ) + ) + (drop (get_local $temp)) + (return + (i32.const 0) + ) + ) ) -- cgit v1.2.3