Skip to content

Commit 2cc5e06

Browse files
authored
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
1 parent ae94994 commit 2cc5e06

File tree

2 files changed

+267
-17
lines changed

2 files changed

+267
-17
lines changed

src/passes/OptimizeInstructions.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,11 +2501,33 @@ struct OptimizeInstructions
25012501
// Ignore extraneous things and compare them syntactically. We can also
25022502
// look at the full fallthrough for both sides now.
25032503
left = Properties::getFallthrough(left, passOptions, *getModule());
2504+
auto* originalRight = right;
25042505
right = Properties::getFallthrough(right, passOptions, *getModule());
25052506
if (!ExpressionAnalyzer::equal(left, right)) {
25062507
return false;
25072508
}
25082509

2510+
// We must also not have non-fallthrough effects that invalidate us, such as
2511+
// this situation:
2512+
//
2513+
// (local.get $x)
2514+
// (block
2515+
// (local.set $x ..)
2516+
// (local.get $x)
2517+
// )
2518+
//
2519+
// The fallthroughs are identical, but the set may cause us to read a
2520+
// different value.
2521+
if (originalRight != right) {
2522+
// TODO: We could be more precise here and ignore right itself in
2523+
// originalRightEffects.
2524+
auto originalRightEffects = effects(originalRight);
2525+
auto rightEffects = effects(right);
2526+
if (originalRightEffects.invalidates(rightEffects)) {
2527+
return false;
2528+
}
2529+
}
2530+
25092531
// To be equal, they must also be known to return the same result
25102532
// deterministically.
25112533
return !Properties::isGenerative(left);

0 commit comments

Comments
 (0)