diff options
author | Alon Zakai <azakai@google.com> | 2024-05-15 12:48:37 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-15 12:48:37 -0700 |
commit | 2cc5e06549a4019eeed7303bfab32b95c32507bc (patch) | |
tree | c21b362b044e10713ddaee408ace41be2a6da854 /src/passes/OptimizeInstructions.cpp | |
parent | ae9499448c9221290f46eac26169db378fcd6997 (diff) | |
download | binaryen-2cc5e06549a4019eeed7303bfab32b95c32507bc.tar.gz binaryen-2cc5e06549a4019eeed7303bfab32b95c32507bc.tar.bz2 binaryen-2cc5e06549a4019eeed7303bfab32b95c32507bc.zip |
OptimizeInstructions: Add missing invalidation check in consecutive equality test (#6596)
This existed before #6495 but became noticeable there. We only looked at
the fallthrough values in the later part of areConsecutiveInputsEqual, but
there can be invalidation due to the non-fallthrough part:
(i32.add
(local.get $x)
(block
(local.set $x ..)
(local.get $x)
)
)
The set can cause the local.get to differ the second time. To fix this,
check if the non-fallthrough part invalidates the fallthrough (but only
on the right hand side).
Fixes #6593
Diffstat (limited to 'src/passes/OptimizeInstructions.cpp')
-rw-r--r-- | src/passes/OptimizeInstructions.cpp | 22 |
1 files changed, 22 insertions, 0 deletions
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 1a474be2c..81d0aef76 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -2501,11 +2501,33 @@ private: // Ignore extraneous things and compare them syntactically. We can also // look at the full fallthrough for both sides now. left = Properties::getFallthrough(left, passOptions, *getModule()); + auto* originalRight = right; right = Properties::getFallthrough(right, passOptions, *getModule()); if (!ExpressionAnalyzer::equal(left, right)) { return false; } + // We must also not have non-fallthrough effects that invalidate us, such as + // this situation: + // + // (local.get $x) + // (block + // (local.set $x ..) + // (local.get $x) + // ) + // + // The fallthroughs are identical, but the set may cause us to read a + // different value. + if (originalRight != right) { + // TODO: We could be more precise here and ignore right itself in + // originalRightEffects. + auto originalRightEffects = effects(originalRight); + auto rightEffects = effects(right); + if (originalRightEffects.invalidates(rightEffects)) { + return false; + } + } + // To be equal, they must also be known to return the same result // deterministically. return !Properties::isGenerative(left); |