Skip to content

Commit 9656b8e

Browse files
authored
OptimizeInstructions: Improve areConsecutiveInputsEqualAndFoldable (#8355)
To check if we can fold `A, B` into `A`, we can just check if `B` has effects we can't remove. The old code used a utility that checks if `A` is also removable, but that is suboptimal, and that entire utility is not needed.
1 parent b8a38f8 commit 9656b8e

File tree

2 files changed

+17
-37
lines changed

2 files changed

+17
-37
lines changed

src/passes/OptimizeInstructions.cpp

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2826,31 +2826,9 @@ struct OptimizeInstructions
28262826
return !Properties::isGenerative(left);
28272827
}
28282828

2829-
// Similar to areConsecutiveInputsEqual() but also checks if we can remove
2830-
// them (but we do not assume the caller will always remove them).
2831-
bool areConsecutiveInputsEqualAndRemovable(Expression* left,
2832-
Expression* right) {
2833-
// First, check for side effects. If there are any, then we can't even
2834-
// assume things like local.get's of the same index being identical. (It is
2835-
// also ok to have removable side effects here, see the function
2836-
// description.)
2837-
auto& passOptions = getPassOptions();
2838-
if (EffectAnalyzer(passOptions, *getModule(), left)
2839-
.hasUnremovableSideEffects() ||
2840-
EffectAnalyzer(passOptions, *getModule(), right)
2841-
.hasUnremovableSideEffects()) {
2842-
return false;
2843-
}
2844-
2845-
return areConsecutiveInputsEqual(left, right);
2846-
}
2847-
28482829
// Check if two consecutive inputs to an instruction are equal and can also be
28492830
// folded into the first of the two (but we do not assume the caller will
2850-
// always fold them). This is similar to areConsecutiveInputsEqualAndRemovable
2851-
// but also identifies reads from the same local variable when the first of
2852-
// them is a "tee" operation and the second is a get (in which case, it is
2853-
// fine to remove the get, but not the tee).
2831+
// always fold them).
28542832
//
28552833
// The inputs here must be consecutive, but it is also ok to have code with no
28562834
// side effects at all in the middle. For example, a Const in between is ok.
@@ -2862,9 +2840,13 @@ struct OptimizeInstructions
28622840
return true;
28632841
}
28642842

2865-
// stronger property than we need - we can not only fold
2866-
// them but remove them entirely.
2867-
return areConsecutiveInputsEqualAndRemovable(left, right);
2843+
// To fold the right side into the left, it must have no effects.
2844+
if (EffectAnalyzer(getPassOptions(), *getModule(), right)
2845+
.hasUnremovableSideEffects()) {
2846+
return false;
2847+
}
2848+
2849+
return areConsecutiveInputsEqual(left, right);
28682850
}
28692851

28702852
// Canonicalizing the order of a symmetric binary helps us

test/lit/passes/optimize-instructions-mvp.wast

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16141,17 +16141,13 @@
1614116141
;; CHECK-NEXT: )
1614216142
;; CHECK-NEXT: )
1614316143
;; CHECK-NEXT: (drop
16144-
;; CHECK-NEXT: (select
16145-
;; CHECK-NEXT: (block (result i32)
16146-
;; CHECK-NEXT: (call $send-i32
16147-
;; CHECK-NEXT: (i32.const 1337)
16148-
;; CHECK-NEXT: )
16149-
;; CHECK-NEXT: (local.tee $x
16150-
;; CHECK-NEXT: (local.get $param)
16151-
;; CHECK-NEXT: )
16144+
;; CHECK-NEXT: (block (result i32)
16145+
;; CHECK-NEXT: (call $send-i32
16146+
;; CHECK-NEXT: (i32.const 1337)
16147+
;; CHECK-NEXT: )
16148+
;; CHECK-NEXT: (local.tee $x
16149+
;; CHECK-NEXT: (local.get $param)
1615216150
;; CHECK-NEXT: )
16153-
;; CHECK-NEXT: (i32.const 0)
16154-
;; CHECK-NEXT: (local.get $x)
1615516151
;; CHECK-NEXT: )
1615616152
;; CHECK-NEXT: )
1615716153
;; CHECK-NEXT: (drop
@@ -16179,7 +16175,9 @@
1617916175
)
1618016176
)
1618116177
)
16182-
;; Side effect on the ifTrue - same outcome, we cannot optimize yet.
16178+
;; Side effect on the ifTrue - we handle this case, as we can easily see
16179+
;; that those effects are not a problem. We keep those effects around, of
16180+
;; course.
1618316181
(drop
1618416182
(select
1618516183
(block (result i32)

0 commit comments

Comments
 (0)