From b2070673bd4f6e4eb9eb7707bc0e64c76e9ecef7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 7 Nov 2018 10:40:45 -0800 Subject: Fix a bug with (add (sub 0 X) Y) => (sub Y X) (#1727) We need to verify that the reordering is valid if there are side effects. Original bug report: https://groups.google.com/forum/?nomobile=true#!topic/emscripten-discuss/HIlGf8o2Ato --- src/ir/effects.h | 8 ++++++++ src/passes/MinifyImportsAndExports.cpp | 1 - src/passes/OptimizeInstructions.cpp | 21 +++++++++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/ir/effects.h b/src/ir/effects.h index 8919c6da4..8c95f463d 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -283,6 +283,14 @@ struct EffectAnalyzer : public PostWalker { isAtomic = true; } void visitUnreachable(Unreachable *curr) { branches = true; } + + // Helpers + + static bool canReorder(PassOptions& passOptions, Expression* a, Expression* b) { + EffectAnalyzer aEffects(passOptions, a); + EffectAnalyzer bEffects(passOptions, b); + return !aEffects.invalidates(bEffects); + } }; } // namespace wasm diff --git a/src/passes/MinifyImportsAndExports.cpp b/src/passes/MinifyImportsAndExports.cpp index 9cbc3b19a..f32859384 100644 --- a/src/passes/MinifyImportsAndExports.cpp +++ b/src/passes/MinifyImportsAndExports.cpp @@ -64,7 +64,6 @@ struct MinifyImportsAndExports : public Pass { reserved.insert("enum"); reserved.insert("void"); reserved.insert("this"); - reserved.insert("void"); reserved.insert("with"); validInitialChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_$"; diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 255ff4cbd..7cdc048f7 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -380,16 +380,32 @@ struct OptimizeInstructions : public WalkerPassleft->dynCast()) { if (sub->op == SubInt32) { if (auto* subZero = sub->left->dynCast()) { if (subZero->value.geti32() == 0) { - sub->left = binary->right; - return sub; + if (EffectAnalyzer::canReorder(getPassOptions(), sub->right, binary->right)) { + sub->left = binary->right; + return sub; + } } } } } + // The flip case is even easier, as no reordering occurs: + // (i32.add + // Y + // (i32.sub + // (i32.const 0) + // X + // ) + // ) + // => + // (i32.sub + // Y + // X + // ) if (auto* sub = binary->right->dynCast()) { if (sub->op == SubInt32) { if (auto* subZero = sub->left->dynCast()) { @@ -761,6 +777,7 @@ private: constant += value * mul; constants.push_back(c); } + return; } else if (auto* binary = curr->dynCast()) { if (binary->op == AddInt32) { seek(binary->left, mul); -- cgit v1.2.3