diff options
author | Alon Zakai <azakai@google.com> | 2024-08-07 11:34:25 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-07 11:34:25 -0700 |
commit | dc87572a2d24f7dd42bec3af3005f09bc2c26af3 (patch) | |
tree | c5561d5d9206c9fdc6e4139b7abbbddf5d1aa574 | |
parent | 9163e0d155a85cc57883257b8cca6fcbb22fe7b6 (diff) | |
download | binaryen-dc87572a2d24f7dd42bec3af3005f09bc2c26af3.tar.gz binaryen-dc87572a2d24f7dd42bec3af3005f09bc2c26af3.tar.bz2 binaryen-dc87572a2d24f7dd42bec3af3005f09bc2c26af3.zip |
GTO: Remove minor optimization of avoiding ChildLocalizer sometimes (#6818)
The optimization is to only use ChildLocalizer, which moves children to
locals, if we actually have a reason to use it. It is simple enough to see if
we are removing fields with side effects here, and only call ChildLocalizer
if we are not. However, this will become much more complicated in a
subsequent PR which will reorder fields, which allows removing yet more
of them (without reordering, we can only remove fields at the end, if any
subtype needs the field).
This is a pretty minor optimization, as it avoids adding a few locals in the rare
case of struct.new operands having side effects. We run --gto at the
start of the pipeline, so later opts will clean that up anyhow. (Though, this
might make us a little less efficient, but the following PR will justify this
regression.)
-rw-r--r-- | src/passes/GlobalTypeOptimization.cpp | 28 | ||||
-rw-r--r-- | test/lit/passes/gto-removals.wast | 16 |
2 files changed, 18 insertions, 26 deletions
diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index e5cd4e410..536213561 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -22,7 +22,6 @@ // * Fields that are never read from can be removed entirely. // -#include "ir/effects.h" #include "ir/localize.h" #include "ir/ordering.h" #include "ir/struct-utils.h" @@ -348,26 +347,11 @@ struct GlobalTypeOptimization : public Pass { auto& operands = curr->operands; assert(indexesAfterRemoval.size() == operands.size()); - // Check for side effects in removed fields. If there are any, we must - // use locals to save the values (while keeping them in order). - bool useLocals = false; - for (Index i = 0; i < operands.size(); i++) { - auto newIndex = indexesAfterRemoval[i]; - if (newIndex == RemovedField && - EffectAnalyzer(getPassOptions(), *getModule(), operands[i]) - .hasUnremovableSideEffects()) { - useLocals = true; - break; - } - } - if (useLocals) { - auto* func = getFunction(); - if (!func) { - Fatal() << "TODO: side effects in removed fields in globals\n"; - } - ChildLocalizer localizer(curr, func, *getModule(), getPassOptions()); - replaceCurrent(localizer.getReplacement()); - } + // Localize things so that we can simply remove the operands we no + // longer need. + ChildLocalizer localizer( + curr, getFunction(), *getModule(), getPassOptions()); + replaceCurrent(localizer.getReplacement()); // Remove the unneeded operands. Index removed = 0; @@ -381,6 +365,8 @@ struct GlobalTypeOptimization : public Pass { } } operands.resize(operands.size() - removed); + // We should only get here if we did actual work. + assert(removed > 0); } void visitStructSet(StructSet* curr) { diff --git a/test/lit/passes/gto-removals.wast b/test/lit/passes/gto-removals.wast index 1d662f0ac..c5e148afa 100644 --- a/test/lit/passes/gto-removals.wast +++ b/test/lit/passes/gto-removals.wast @@ -589,17 +589,23 @@ ) ;; CHECK: (func $new-side-effect-in-kept (type $3) (param $any (ref any)) + ;; CHECK-NEXT: (local $1 i32) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.new $struct - ;; CHECK-NEXT: (call $helper0 - ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (block (result (ref $struct)) + ;; CHECK-NEXT: (local.set $1 + ;; CHECK-NEXT: (call $helper0 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.new $struct + ;; CHECK-NEXT: (local.get $1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $new-side-effect-in-kept (param $any (ref any)) - ;; Side effects appear in fields that we do *not* remove. In that case, - ;; we do not need to use locals. + ;; Side effects appear in fields that we do *not* remove. We do not need to + ;; use locals here, but for simplicity we do, and rely on later opts. (drop (struct.new $struct (call $helper0 (i32.const 0)) |