[NFC] Rename `invalidates` to `observedBy` Clarify the role of `EffectAnalyzer::invalidates` by renaming it to `observedBy`. This method returns whether the current effects are observed by and therefore ordered with some other effects. Although the effect of this ordering is to "invalidate" various potential optimizations, nothing about the effects themselves are invalidated.
diff --git a/src/ir/effects.h b/src/ir/effects.h index 1a7f4af..9add1197 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h
@@ -245,8 +245,8 @@ // check if we break to anything external from ourselves bool hasExternalBreakTargets() const { return !breakTargets.empty(); } - // Checks if these effects would invalidate another set of effects (e.g., if - // we write, we invalidate someone that reads). + // Checks if these effects would be observed by another set of effects (e.g., + // if we write, we are observed by someone that reads). // // This assumes the things whose effects we are comparing will both execute, // at least if neither of them transfers control flow away. That is, we assume @@ -278,7 +278,7 @@ // example we can't reorder A and B if B traps, but in the first example we // can reorder them even if B traps (even if A has a global effect like a // global.set, since we assume B does not trap in traps-never-happen). - bool invalidates(const EffectAnalyzer& other) { + bool observedBy(const EffectAnalyzer& other) { if ((transfersControlFlow() && other.hasSideEffects()) || (other.transfersControlFlow() && hasSideEffects()) || ((writesMemory || calls) && other.accessesMemory()) || @@ -1164,14 +1164,14 @@ public: // Helpers - // See comment on invalidate() for the assumptions on the inputs here. + // See comment on observedBy() for the assumptions on the inputs here. static bool canReorder(const PassOptions& passOptions, Module& module, Expression* a, Expression* b) { EffectAnalyzer aEffects(passOptions, module, a); EffectAnalyzer bEffects(passOptions, module, b); - return !aEffects.invalidates(bEffects); + return !aEffects.observedBy(bEffects); } // C-API
diff --git a/src/ir/localize.h b/src/ir/localize.h index b36fe63..1208205 100644 --- a/src/ir/localize.h +++ b/src/ir/localize.h
@@ -133,7 +133,7 @@ // TODO: Avoid quadratic time here by accumulating effects and checking // vs the accumulation. for (Index j = 0; j < num; j++) { - if (j != i && effects[i].invalidates(effects[j])) { + if (j != i && effects[i].observedBy(effects[j])) { needLocal = true; break; }
diff --git a/src/passes/CodePushing.cpp b/src/passes/CodePushing.cpp index 65eedba..83ba14b 100644 --- a/src/passes/CodePushing.cpp +++ b/src/passes/CodePushing.cpp
@@ -204,7 +204,7 @@ auto* pushable = isPushable(list[i]); if (pushable) { const auto& effects = getPushableEffects(pushable); - if (cumulativeEffects.invalidates(effects)) { + if (cumulativeEffects.observedBy(effects)) { // we can't push this, so further pushables must pass it cumulativeEffects.mergeIn(effects); } else { @@ -354,7 +354,7 @@ const auto& effects = getPushableEffects(pushable); - if (cumulativeEffects.invalidates(effects)) { + if (cumulativeEffects.observedBy(effects)) { // This can't be moved forward. Add it to the things that are not // moving. cumulativeEffects.walk(list[i]);
diff --git a/src/passes/HeapStoreOptimization.cpp b/src/passes/HeapStoreOptimization.cpp index e4d3d7a..dc59a17 100644 --- a/src/passes/HeapStoreOptimization.cpp +++ b/src/passes/HeapStoreOptimization.cpp
@@ -181,7 +181,7 @@ // effects. auto firstEffects = effects(list[i]); auto secondEffects = effects(list[j]); - if (secondEffects.invalidates(firstEffects)) { + if (secondEffects.observedBy(firstEffects)) { return false; } @@ -241,7 +241,7 @@ if (!new_->isWithDefault()) { for (Index i = index + 1; i < operands.size(); i++) { auto operandEffects = effects(operands[i]); - if (operandEffects.invalidates(setValueEffects)) { + if (operandEffects.observedBy(setValueEffects)) { // TODO: we could use locals to reorder everything return false; } @@ -252,7 +252,7 @@ // if it exists. if (new_->desc) { auto descEffects = effects(new_->desc); - if (descEffects.invalidates(setValueEffects)) { + if (descEffects.observedBy(setValueEffects)) { // TODO: we could use locals to reorder everything return false; } @@ -264,7 +264,7 @@ // the optimization X' would happen first. ShallowEffectAnalyzer structNewEffects( getPassOptions(), *getModule(), new_); - if (structNewEffects.invalidates(setValueEffects)) { + if (structNewEffects.observedBy(setValueEffects)) { return false; }
diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp index 79831fb..6f038ec 100644 --- a/src/passes/LocalCSE.cpp +++ b/src/passes/LocalCSE.cpp
@@ -460,7 +460,7 @@ for (auto& kv : activeOriginals) { auto* original = kv.first; auto& originalInfo = kv.second; - if (effects.invalidates(originalInfo.effects)) { + if (effects.observedBy(originalInfo.effects)) { invalidated.push_back(original); } }
diff --git a/src/passes/LoopInvariantCodeMotion.cpp b/src/passes/LoopInvariantCodeMotion.cpp index 6c7aecb..25ac533 100644 --- a/src/passes/LoopInvariantCodeMotion.cpp +++ b/src/passes/LoopInvariantCodeMotion.cpp
@@ -123,7 +123,7 @@ // take into account global state like interacting loads and // stores. bool unsafeToMove = effects.writesGlobalState() || - effectsSoFar.invalidates(effects) || + effectsSoFar.observedBy(effects) || (effects.readsMutableGlobalState() && loopEffects.writesGlobalState()); // TODO: look into optimizing this with exceptions. for now, disallow
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp index 3013f67..d99696a 100644 --- a/src/passes/MergeBlocks.cpp +++ b/src/passes/MergeBlocks.cpp
@@ -512,12 +512,12 @@ EffectAnalyzer childEffects(getPassOptions(), *getModule(), child); if (dependency1 && *dependency1 && EffectAnalyzer(getPassOptions(), *getModule(), *dependency1) - .invalidates(childEffects)) { + .observedBy(childEffects)) { return outer; } if (dependency2 && *dependency2 && EffectAnalyzer(getPassOptions(), *getModule(), *dependency2) - .invalidates(childEffects)) { + .observedBy(childEffects)) { return outer; } } @@ -665,7 +665,7 @@ EffectAnalyzer blockChildEffects( getPassOptions(), *getModule(), blockChild); for (auto& effects : childEffects) { - if (blockChildEffects.invalidates(effects)) { + if (blockChildEffects.observedBy(effects)) { fail = true; break; }
diff --git a/src/passes/Monomorphize.cpp b/src/passes/Monomorphize.cpp index 9f02d00..9335088 100644 --- a/src/passes/Monomorphize.cpp +++ b/src/passes/Monomorphize.cpp
@@ -386,7 +386,7 @@ // (as described before, we want to move this past immovable code) and // reasons intrinsic to the expression itself that might prevent moving. ShallowEffectAnalyzer currEffects(options, wasm, curr); - if (currEffects.invalidates(nonMovingEffects) || + if (currEffects.observedBy(nonMovingEffects) || !canBeMovedIntoContext(curr, currEffects)) { immovable.insert(curr); currImmovable = true;
diff --git a/src/passes/OptimizeCasts.cpp b/src/passes/OptimizeCasts.cpp index aa7168f..5649c9d 100644 --- a/src/passes/OptimizeCasts.cpp +++ b/src/passes/OptimizeCasts.cpp
@@ -235,13 +235,13 @@ // if a cast can be moved past it. ShallowEffectAnalyzer currAnalyzer(options, *getModule(), curr); - if (testRefCast.invalidates(currAnalyzer)) { + if (testRefCast.observedBy(currAnalyzer)) { for (size_t i = 0; i < numLocals; i++) { flushRefCastResult(i, *getModule()); } } - if (testRefAs.invalidates(currAnalyzer)) { + if (testRefAs.observedBy(currAnalyzer)) { for (size_t i = 0; i < numLocals; i++) { flushRefAsResult(i, *getModule()); }
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 28d5ae6..ec2c396 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp
@@ -1585,7 +1585,7 @@ // trap we want to move. (We use a shallow effect analyzer since we // will only move the ref.as_non_null itself.) ShallowEffectAnalyzer movingEffects(options, *getModule(), input); - if (crossedEffects.invalidates(movingEffects)) { + if (crossedEffects.observedBy(movingEffects)) { return; } @@ -2563,7 +2563,7 @@ auto& options = getPassRunner()->options; EffectAnalyzer descEffects(options, *getModule(), curr->desc); ShallowEffectAnalyzer movingEffects(options, *getModule(), curr->ref); - if (descEffects.invalidates(movingEffects)) { + if (descEffects.observedBy(movingEffects)) { return; } } @@ -2816,7 +2816,7 @@ // originalRightEffects. auto originalRightEffects = effects(originalRight); auto rightEffects = effects(right); - if (originalRightEffects.invalidates(rightEffects)) { + if (originalRightEffects.observedBy(rightEffects)) { return false; } } @@ -2877,7 +2877,7 @@ } ShallowEffectAnalyzer parentEffects( getPassOptions(), *getModule(), call); - if (parentEffects.invalidates(childEffects)) { + if (parentEffects.observedBy(childEffects)) { return false; } // No effects are possible. @@ -3341,7 +3341,7 @@ // The condition is last, so we need a new local, and it may be a // bad idea to use a block like we do for an if. Do it only if we // can reorder - if (!condition.invalidates(value)) { + if (!condition.observedBy(value)) { return builder.makeSequence(builder.makeDrop(c), ifTrue); } } @@ -3659,7 +3659,7 @@ if (CostAnalyzer(left).cost < MIN_COST) { return nullptr; // avoidable code is too cheap } - if (leftEffects.invalidates(rightEffects)) { + if (leftEffects.observedBy(rightEffects)) { return nullptr; // cannot reorder } std::swap(left, right);
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp index 1a6d355..cd17499 100644 --- a/src/passes/RemoveUnusedBrs.cpp +++ b/src/passes/RemoveUnusedBrs.cpp
@@ -83,7 +83,7 @@ if (value.hasSideEffects()) { return false; } - return !EffectAnalyzer(options, wasm, ifCondition).invalidates(value); + return !EffectAnalyzer(options, wasm, ifCondition).observedBy(value); } // This leads to similar choices as LLVM does in some cases, by balancing the @@ -1421,7 +1421,7 @@ if (!valueEffects.hasUnremovableSideEffects()) { auto conditionEffects = EffectAnalyzer(passOptions, *getModule(), br->condition); - if (!conditionEffects.invalidates(valueEffects)) { + if (!conditionEffects.observedBy(valueEffects)) { // All conditions met, perform the update. drop->value = br->condition; } @@ -1621,7 +1621,7 @@ return nullptr; } EffectAnalyzer condition(passOptions, *getModule(), iff->condition); - if (condition.invalidates(ifTrue) || condition.invalidates(ifFalse)) { + if (condition.observedBy(ifTrue) || condition.observedBy(ifFalse)) { return nullptr; } auto* select = Builder(*getModule())
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index cf0b749..4b6d6ca 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp
@@ -303,7 +303,7 @@ // TODO: this is O(bad) std::vector<Index> invalidated; for (auto& [index, info] : sinkables) { - if (effects.invalidates(info.effects)) { + if (effects.observedBy(info.effects)) { invalidated.push_back(index); } } @@ -573,7 +573,7 @@ EffectAnalyzer value( this->getPassOptions(), *this->getModule(), set); *breakLocalSetPointer = set; - if (condition.invalidates(value)) { + if (condition.observedBy(value)) { // indeed, we can't do this, stop return; }
diff --git a/src/wasm2js.h b/src/wasm2js.h index 2ae0b85..93c00eb 100644 --- a/src/wasm2js.h +++ b/src/wasm2js.h
@@ -1337,7 +1337,7 @@ EffectAnalyzer targetEffects(parent->options, *module, curr->target); if (targetEffects.hasAnything()) { for (auto* operand : curr->operands) { - if (targetEffects.invalidates( + if (targetEffects.observedBy( EffectAnalyzer(parent->options, *module, operand))) { mustReorder = true; break; @@ -1969,8 +1969,8 @@ parent->options, *module, curr->condition); EffectAnalyzer ifTrueEffects(parent->options, *module, curr->ifTrue); EffectAnalyzer ifFalseEffects(parent->options, *module, curr->ifFalse); - if (conditionEffects.invalidates(ifTrueEffects) || - conditionEffects.invalidates(ifFalseEffects) || + if (conditionEffects.observedBy(ifTrueEffects) || + conditionEffects.observedBy(ifFalseEffects) || ifTrueEffects.hasSideEffects() || ifFalseEffects.hasSideEffects()) { useLocals = true; }