From 1d67ab02aeb71b1a250a44161c8fdb3e97b04210 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 28 Jul 2017 12:45:19 -0700 Subject: do not combine a load/store offset with a constant pointer if it would wrap a negative value to a positive one, as trapping is tricky --- src/passes/OptimizeInstructions.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index de6f96ccc..2930fe9c9 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -908,8 +908,15 @@ private: // it's better to do the opposite for gzip purposes as well as for readability. auto* last = ptr->dynCast(); if (last) { - last->value = Literal(int32_t(last->value.geti32() + offset)); - offset = 0; + // don't do this if it would wrap the pointer + uint64_t value64 = last->value.geti32(); + uint64_t offset64 = offset; + if (value64 <= std::numeric_limits::max() && + offset64 <= std::numeric_limits::max() && + value64 + offset64 <= std::numeric_limits::max()) { + last->value = Literal(int32_t(value64 + offset64)); + offset = 0; + } } } -- cgit v1.2.3 From 1de12041904d5571ead6e64b5e51a3e26c8cfce1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 28 Jul 2017 14:10:25 -0700 Subject: fix shift computation in getMaxBits - in wasm only the lower 5 bits matter for a 32-bit shift --- src/passes/OptimizeInstructions.cpp | 4 ++-- test/passes/optimize-instructions.txt | 11 ++++++++++- test/passes/optimize-instructions.wast | 11 ++++++++++- 3 files changed, 22 insertions(+), 4 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 2930fe9c9..43a8faa32 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -195,7 +195,7 @@ Index getMaxBits(Expression* curr, LocalInfoProvider* localInfoProvider) { case ShrUInt32: { if (auto* shift = binary->right->dynCast()) { auto maxBits = getMaxBits(binary->left, localInfoProvider); - auto shifts = std::min(Index(shift->value.geti32()), maxBits); // can ignore more shifts than zero us out + auto shifts = std::min(Index(shift->value.geti32() & 31), maxBits); // can ignore more shifts than zero us out return std::max(Index(0), maxBits - shifts); } return 32; @@ -204,7 +204,7 @@ Index getMaxBits(Expression* curr, LocalInfoProvider* localInfoProvider) { if (auto* shift = binary->right->dynCast()) { auto maxBits = getMaxBits(binary->left, localInfoProvider); if (maxBits == 32) return 32; - auto shifts = std::min(Index(shift->value.geti32()), maxBits); // can ignore more shifts than zero us out + auto shifts = std::min(Index(shift->value.geti32() & 31), maxBits); // can ignore more shifts than zero us out return std::max(Index(0), maxBits - shifts); } return 32; diff --git a/test/passes/optimize-instructions.txt b/test/passes/optimize-instructions.txt index 301a6cfc0..68359d9fb 100644 --- a/test/passes/optimize-instructions.txt +++ b/test/passes/optimize-instructions.txt @@ -805,7 +805,7 @@ (i32.const -1) (i32.const 2147483647) ) - (i32.const 32) + (i32.const 31) ) ) (drop @@ -2006,4 +2006,13 @@ (i32.const 1) ) ) + (func $neg-shifts-and-255 (type $2) (result i32) + (i32.and + (i32.shr_u + (i32.const -99) + (i32.const -32) + ) + (i32.const 255) + ) + ) ) diff --git a/test/passes/optimize-instructions.wast b/test/passes/optimize-instructions.wast index cf8275412..5300b8da8 100644 --- a/test/passes/optimize-instructions.wast +++ b/test/passes/optimize-instructions.wast @@ -842,7 +842,7 @@ (i32.const -1) (i32.const 2147483647) ) - (i32.const 32) + (i32.const 31) ;; adjusted after we fixed shift computation to just look at lower 5 bits ) (i32.const 24) ) @@ -2433,4 +2433,13 @@ (i32.const 1) ) ) + (func $neg-shifts-and-255 (result i32) + (i32.and + (i32.shr_u + (i32.const -99) + (i32.const -32) ;; this shift does nothing + ) + (i32.const 255) + ) + ) ) -- cgit v1.2.3 From bd7f7ca58a93f438bd278e6ecb13afb685e5ed6e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 28 Jul 2017 15:41:10 -0700 Subject: fix shl shift computation in getMaxBits --- src/passes/OptimizeInstructions.cpp | 2 +- test/passes/optimize-instructions.txt | 9 +++++++++ test/passes/optimize-instructions.wast | 9 +++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 43a8faa32..2a77b408a 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -188,7 +188,7 @@ Index getMaxBits(Expression* curr, LocalInfoProvider* localInfoProvider) { case OrInt32: case XorInt32: return std::max(getMaxBits(binary->left, localInfoProvider), getMaxBits(binary->right, localInfoProvider)); case ShlInt32: { if (auto* shifts = binary->right->dynCast()) { - return std::min(Index(32), getMaxBits(binary->left, localInfoProvider) + shifts->value.geti32()); + return std::min(Index(32), getMaxBits(binary->left, localInfoProvider) + (shifts->value.geti32() & 31)); } return 32; } diff --git a/test/passes/optimize-instructions.txt b/test/passes/optimize-instructions.txt index 68359d9fb..f7948860c 100644 --- a/test/passes/optimize-instructions.txt +++ b/test/passes/optimize-instructions.txt @@ -2015,4 +2015,13 @@ (i32.const 255) ) ) + (func $neg-shifts-and-255-b (type $2) (result i32) + (i32.and + (i32.shl + (i32.const -2349025) + (i32.const -32) + ) + (i32.const 255) + ) + ) ) diff --git a/test/passes/optimize-instructions.wast b/test/passes/optimize-instructions.wast index 5300b8da8..107451b87 100644 --- a/test/passes/optimize-instructions.wast +++ b/test/passes/optimize-instructions.wast @@ -2442,4 +2442,13 @@ (i32.const 255) ) ) + (func $neg-shifts-and-255-b (result i32) + (i32.and + (i32.shl + (i32.const -2349025) + (i32.const -32) ;; this shift does nothing + ) + (i32.const 255) + ) + ) ) -- cgit v1.2.3 From db51c2efa3a9c2da064db199792b3bf0de4e850f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 28 Jul 2017 15:45:27 -0700 Subject: refactor effective shift size computation --- src/ast/bits.h | 11 +++++++++++ src/passes/OptimizeInstructions.cpp | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/ast/bits.h b/src/ast/bits.h index d88cb5edb..c7d2ea4f0 100644 --- a/src/ast/bits.h +++ b/src/ast/bits.h @@ -39,6 +39,17 @@ struct Bits { // this is indeed a mask return 32 - CountLeadingZeroes(mask); } + + // gets the number of effective shifts a shift operation does. In + // wasm, only 5 bits matter for 32-bit shifts, and 6 for 64. + static uint32_t getEffectiveShifts(Const* amount) { + if (amount->type == i32) { + return amount->value.geti32() & 31; + } else if (amount->type == i64) { + return amount->value.geti64() & 63; + } + WASM_UNREACHABLE(); + } }; } // namespace wasm diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 2a77b408a..263d4a96d 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -188,14 +188,14 @@ Index getMaxBits(Expression* curr, LocalInfoProvider* localInfoProvider) { case OrInt32: case XorInt32: return std::max(getMaxBits(binary->left, localInfoProvider), getMaxBits(binary->right, localInfoProvider)); case ShlInt32: { if (auto* shifts = binary->right->dynCast()) { - return std::min(Index(32), getMaxBits(binary->left, localInfoProvider) + (shifts->value.geti32() & 31)); + return std::min(Index(32), getMaxBits(binary->left, localInfoProvider) + Bits::getEffectiveShifts(shifts)); } return 32; } case ShrUInt32: { if (auto* shift = binary->right->dynCast()) { auto maxBits = getMaxBits(binary->left, localInfoProvider); - auto shifts = std::min(Index(shift->value.geti32() & 31), maxBits); // can ignore more shifts than zero us out + auto shifts = std::min(Index(Bits::getEffectiveShifts(shift)), maxBits); // can ignore more shifts than zero us out return std::max(Index(0), maxBits - shifts); } return 32; @@ -204,7 +204,7 @@ Index getMaxBits(Expression* curr, LocalInfoProvider* localInfoProvider) { if (auto* shift = binary->right->dynCast()) { auto maxBits = getMaxBits(binary->left, localInfoProvider); if (maxBits == 32) return 32; - auto shifts = std::min(Index(shift->value.geti32() & 31), maxBits); // can ignore more shifts than zero us out + auto shifts = std::min(Index(Bits::getEffectiveShifts(shift)), maxBits); // can ignore more shifts than zero us out return std::max(Index(0), maxBits - shifts); } return 32; -- cgit v1.2.3 From 56e49752b4258b89660825f2970a7e55067d7122 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sat, 29 Jul 2017 19:00:37 -0700 Subject: do not swap elements in conditionalizeExpensiveOnBitwise if they invalidate each other - it is not enough to check side effects, we must check the interaction as well --- src/passes/OptimizeInstructions.cpp | 15 +++-- ...ions_optimize-level=2_ignore-implicit-traps.txt | 70 ++++++++++++++++++++++ ...ons_optimize-level=2_ignore-implicit-traps.wast | 64 ++++++++++++++++++++ 3 files changed, 143 insertions(+), 6 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 263d4a96d..424cbcc67 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -874,14 +874,17 @@ private: auto* left = binary->left; auto* right = binary->right; if (!Properties::emitsBoolean(left) || !Properties::emitsBoolean(right)) return nullptr; - auto leftEffects = EffectAnalyzer(getPassOptions(), left).hasSideEffects(); - auto rightEffects = EffectAnalyzer(getPassOptions(), right).hasSideEffects(); - if (leftEffects && rightEffects) return nullptr; // both must execute - // canonicalize with side effects, if any, happening on the left - if (rightEffects) { + auto leftEffects = EffectAnalyzer(getPassOptions(), left); + auto rightEffects = EffectAnalyzer(getPassOptions(), right); + auto leftSideEffects = leftEffects.hasSideEffects(); + auto rightSideEffects = rightEffects.hasSideEffects(); + if (leftSideEffects && rightSideEffects) return nullptr; // both must execute + // canonicalize with side effects, if any, happening on the left + if (rightSideEffects) { if (CostAnalyzer(left).cost < MIN_COST) return nullptr; // avoidable code is too cheap + if (leftEffects.invalidates(rightEffects)) return nullptr; // cannot reorder std::swap(left, right); - } else if (leftEffects) { + } else if (leftSideEffects) { if (CostAnalyzer(right).cost < MIN_COST) return nullptr; // avoidable code is too cheap } else { // no side effects, reorder based on cost estimation diff --git a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt index e2c4bb786..922f64008 100644 --- a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt +++ b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.txt @@ -251,4 +251,74 @@ (get_local $5) ) ) + (func $invalidate-conditionalizeExpensiveOnBitwise (type $0) (param $0 i32) (param $1 i32) (result i32) + (if + (i32.eqz + (i32.and + (i32.lt_s + (i32.and + (i32.shr_s + (i32.shl + (i32.add + (get_local $1) + (i32.const -1) + ) + (i32.const 24) + ) + (i32.const 24) + ) + (i32.const 255) + ) + (i32.const 3) + ) + (i32.ne + (tee_local $1 + (i32.const 0) + ) + (i32.const 0) + ) + ) + ) + (return + (get_local $0) + ) + ) + (return + (get_local $1) + ) + ) + (func $invalidate-conditionalizeExpensiveOnBitwise-ok (type $0) (param $0 i32) (param $1 i32) (result i32) + (if + (i32.eqz + (if (result i32) + (tee_local $1 + (i32.const 0) + ) + (i32.lt_s + (i32.and + (i32.shr_s + (i32.shl + (i32.add + (get_local $0) + (i32.const -1) + ) + (i32.const 24) + ) + (i32.const 24) + ) + (i32.const 255) + ) + (i32.const 3) + ) + (i32.const 0) + ) + ) + (return + (get_local $0) + ) + ) + (return + (get_local $1) + ) + ) ) diff --git a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast index e1215fa3f..7e8365812 100644 --- a/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast +++ b/test/passes/optimize-instructions_optimize-level=2_ignore-implicit-traps.wast @@ -259,5 +259,69 @@ (get_local $5) ) ) + (func $invalidate-conditionalizeExpensiveOnBitwise (param $0 i32) (param $1 i32) (result i32) + (if + (i32.eqz + (i32.and + (i32.lt_s + (i32.and + (i32.shr_s + (i32.shl + (i32.add + (get_local $1) ;; conflict with tee + (i32.const -1) + ) + (i32.const 24) + ) + (i32.const 24) + ) + (i32.const 255) + ) + (i32.const 3) + ) + (i32.ne + (tee_local $1 + (i32.const 0) + ) + (i32.const 0) + ) + ) + ) + (return (get_local $0)) + ) + (return (get_local $1)) + ) + (func $invalidate-conditionalizeExpensiveOnBitwise-ok (param $0 i32) (param $1 i32) (result i32) + (if + (i32.eqz + (i32.and + (i32.lt_s + (i32.and + (i32.shr_s + (i32.shl + (i32.add + (get_local $0) ;; no conflict + (i32.const -1) + ) + (i32.const 24) + ) + (i32.const 24) + ) + (i32.const 255) + ) + (i32.const 3) + ) + (i32.ne + (tee_local $1 + (i32.const 0) + ) + (i32.const 0) + ) + ) + ) + (return (get_local $0)) + ) + (return (get_local $1)) + ) ) -- cgit v1.2.3 From 6cdffee098cb891c7309eb372aea63c0baa7a2c5 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sun, 30 Jul 2017 11:07:51 -0700 Subject: fix optimizing two shifts into one; if the number of effective shifts overflows, it is not vali to just add them --- src/ast/bits.h | 15 ++++++++++--- src/ast/literal-utils.h | 26 ++++++++++++++++------- src/passes/OptimizeInstructions.cpp | 14 +++++++++--- test/passes/optimize-instructions.txt | 39 ++++++++++++++++++++++++++++++---- test/passes/optimize-instructions.wast | 36 +++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 18 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/ast/bits.h b/src/ast/bits.h index c7d2ea4f0..11cf7b06d 100644 --- a/src/ast/bits.h +++ b/src/ast/bits.h @@ -42,11 +42,20 @@ struct Bits { // gets the number of effective shifts a shift operation does. In // wasm, only 5 bits matter for 32-bit shifts, and 6 for 64. - static uint32_t getEffectiveShifts(Const* amount) { + static Index getEffectiveShifts(Index amount, WasmType type) { + if (type == i32) { + return amount & 31; + } else if (type == i64) { + return amount & 63; + } + WASM_UNREACHABLE(); + } + + static Index getEffectiveShifts(Const* amount) { if (amount->type == i32) { - return amount->value.geti32() & 31; + return getEffectiveShifts(amount->value.geti32(), i32); } else if (amount->type == i64) { - return amount->value.geti64() & 63; + return getEffectiveShifts(amount->value.geti64(), i64); } WASM_UNREACHABLE(); } diff --git a/src/ast/literal-utils.h b/src/ast/literal-utils.h index 7e75e8bc8..afa8146b9 100644 --- a/src/ast/literal-utils.h +++ b/src/ast/literal-utils.h @@ -23,21 +23,31 @@ namespace wasm { namespace LiteralUtils { -inline Expression* makeZero(WasmType type, Module& wasm) { - Literal value; +inline Literal makeLiteralFromInt32(int32_t x, WasmType type) { switch (type) { - case i32: value = Literal(int32_t(0)); break; - case i64: value = Literal(int64_t(0)); break; - case f32: value = Literal(float(0)); break; - case f64: value = Literal(double(0)); break; + case i32: return Literal(int32_t(x)); break; + case i64: return Literal(int64_t(x)); break; + case f32: return Literal(float(x)); break; + case f64: return Literal(double(x)); break; default: WASM_UNREACHABLE(); } +} + +inline Literal makeLiteralZero(WasmType type) { + return makeLiteralFromInt32(0, type); +} + +inline Expression* makeFromInt32(int32_t x, WasmType type, Module& wasm) { auto* ret = wasm.allocator.alloc(); - ret->value = value; - ret->type = value.type; + ret->value = makeLiteralFromInt32(x, type); + ret->type = type; return ret; } +inline Expression* makeZero(WasmType type, Module& wasm) { + return makeFromInt32(0, type, wasm); +} + } // namespace LiteralUtils } // namespace wasm diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 424cbcc67..60222c3cd 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -29,6 +29,7 @@ #include #include #include +#include namespace wasm { @@ -533,9 +534,16 @@ struct OptimizeInstructions : public WalkerPassop == OrInt32) { leftRight->value = leftRight->value.or_(right->value); return left; - } else if (left->op == ShlInt32 || left->op == ShrUInt32 || left->op == ShrSInt32) { - leftRight->value = leftRight->value.add(right->value); - return left; + } else if (left->op == ShlInt32 || left->op == ShrUInt32 || left->op == ShrSInt32 || + left->op == ShlInt64 || left->op == ShrUInt64 || left->op == ShrSInt64) { + // shifts only use an effective amount from the constant, so adding must + // be done carefully + auto total = Bits::getEffectiveShifts(leftRight) + Bits::getEffectiveShifts(right); + if (total == Bits::getEffectiveShifts(total, left->type)) { + // no overflow, we can do this + leftRight->value = LiteralUtils::makeLiteralFromInt32(total, left->type); + return left; + } // TODO: handle overflows } } } diff --git a/test/passes/optimize-instructions.txt b/test/passes/optimize-instructions.txt index f7948860c..e376d97d9 100644 --- a/test/passes/optimize-instructions.txt +++ b/test/passes/optimize-instructions.txt @@ -6,6 +6,7 @@ (type $4 (func (param i32 i32))) (type $5 (func (param i32))) (type $6 (func (param i32 i32) (result i32))) + (type $7 (func (param i64) (result i64))) (memory $0 0) (export "load-off-2" (func $load-off-2)) (func $f (type $0) (param $i1 i32) (param $i2 i64) @@ -734,7 +735,7 @@ (i32.shr_s (i32.shl (i32.const 32) - (i32.const 59) + (i32.const 27) ) (i32.const 24) ) @@ -1083,19 +1084,19 @@ (drop (i32.shl (get_local $0) - (i32.const 211) + (i32.const 19) ) ) (drop (i32.shr_s (get_local $0) - (i32.const 211) + (i32.const 19) ) ) (drop (i32.shr_u (get_local $0) - (i32.const 211) + (i32.const 19) ) ) (drop @@ -2024,4 +2025,34 @@ (i32.const 255) ) ) + (func $shifts-square-overflow (type $3) (param $x i32) (result i32) + (i32.shr_u + (i32.shr_u + (get_local $x) + (i32.const 65535) + ) + (i32.const 32767) + ) + ) + (func $shifts-square-no-overflow-small (type $3) (param $x i32) (result i32) + (i32.shr_u + (get_local $x) + (i32.const 9) + ) + ) + (func $shifts-square-overflow-64 (type $7) (param $x i64) (result i64) + (i64.shr_u + (i64.shr_u + (get_local $x) + (i64.const 65535) + ) + (i64.const 64767) + ) + ) + (func $shifts-square-no-overflow-small-64 (type $7) (param $x i64) (result i64) + (i64.shr_u + (get_local $x) + (i64.const 9) + ) + ) ) diff --git a/test/passes/optimize-instructions.wast b/test/passes/optimize-instructions.wast index 107451b87..7e07e8aee 100644 --- a/test/passes/optimize-instructions.wast +++ b/test/passes/optimize-instructions.wast @@ -2451,4 +2451,40 @@ (i32.const 255) ) ) + (func $shifts-square-overflow (param $x i32) (result i32) + (i32.shr_u + (i32.shr_u + (get_local $x) + (i32.const 65535) ;; 31 bits effectively + ) + (i32.const 32767) ;; also 31 bits, so two shifts that force the value into nothing for sure + ) + ) + (func $shifts-square-no-overflow-small (param $x i32) (result i32) + (i32.shr_u + (i32.shr_u + (get_local $x) + (i32.const 1031) ;; 7 bits effectively + ) + (i32.const 4098) ;; 2 bits effectively + ) + ) + (func $shifts-square-overflow-64 (param $x i64) (result i64) + (i64.shr_u + (i64.shr_u + (get_local $x) + (i64.const 65535) ;; 63 bits effectively + ) + (i64.const 64767) ;; also 63 bits, so two shifts that force the value into nothing for sure + ) + ) + (func $shifts-square-no-overflow-small-64 (param $x i64) (result i64) + (i64.shr_u + (i64.shr_u + (get_local $x) + (i64.const 1031) ;; 7 bits effectively + ) + (i64.const 4098) ;; 2 bits effectively + ) + ) ) -- cgit v1.2.3 From 4aa3463a6b98e41070d03ddbe60e689537b02b59 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 31 Jul 2017 08:05:26 -0700 Subject: handle squared shifts of an unreachable --- src/passes/OptimizeInstructions.cpp | 4 ++-- test/passes/optimize-instructions.txt | 6 ++++++ test/passes/optimize-instructions.wast | 9 +++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 60222c3cd..6179833b8 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -539,9 +539,9 @@ struct OptimizeInstructions : public WalkerPasstype)) { + if (total == Bits::getEffectiveShifts(total, right->type)) { // no overflow, we can do this - leftRight->value = LiteralUtils::makeLiteralFromInt32(total, left->type); + leftRight->value = LiteralUtils::makeLiteralFromInt32(total, right->type); return left; } // TODO: handle overflows } diff --git a/test/passes/optimize-instructions.txt b/test/passes/optimize-instructions.txt index e376d97d9..a399a9096 100644 --- a/test/passes/optimize-instructions.txt +++ b/test/passes/optimize-instructions.txt @@ -2055,4 +2055,10 @@ (i64.const 9) ) ) + (func $shifts-square-unreachable (type $3) (param $x i32) (result i32) + (i32.shr_u + (unreachable) + (i32.const 9) + ) + ) ) diff --git a/test/passes/optimize-instructions.wast b/test/passes/optimize-instructions.wast index 7e07e8aee..8362453e9 100644 --- a/test/passes/optimize-instructions.wast +++ b/test/passes/optimize-instructions.wast @@ -2487,4 +2487,13 @@ (i64.const 4098) ;; 2 bits effectively ) ) + (func $shifts-square-unreachable (param $x i32) (result i32) + (i32.shr_u + (i32.shr_u + (unreachable) + (i32.const 1031) ;; 7 bits effectively + ) + (i32.const 4098) ;; 2 bits effectively + ) + ) ) -- cgit v1.2.3 From bfdbc0c3f6a835231b218a60ddd6b52f7e9affed Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 31 Jul 2017 13:39:46 -0700 Subject: review comments --- src/passes/MergeBlocks.cpp | 10 +++++----- src/passes/OptimizeInstructions.cpp | 10 +++++----- src/wasm/wasm-binary.cpp | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) (limited to 'src/passes/OptimizeInstructions.cpp') diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index 05aa468e1..7c086a920 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp @@ -73,7 +73,7 @@ namespace wasm { // For example, if there is a switch targeting us, we can't do it - we can't remove the value from other targets struct ProblemFinder : public ControlFlowWalker { Name origin; - bool noWay = false; + bool foundProblem = false; // count br_ifs, and dropped br_ifs. if they don't match, then a br_if flow value is used, and we can't drop it Index brIfs = 0; Index droppedBrIfs = 0; @@ -88,7 +88,7 @@ struct ProblemFinder : public ControlFlowWalker { } // if the value has side effects, we can't remove it if (EffectAnalyzer(passOptions, curr->value).hasSideEffects()) { - noWay = true; + foundProblem = true; } } } @@ -103,12 +103,12 @@ struct ProblemFinder : public ControlFlowWalker { void visitSwitch(Switch* curr) { if (curr->default_ == origin) { - noWay = true; + foundProblem = true; return; } for (auto& target : curr->targets) { if (target == origin) { - noWay = true; + foundProblem = true; return; } } @@ -116,7 +116,7 @@ struct ProblemFinder : public ControlFlowWalker { bool found() { assert(brIfs >= droppedBrIfs); - return noWay || brIfs > droppedBrIfs; + return foundProblem || brIfs > droppedBrIfs; } }; diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 6179833b8..4fd7cbe8d 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -884,15 +884,15 @@ private: if (!Properties::emitsBoolean(left) || !Properties::emitsBoolean(right)) return nullptr; auto leftEffects = EffectAnalyzer(getPassOptions(), left); auto rightEffects = EffectAnalyzer(getPassOptions(), right); - auto leftSideEffects = leftEffects.hasSideEffects(); - auto rightSideEffects = rightEffects.hasSideEffects(); - if (leftSideEffects && rightSideEffects) return nullptr; // both must execute + auto leftHasSideEffects = leftEffects.hasSideEffects(); + auto rightHasSideEffects = rightEffects.hasSideEffects(); + if (leftHasSideEffects && rightHasSideEffects) return nullptr; // both must execute // canonicalize with side effects, if any, happening on the left - if (rightSideEffects) { + if (rightHasSideEffects) { if (CostAnalyzer(left).cost < MIN_COST) return nullptr; // avoidable code is too cheap if (leftEffects.invalidates(rightEffects)) return nullptr; // cannot reorder std::swap(left, right); - } else if (leftSideEffects) { + } else if (leftHasSideEffects) { if (CostAnalyzer(right).cost < MIN_COST) return nullptr; // avoidable code is too cheap } else { // no side effects, reorder based on cost estimation diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index ac8df5711..1bc5ada94 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2073,7 +2073,7 @@ void WasmBinaryBuilder::visitBlock(Block *curr) { auto* item = expressionStack[i]; curr->list.push_back(item); if (i < end - 1) { - // stacky&unreachable code may introduce elements that need to be dropped in non-final positoins + // stacky&unreachable code may introduce elements that need to be dropped in non-final positions if (isConcreteWasmType(item->type)) { curr->list.back() = Builder(wasm).makeDrop(curr->list.back()); } -- cgit v1.2.3