summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlon Zakai <azakai@google.com>2024-08-07 11:34:25 -0700
committerGitHub <noreply@github.com>2024-08-07 11:34:25 -0700
commitdc87572a2d24f7dd42bec3af3005f09bc2c26af3 (patch)
treec5561d5d9206c9fdc6e4139b7abbbddf5d1aa574
parent9163e0d155a85cc57883257b8cca6fcbb22fe7b6 (diff)
downloadbinaryen-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.cpp28
-rw-r--r--test/lit/passes/gto-removals.wast16
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))