From 0c1fe2dffcf1e6530962d0d4aad7cde09ccee6cf Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 27 Feb 2026 18:07:55 -0800 Subject: [PATCH 1/2] [NFC] Use a bitset instead of bools in effects.h EffectAnalyzer tracks many effects that are just bools. Update it to use bits in a uint32_t instead. This not only makes EffectAnalyzer smaller now, but also allows us to add more effects in the future without making it larger. This change does not appear to have a meaningful effect on the performance of -O3 on large Wasm GC binaries, though. --- src/ir/drop.cpp | 2 +- src/ir/effects.cpp | 32 +- src/ir/effects.h | 669 +++++++++++++------------- src/passes/CodeFolding.cpp | 2 +- src/passes/GlobalEffects.cpp | 15 +- src/passes/GlobalTypeOptimization.cpp | 4 +- src/passes/LocalCSE.cpp | 4 +- src/passes/Monomorphize.cpp | 5 +- src/passes/SimplifyGlobals.cpp | 2 +- src/passes/SimplifyLocals.cpp | 2 +- src/passes/Unsubtyping.cpp | 3 +- src/passes/Vacuum.cpp | 8 +- 12 files changed, 368 insertions(+), 380 deletions(-) diff --git a/src/ir/drop.cpp b/src/ir/drop.cpp index f732e373003..4ec313c4f0e 100644 --- a/src/ir/drop.cpp +++ b/src/ir/drop.cpp @@ -40,7 +40,7 @@ Expression* getDroppedChildrenAndAppend(Expression* parent, ShallowEffectAnalyzer effects(options, wasm, parent); // Ignore a trap, as the unreachable replacement would trap too. if (last->is()) { - effects.trap = false; + effects.set(EffectAnalyzer::Bits::Trap, false); } keepParent = effects.hasUnremovableSideEffects(); } diff --git a/src/ir/effects.cpp b/src/ir/effects.cpp index af89dde2ca7..ac1645eced2 100644 --- a/src/ir/effects.cpp +++ b/src/ir/effects.cpp @@ -21,10 +21,10 @@ namespace std { std::ostream& operator<<(std::ostream& o, wasm::EffectAnalyzer& effects) { o << "EffectAnalyzer {\n"; - if (effects.branchesOut) { + if (effects.get(wasm::EffectAnalyzer::Bits::BranchesOut)) { o << "branchesOut\n"; } - if (effects.calls) { + if (effects.get(wasm::EffectAnalyzer::Bits::Calls)) { o << "calls\n"; } if (effects.localsRead.size()) { @@ -39,40 +39,40 @@ std::ostream& operator<<(std::ostream& o, wasm::EffectAnalyzer& effects) { if (effects.globalsWritten.size()) { o << "globalsWritten\n"; } - if (effects.readsMemory) { + if (effects.get(wasm::EffectAnalyzer::Bits::ReadsMemory)) { o << "readsMemory\n"; } - if (effects.writesMemory) { + if (effects.get(wasm::EffectAnalyzer::Bits::WritesMemory)) { o << "writesMemory\n"; } - if (effects.readsTable) { + if (effects.get(wasm::EffectAnalyzer::Bits::ReadsTable)) { o << "readsTable\n"; } - if (effects.writesTable) { + if (effects.get(wasm::EffectAnalyzer::Bits::WritesTable)) { o << "writesTable\n"; } - if (effects.readsMutableStruct) { + if (effects.get(wasm::EffectAnalyzer::Bits::ReadsMutableStruct)) { o << "readsMutableStruct\n"; } - if (effects.writesStruct) { + if (effects.get(wasm::EffectAnalyzer::Bits::WritesStruct)) { o << "writesStruct\n"; } - if (effects.readsArray) { + if (effects.get(wasm::EffectAnalyzer::Bits::ReadsArray)) { o << "readsArray\n"; } - if (effects.writesArray) { + if (effects.get(wasm::EffectAnalyzer::Bits::WritesArray)) { o << "writesArray\n"; } - if (effects.trap) { + if (effects.traps()) { o << "trap\n"; } - if (effects.implicitTrap) { + if (effects.get(wasm::EffectAnalyzer::Bits::ImplicitTrap)) { o << "implicitTrap\n"; } - if (effects.isAtomic) { + if (effects.get(wasm::EffectAnalyzer::Bits::IsAtomic)) { o << "isAtomic\n"; } - if (effects.throws_) { + if (effects.get(wasm::EffectAnalyzer::Bits::Throws)) { o << "throws_\n"; } if (effects.tryDepth) { @@ -81,10 +81,10 @@ std::ostream& operator<<(std::ostream& o, wasm::EffectAnalyzer& effects) { if (effects.catchDepth) { o << "catchDepth\n"; } - if (effects.danglingPop) { + if (effects.get(wasm::EffectAnalyzer::Bits::DanglingPop)) { o << "danglingPop\n"; } - if (effects.mayNotReturn) { + if (effects.get(wasm::EffectAnalyzer::Bits::MayNotReturn)) { o << "mayNotReturn\n"; } if (effects.hasReturnCallThrow) { diff --git a/src/ir/effects.h b/src/ir/effects.h index 15786aab89e..4cbe6aa9af4 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -79,12 +79,12 @@ class EffectAnalyzer { // Effects of return-called functions will be visible to the caller. if (hasReturnCallThrow) { - throws_ = true; + set(Bits::Throws); } // We can ignore branching out of the function body - this can only be // a return, and that is only noticeable in the function, not outside. - branchesOut = false; + set(Bits::BranchesOut, false); // When the function exits, changes to locals cannot be noticed any more. localsWritten.clear(); @@ -93,53 +93,11 @@ class EffectAnalyzer { // Core effect tracking - // Definitely branches out of this expression, or does a return, etc. - // breakTargets tracks individual targets, which we may eventually see are - // internal, while this is set when we see something that will definitely - // not be internal, or is otherwise special like an infinite loop (which - // does not technically branch "out", but it does break the normal assumption - // of control flow proceeding normally). - bool branchesOut = false; - bool calls = false; std::set localsRead; std::set localsWritten; std::set mutableGlobalsRead; std::set globalsWritten; - bool readsMemory = false; - bool writesMemory = false; - bool readsTable = false; - bool writesTable = false; - // TODO: More specific type-based alias analysis, and not just at the - // struct/array level. - bool readsMutableStruct = false; - bool writesStruct = false; - bool readsArray = false; - bool writesArray = false; - // A trap, either from an unreachable instruction, or from an implicit trap - // that we do not ignore (see below). - // - // Note that we ignore trap differences, so it is ok to reorder traps with - // each other, but it is not ok to remove them or reorder them with other - // effects in a noticeable way. - // - // Note also that we ignore *optional* runtime-specific traps: we only - // consider as trapping something that will trap in *all* VMs, and *all* the - // time. For example, a single allocation might trap in a VM in a particular - // execution, if it happens to run out of memory just there, but that is not - // enough for us to mark it as having a trap effect. (Note that not marking - // each allocation as possibly trapping has the nice benefit of making it - // possible to eliminate an allocation whose result is not captured.) OTOH, we - // *do* mark a potentially infinite number of allocations as trapping, as all - // VMs would trap eventually, and the same for potentially infinite recursion, - // etc. - bool trap = false; - // A trap from an instruction like a load or div/rem, which may trap on corner - // cases. If we do not ignore implicit traps then these are counted as a trap. - bool implicitTrap = false; - // An atomic load/store/RMW/Cmpxchg or an operator that has a defined ordering - // wrt atomics (e.g. memory.grow) - bool isAtomic = false; - bool throws_ = false; + // The nested depth of try-catch_all. If an instruction that may throw is // inside an inner try-catch_all, we don't mark it as 'throws_', because it // will be caught by an inner catch_all. We only count 'try's with a @@ -148,12 +106,90 @@ class EffectAnalyzer { size_t tryDepth = 0; // The nested depth of catch. This is necessary to track danglng pops. size_t catchDepth = 0; - // If this expression contains 'pop's that are not enclosed in 'catch' body. - // For example, (drop (pop i32)) should set this to true. - bool danglingPop = false; - // Whether this code may "hang" and not eventually complete. An infinite loop, - // or a continuation that is never continued, are examples of that. - bool mayNotReturn = false; + + struct Bits { + enum : uint32_t { + None = 0, + // Definitely branches out of this expression, or does a return, etc. + // breakTargets tracks individual targets, which we may eventually see are + // internal, while this is set when we see something that will definitely + // not be internal, or is otherwise special like an infinite loop (which + // does not technically branch "out", but it does break the normal + // assumption of control flow proceeding normally). + BranchesOut = 1 << 0, + Calls = 1 << 1, + ReadsMemory = 1 << 2, + WritesMemory = 1 << 3, + ReadsTable = 1 << 4, + WritesTable = 1 << 5, + // TODO: More specific type-based alias analysis, and not just at the + // struct/array level. + ReadsMutableStruct = 1 << 6, + WritesStruct = 1 << 7, + ReadsArray = 1 << 8, + WritesArray = 1 << 9, + // A trap, either from an unreachable instruction, or from an implicit + // trap that we do not ignore (see below). + // + // Note that we ignore trap differences, so it is ok to reorder traps with + // each other, but it is not ok to remove them or reorder them with other + // effects in a noticeable way. + // + // Note also that we ignore *optional* runtime-specific traps: we only + // consider as trapping something that will trap in *all* VMs, and *all* + // the time. For example, a single allocation might trap in a VM in a + // particular execution, if it happens to run out of memory just there, + // but that is not enough for us to mark it as having a trap effect. (Note + // that not marking each allocation as possibly trapping has the nice + // benefit of making it possible to eliminate an allocation whose result + // is not captured.) OTOH, we *do* mark a potentially infinite number of + // allocations as trapping, as all VMs would trap eventually, and the same + // for potentially infinite recursion, etc. + Trap = 1 << 10, + // A trap from an instruction like a load or div/rem, which may trap on + // corner cases. If we do not ignore implicit traps then these are counted + // as a trap. + ImplicitTrap = 1 << 11, + // An atomic load/store/RMW/Cmpxchg or an operator that has a defined + // ordering wrt atomics (e.g. memory.grow) + IsAtomic = 1 << 12, + Throws = 1 << 13, + + // If this expression contains 'pop's that are not enclosed in 'catch' + // body. For example, (drop (pop i32)) should set this to true. + DanglingPop = 1 << 14, + // Whether this code may "hang" and not eventually complete. An infinite + // loop, or a continuation that is never continued, are examples of that. + MayNotReturn = 1 << 15, + + // Since return calls return out of the body of the function before + // performing their call, they are indistinguishable from normal returns + // from the perspective of their surrounding code, and the return-callee's + // effects only become visible when considering the effects of the whole + // function containing the return call. To model this correctly, stash the + // callee's effects on the side and only merge them in after walking a + // full function body. + // + // We currently do this stashing only for the throw effect, but in + // principle we could do it for all effects if it made a difference. + // (Only throw is noticeable now because the only thing that can change + // between doing the call here and doing it outside at the function exit + // is the scoping of try-catch blocks. If future wasm scoping additions + // are added, we may need more here.) + HasReturnCallThrow = 1 << 16, + }; + }; + uint32_t effectBits = Bits::None; + + void set(uint32_t bits, bool value = true) { + if (value) { + effectBits |= bits; + } else { + effectBits &= ~bits; + } + } + + bool get(uint32_t bits) const { return (effectBits & bits) != 0; } // Since return calls return out of the body of the function before performing // their call, they are indistinguishable from normal returns from the @@ -179,13 +215,21 @@ class EffectAnalyzer { bool accessesMutableGlobal() const { return globalsWritten.size() + mutableGlobalsRead.size() > 0; } - bool accessesMemory() const { return calls || readsMemory || writesMemory; } - bool accessesTable() const { return calls || readsTable || writesTable; } + bool accessesMemory() const { + return get(Bits::Calls | Bits::ReadsMemory | Bits::WritesMemory); + } + bool accessesTable() const { + return get(Bits::Calls | Bits::ReadsTable | Bits::WritesTable); + } bool accessesMutableStruct() const { - return calls || readsMutableStruct || writesStruct; + return get(Bits::Calls | Bits::ReadsMutableStruct | Bits::WritesStruct); + } + bool accessesArray() const { + return get(Bits::Calls | Bits::ReadsArray | Bits::WritesArray); } - bool accessesArray() const { return calls || readsArray || writesArray; } - bool throws() const { return throws_ || !delegateTargets.empty(); } + bool throws() const { return get(Bits::Throws) || !delegateTargets.empty(); } + bool traps() const { return get(Bits::Trap); } + // Check whether this may transfer control flow to somewhere outside of this // expression (aside from just flowing out normally). That includes a break // or a throw (if the throw is not known to be caught inside this expression; @@ -194,25 +238,27 @@ class EffectAnalyzer { // caught in the function at all, which would mean control flow cannot be // transferred inside the function, but this expression does not know that). bool transfersControlFlow() const { - return branchesOut || throws() || hasExternalBreakTargets(); + return get(Bits::BranchesOut) || throws() || hasExternalBreakTargets(); } // Changes something in globally-stored state. bool writesGlobalState() const { - return globalsWritten.size() || writesMemory || writesTable || - writesStruct || writesArray || isAtomic || calls; + return get(Bits::WritesMemory | Bits::WritesTable | Bits::WritesStruct | + Bits::WritesArray | Bits::IsAtomic | Bits::Calls) || + globalsWritten.size(); } bool readsMutableGlobalState() const { - return mutableGlobalsRead.size() || readsMemory || readsTable || - readsMutableStruct || readsArray || isAtomic || calls; + return get(Bits::ReadsMemory | Bits::ReadsTable | Bits::ReadsMutableStruct | + Bits::ReadsArray | Bits::IsAtomic | Bits::Calls) || + mutableGlobalsRead.size(); } bool hasNonTrapSideEffects() const { - return localsWritten.size() > 0 || danglingPop || writesGlobalState() || - throws() || transfersControlFlow() || mayNotReturn; + return get(Bits::DanglingPop | Bits::MayNotReturn) || writesGlobalState() || + throws() || transfersControlFlow() || localsWritten.size() > 0; } - bool hasSideEffects() const { return trap || hasNonTrapSideEffects(); } + bool hasSideEffects() const { return hasNonTrapSideEffects() || traps(); } // Check if there are side effects, and they are of a kind that cannot be // removed by optimization passes. @@ -235,7 +281,7 @@ class EffectAnalyzer { // TODO: Go through the optimizer and use this in all places that do not move // code around. bool hasUnremovableSideEffects() const { - return hasNonTrapSideEffects() || (trap && !trapsNeverHappen); + return hasNonTrapSideEffects() || (traps() && !trapsNeverHappen); } bool hasAnything() const { @@ -281,21 +327,23 @@ class EffectAnalyzer { bool invalidates(const EffectAnalyzer& other) { if ((transfersControlFlow() && other.hasSideEffects()) || (other.transfersControlFlow() && hasSideEffects()) || - ((writesMemory || calls) && other.accessesMemory()) || - ((other.writesMemory || other.calls) && accessesMemory()) || - ((writesTable || calls) && other.accessesTable()) || - ((other.writesTable || other.calls) && accessesTable()) || - ((writesStruct || calls) && other.accessesMutableStruct()) || - ((other.writesStruct || other.calls) && accessesMutableStruct()) || - ((writesArray || calls) && other.accessesArray()) || - ((other.writesArray || other.calls) && accessesArray()) || - (danglingPop || other.danglingPop)) { + (get(Bits::WritesMemory | Bits::Calls) && other.accessesMemory()) || + (other.get(Bits::WritesMemory | Bits::Calls) && accessesMemory()) || + (get(Bits::WritesTable | Bits::Calls) && other.accessesTable()) || + (other.get(Bits::WritesTable | Bits::Calls) && accessesTable()) || + (get(Bits::WritesStruct | Bits::Calls) && + other.accessesMutableStruct()) || + (other.get(Bits::WritesStruct | Bits::Calls) && + accessesMutableStruct()) || + (get(Bits::WritesArray | Bits::Calls) && other.accessesArray()) || + (other.get(Bits::WritesArray | Bits::Calls) && accessesArray()) || + get(Bits::DanglingPop) || other.get(Bits::DanglingPop)) { return true; } // All atomics are sequentially consistent for now, and ordered wrt other // memory references. - if ((isAtomic && other.accessesMemory()) || - (other.isAtomic && accessesMemory())) { + if ((get(Bits::IsAtomic) && other.accessesMemory()) || + (other.get(Bits::IsAtomic) && accessesMemory())) { return true; } for (auto local : localsWritten) { @@ -308,8 +356,8 @@ class EffectAnalyzer { return true; } } - if ((other.calls && accessesMutableGlobal()) || - (calls && other.accessesMutableGlobal())) { + if ((other.get(Bits::Calls) && accessesMutableGlobal()) || + (get(Bits::Calls) && other.accessesMutableGlobal())) { return true; } for (auto global : globalsWritten) { @@ -328,7 +376,7 @@ class EffectAnalyzer { // function, so transfersControlFlow would be true) - while we allow the // reordering of traps with each other, we do not reorder exceptions with // anything. - assert(!((trap && other.throws()) || (throws() && other.trap))); + assert(!((traps() && other.throws()) || (throws() && other.traps()))); // We can't reorder an implicit trap in a way that could alter what global // state is modified. However, in trapsNeverHappen mode we assume traps do // not occur in practice, which lets us ignore this, at least in the case @@ -337,8 +385,8 @@ class EffectAnalyzer { // need to do is check for such transfers in them. if (!trapsNeverHappen || transfersControlFlow() || other.transfersControlFlow()) { - if ((trap && other.writesGlobalState()) || - (other.trap && writesGlobalState())) { + if ((traps() && other.writesGlobalState()) || + (other.traps() && writesGlobalState())) { return true; } } @@ -346,23 +394,7 @@ class EffectAnalyzer { } void mergeIn(const EffectAnalyzer& other) { - branchesOut = branchesOut || other.branchesOut; - calls = calls || other.calls; - readsMemory = readsMemory || other.readsMemory; - writesMemory = writesMemory || other.writesMemory; - readsTable = readsTable || other.readsTable; - writesTable = writesTable || other.writesTable; - readsMutableStruct = readsMutableStruct || other.readsMutableStruct; - writesStruct = writesStruct || other.writesStruct; - readsArray = readsArray || other.readsArray; - writesArray = writesArray || other.writesArray; - trap = trap || other.trap; - implicitTrap = implicitTrap || other.implicitTrap; - trapsNeverHappen = trapsNeverHappen || other.trapsNeverHappen; - isAtomic = isAtomic || other.isAtomic; - throws_ = throws_ || other.throws_; - danglingPop = danglingPop || other.danglingPop; - mayNotReturn = mayNotReturn || other.mayNotReturn; + set(other.effectBits); for (auto i : other.localsRead) { localsRead.insert(i); } @@ -388,7 +420,7 @@ class EffectAnalyzer { // the children, i.e., loops bool checkPre(Expression* curr) { if (curr->is()) { - branchesOut = true; + set(Bits::BranchesOut); return true; } return false; @@ -397,7 +429,7 @@ class EffectAnalyzer { bool checkPost(Expression* curr) { visit(curr); if (curr->is()) { - branchesOut = true; + set(Bits::BranchesOut); } return hasAnything(); } @@ -410,8 +442,12 @@ class EffectAnalyzer { : public PostWalker> { EffectAnalyzer& parent; + uint32_t& effectBits; - InternalAnalyzer(EffectAnalyzer& parent) : parent(parent) {} + InternalAnalyzer(EffectAnalyzer& parent) + : parent(parent), effectBits(parent.effectBits) {} + + void set(uint32_t bit, bool value = true) { parent.set(bit, value); } static void scan(InternalAnalyzer* self, Expression** currp) { Expression* curr = *currp; @@ -460,7 +496,7 @@ class EffectAnalyzer { if (curr->name.is()) { if (self->parent.delegateTargets.count(curr->name) && self->parent.tryDepth == 0) { - self->parent.throws_ = true; + self->set(Bits::Throws); } self->parent.delegateTargets.erase(curr->name); } @@ -504,7 +540,7 @@ class EffectAnalyzer { void visitIf(If* curr) {} void visitLoop(Loop* curr) { if (curr->name.is() && parent.breakTargets.erase(curr->name) > 0) { - parent.mayNotReturn = true; + set(Bits::MayNotReturn); } } void visitBreak(Break* curr) { parent.breakTargets.insert(curr->name); } @@ -532,7 +568,7 @@ class EffectAnalyzer { } if (curr->isReturn) { - parent.branchesOut = true; + set(Bits::BranchesOut); // When EH is enabled, any call can throw. if (parent.features.hasExceptionHandling() && (!targetEffects || targetEffects->throws())) { @@ -542,16 +578,17 @@ class EffectAnalyzer { if (targetEffects) { // We have effect information for this call target, and can just use - // that. The one change we may want to make is to remove throws_, if the + // that. The one change we may want to make is to remove throws, if the // target function throws and we know that will be caught anyhow, the // same as the code below for the general path. We can always filter out // throws for return calls because they are already more precisely - // captured by `branchesOut`, which models the return, and + // captured by `BranchesOut`, which models the return, and // `hasReturnCallThrow`, which models the throw that will happen after // the return. - if (targetEffects->throws_ && (parent.tryDepth > 0 || curr->isReturn)) { + if (targetEffects->get(Bits::Throws) && + (parent.tryDepth > 0 || curr->isReturn)) { auto filteredEffects = *targetEffects; - filteredEffects.throws_ = false; + filteredEffects.set(Bits::Throws, false); parent.mergeIn(filteredEffects); } else { // Just merge in all the effects. @@ -560,26 +597,26 @@ class EffectAnalyzer { return; } - parent.calls = true; + set(Bits::Calls); // When EH is enabled, any call can throw. Skip this for return calls // because the throw is already more precisely captured by the combination - // of `hasReturnCallThrow` and `branchesOut`. + // of `HasReturnCallThrow` and `BranchesOut`. if (parent.features.hasExceptionHandling() && parent.tryDepth == 0 && !curr->isReturn) { - parent.throws_ = true; + set(Bits::Throws); } } void visitCallIndirect(CallIndirect* curr) { - parent.calls = true; + set(Bits::Calls); if (curr->isReturn) { - parent.branchesOut = true; + set(Bits::BranchesOut); if (parent.features.hasExceptionHandling()) { parent.hasReturnCallThrow = true; } } if (parent.features.hasExceptionHandling() && (parent.tryDepth == 0 && !curr->isReturn)) { - parent.throws_ = true; + set(Bits::Throws); } } void visitLocalGet(LocalGet* curr) { @@ -597,57 +634,51 @@ class EffectAnalyzer { parent.globalsWritten.insert(curr->name); } void visitLoad(Load* curr) { - parent.readsMemory = true; - parent.isAtomic |= curr->isAtomic(); - parent.implicitTrap = true; + set(Bits::ReadsMemory); + if (curr->isAtomic()) { + set(Bits::IsAtomic); + } + set(Bits::ImplicitTrap); } void visitStore(Store* curr) { - parent.writesMemory = true; - parent.isAtomic |= curr->isAtomic(); - parent.implicitTrap = true; + set(Bits::WritesMemory); + if (curr->isAtomic()) { + set(Bits::IsAtomic); + } + set(Bits::ImplicitTrap); } void visitAtomicRMW(AtomicRMW* curr) { - parent.readsMemory = true; - parent.writesMemory = true; - parent.isAtomic = true; - parent.implicitTrap = true; + set(Bits::ReadsMemory | Bits::WritesMemory | Bits::IsAtomic | + Bits::ImplicitTrap); } void visitAtomicCmpxchg(AtomicCmpxchg* curr) { - parent.readsMemory = true; - parent.writesMemory = true; - parent.isAtomic = true; - parent.implicitTrap = true; + set(Bits::ReadsMemory | Bits::WritesMemory | Bits::IsAtomic | + Bits::ImplicitTrap); } void visitAtomicWait(AtomicWait* curr) { - parent.readsMemory = true; // AtomicWait doesn't strictly write memory, but it does modify the // waiters list associated with the specified address, which we can think // of as a write. - parent.writesMemory = true; - parent.isAtomic = true; - parent.implicitTrap = true; + set(Bits::ReadsMemory | Bits::WritesMemory | Bits::IsAtomic | + Bits::ImplicitTrap); } void visitAtomicNotify(AtomicNotify* curr) { // AtomicNotify doesn't strictly write memory, but it does modify the // waiters list associated with the specified address, which we can think // of as a write. - parent.readsMemory = true; - parent.writesMemory = true; - parent.isAtomic = true; - parent.implicitTrap = true; + set(Bits::ReadsMemory | Bits::WritesMemory | Bits::IsAtomic | + Bits::ImplicitTrap); } void visitAtomicFence(AtomicFence* curr) { // AtomicFence should not be reordered with any memory operations, so we // set these to true. - parent.readsMemory = true; - parent.writesMemory = true; - parent.isAtomic = true; + set(Bits::ReadsMemory | Bits::WritesMemory | Bits::IsAtomic); } void visitPause(Pause* curr) { // We don't want this to be moved out of loops, but it doesn't otherwises // matter much how it gets reordered. Say we transfer control as a coarse // approximation of this. - parent.branchesOut = true; + set(Bits::BranchesOut); } void visitSIMDExtract(SIMDExtract* curr) {} void visitSIMDReplace(SIMDReplace* curr) {} @@ -655,36 +686,30 @@ class EffectAnalyzer { void visitSIMDTernary(SIMDTernary* curr) {} void visitSIMDShift(SIMDShift* curr) {} void visitSIMDLoad(SIMDLoad* curr) { - parent.readsMemory = true; - parent.implicitTrap = true; + set(Bits::ReadsMemory | Bits::ImplicitTrap); } void visitSIMDLoadStoreLane(SIMDLoadStoreLane* curr) { if (curr->isLoad()) { - parent.readsMemory = true; + set(Bits::ReadsMemory); } else { - parent.writesMemory = true; + set(Bits::WritesMemory); } - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } void visitMemoryInit(MemoryInit* curr) { - parent.writesMemory = true; - parent.implicitTrap = true; + set(Bits::WritesMemory | Bits::ImplicitTrap); } void visitDataDrop(DataDrop* curr) { // data.drop does not actually write memory, but it does alter the size of // a segment, which can be noticeable later by memory.init, so we need to // mark it as having a global side effect of some kind. - parent.writesMemory = true; - parent.implicitTrap = true; + set(Bits::WritesMemory | Bits::ImplicitTrap); } void visitMemoryCopy(MemoryCopy* curr) { - parent.readsMemory = true; - parent.writesMemory = true; - parent.implicitTrap = true; + set(Bits::ReadsMemory | Bits::WritesMemory | Bits::ImplicitTrap); } void visitMemoryFill(MemoryFill* curr) { - parent.writesMemory = true; - parent.implicitTrap = true; + set(Bits::WritesMemory | Bits::ImplicitTrap); } void visitConst(Const* curr) {} void visitUnary(Unary* curr) { @@ -697,7 +722,7 @@ class EffectAnalyzer { case TruncSFloat64ToInt64: case TruncUFloat64ToInt32: case TruncUFloat64ToInt64: { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); break; } default: { @@ -720,13 +745,13 @@ class EffectAnalyzer { // divider. if (auto* c = curr->right->dynCast()) { if (c->value.isZero()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } else if ((curr->op == DivSInt32 || curr->op == DivSInt64) && c->value.getInteger() == -1LL) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } } else { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } break; } @@ -736,59 +761,49 @@ class EffectAnalyzer { } void visitSelect(Select* curr) {} void visitDrop(Drop* curr) {} - void visitReturn(Return* curr) { parent.branchesOut = true; } + void visitReturn(Return* curr) { set(Bits::BranchesOut); } void visitMemorySize(MemorySize* curr) { // memory.size accesses the size of the memory, and thus can be modeled as - // reading memory - parent.readsMemory = true; - // Atomics are sequentially consistent with memory.size. - parent.isAtomic = true; + // reading memory. It is also sequentially consistent with other atomic + // operations. + set(Bits::ReadsMemory | Bits::IsAtomic); } void visitMemoryGrow(MemoryGrow* curr) { - // TODO: find out if calls is necessary here - parent.calls = true; // memory.grow technically does a read-modify-write operation on the // memory size in the successful case, modifying the set of valid - // addresses, and just a read operation in the failure case - parent.readsMemory = true; - parent.writesMemory = true; - // Atomics are also sequentially consistent with memory.grow. - parent.isAtomic = true; + // addresses, and just a read operation in the failure case. It is + // also sequentially consistent with other atomic operations. + // TODO: find out if calls is necessary here + set(Bits::Calls | Bits::ReadsMemory | Bits::WritesMemory | + Bits::IsAtomic); } void visitRefNull(RefNull* curr) {} void visitRefIsNull(RefIsNull* curr) {} void visitRefFunc(RefFunc* curr) {} void visitRefEq(RefEq* curr) {} void visitTableGet(TableGet* curr) { - parent.readsTable = true; - parent.implicitTrap = true; + set(Bits::ReadsTable | Bits::ImplicitTrap); } void visitTableSet(TableSet* curr) { - parent.writesTable = true; - parent.implicitTrap = true; + set(Bits::WritesTable | Bits::ImplicitTrap); } - void visitTableSize(TableSize* curr) { parent.readsTable = true; } + void visitTableSize(TableSize* curr) { set(Bits::ReadsTable); } void visitTableGrow(TableGrow* curr) { // table.grow technically does a read-modify-write operation on the // table size in the successful case, modifying the set of valid // indices, and just a read operation in the failure case - parent.readsTable = true; - parent.writesTable = true; + set(Bits::ReadsTable | Bits::WritesTable); } void visitTableFill(TableFill* curr) { - parent.writesTable = true; - parent.implicitTrap = true; + set(Bits::WritesTable | Bits::ImplicitTrap); } void visitTableCopy(TableCopy* curr) { - parent.readsTable = true; - parent.writesTable = true; - parent.implicitTrap = true; + set(Bits::ReadsTable | Bits::WritesTable | Bits::ImplicitTrap); } void visitTableInit(TableInit* curr) { - parent.writesTable = true; - parent.implicitTrap = true; + set(Bits::WritesTable | Bits::ImplicitTrap); } - void visitElemDrop(ElemDrop* curr) { parent.writesTable = true; } + void visitElemDrop(ElemDrop* curr) { set(Bits::WritesTable); } void visitTry(Try* curr) { if (curr->delegateTarget.is()) { parent.delegateTargets.insert(curr->delegateTarget); @@ -801,26 +816,26 @@ class EffectAnalyzer { } void visitThrow(Throw* curr) { if (parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } } void visitRethrow(Rethrow* curr) { if (parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } } void visitThrowRef(ThrowRef* curr) { if (parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } - // traps when the arg is null - parent.implicitTrap = true; + // Traps when the arg is null. + set(Bits::ImplicitTrap); } void visitNop(Nop* curr) {} - void visitUnreachable(Unreachable* curr) { parent.trap = true; } + void visitUnreachable(Unreachable* curr) { set(Bits::Trap); } void visitPop(Pop* curr) { if (parent.catchDepth == 0) { - parent.danglingPop = true; + set(Bits::DanglingPop); } } void visitTupleMake(TupleMake* curr) {} @@ -829,29 +844,29 @@ class EffectAnalyzer { void visitI31Get(I31Get* curr) { // traps when the ref is null if (curr->i31->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } } void visitCallRef(CallRef* curr) { if (curr->isReturn) { - parent.branchesOut = true; + set(Bits::BranchesOut); if (parent.features.hasExceptionHandling()) { parent.hasReturnCallThrow = true; } } if (curr->target->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } // traps when the call target is null if (curr->target->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } - parent.calls = true; + set(Bits::Calls); if (parent.features.hasExceptionHandling() && (parent.tryDepth == 0 && !curr->isReturn)) { - parent.throws_ = true; + set(Bits::Throws); } } void visitRefTest(RefTest* curr) {} @@ -859,20 +874,20 @@ class EffectAnalyzer { if (desc) { // Traps when the descriptor is null. if (desc->type.isNull()) { - parent.trap = true; + set(Bits::Trap); } else if (desc->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } } } void visitRefCast(RefCast* curr) { // Traps if the cast fails. - parent.implicitTrap = true; + set(Bits::ImplicitTrap); maybeHandleDescriptor(curr->desc); } void visitRefGetDesc(RefGetDesc* curr) { // Traps if the ref is null. - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } void visitBrOn(BrOn* curr) { parent.breakTargets.insert(curr->name); @@ -884,85 +899,77 @@ class EffectAnalyzer { return; } if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } if (curr->ref->type.getHeapType() .getStruct() .fields[curr->index] .mutable_ == Mutable) { - parent.readsMutableStruct = true; + set(Bits::ReadsMutableStruct); } // traps when the arg is null if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } switch (curr->order) { case MemoryOrder::Unordered: break; case MemoryOrder::SeqCst: // Synchronizes with other threads. - parent.isAtomic = true; + set(Bits::IsAtomic); break; case MemoryOrder::AcqRel: // Only synchronizes if other threads can read the field. - parent.isAtomic = curr->ref->type.getHeapType().isShared(); + if (curr->ref->type.getHeapType().isShared()) { + set(Bits::IsAtomic); + } break; } } void visitStructSet(StructSet* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.writesStruct = true; + set(Bits::WritesStruct); // traps when the arg is null if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } if (curr->order != MemoryOrder::Unordered) { - parent.isAtomic = true; + set(Bits::IsAtomic); } } void visitStructRMW(StructRMW* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.readsMutableStruct = true; - parent.writesStruct = true; + assert(curr->order != MemoryOrder::Unordered); + set(Bits::ReadsMutableStruct | Bits::WritesStruct | Bits::IsAtomic); if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } - assert(curr->order != MemoryOrder::Unordered); - parent.isAtomic = true; } void visitStructCmpxchg(StructCmpxchg* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.readsMutableStruct = true; - parent.writesStruct = true; + assert(curr->order != MemoryOrder::Unordered); + set(Bits::ReadsMutableStruct | Bits::WritesStruct | Bits::IsAtomic); if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } - assert(curr->order != MemoryOrder::Unordered); - parent.isAtomic = true; } void visitStructWait(StructWait* curr) { - parent.isAtomic = true; - - // If the ref is null. - parent.implicitTrap = true; - - // If the timeout is negative and no-one wakes us. - parent.mayNotReturn = true; - // struct.wait mutates an opaque waiter queue which isn't visible in user // code. Model this as a struct write which prevents reorderings (since // isAtomic == true). - parent.writesStruct = true; + // TODO: Implicit trap only if ref is nullable. + set(Bits::IsAtomic | Bits::ImplicitTrap | Bits::MayNotReturn | + Bits::WritesStruct); if (curr->ref->type == Type::unreachable) { return; @@ -978,116 +985,104 @@ class EffectAnalyzer { .fields.at(curr->index) .packedType == Field::PackedType::WaitQueue); - parent.readsMutableStruct |= curr->ref->type.getHeapType() - .getStruct() - .fields.at(curr->index) - .mutable_ == Mutable; + if (curr->ref->type.getHeapType() + .getStruct() + .fields.at(curr->index) + .mutable_ == Mutable) { + set(Bits::ReadsMutableStruct); + } } void visitStructNotify(StructNotify* curr) { - parent.isAtomic = true; - - // If the ref is null. - parent.implicitTrap = true; - // struct.notify mutates an opaque waiter queue which isn't visible in // user code. Model this as a struct write which prevents reorderings // (since isAtomic == true). - parent.writesStruct = true; + // TODO: Implicit trap only if ref is nullable. + set(Bits::IsAtomic | Bits::ImplicitTrap | Bits::WritesStruct); } void visitArrayNew(ArrayNew* curr) {} void visitArrayNewData(ArrayNewData* curr) { // Traps on out of bounds access to segments or access to dropped // segments. - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } void visitArrayNewElem(ArrayNewElem* curr) { // Traps on out of bounds access to segments or access to dropped // segments. - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } void visitArrayNewFixed(ArrayNewFixed* curr) {} void visitArrayGet(ArrayGet* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.readsArray = true; - // traps when the arg is null or the index out of bounds - parent.implicitTrap = true; + // Traps when the arg is null or the index out of bounds. + set(Bits::ReadsArray | Bits::ImplicitTrap); } void visitArraySet(ArraySet* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.writesArray = true; - // traps when the arg is null or the index out of bounds - parent.implicitTrap = true; + // Traps when the arg is null or the index out of bounds. + set(Bits::WritesArray | Bits::ImplicitTrap); } void visitArrayLen(ArrayLen* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } // traps when the arg is null if (curr->ref->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } } void visitArrayCopy(ArrayCopy* curr) { if (curr->destRef->type.isNull() || curr->srcRef->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.readsArray = true; - parent.writesArray = true; - // traps when a ref is null, or when out of bounds. - parent.implicitTrap = true; + // Traps when a ref is null, or when out of bounds. + set(Bits::ReadsArray | Bits::WritesArray | Bits::ImplicitTrap); } void visitArrayFill(ArrayFill* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.writesArray = true; // Traps when the destination is null or when out of bounds. - parent.implicitTrap = true; + set(Bits::WritesArray | Bits::ImplicitTrap); } template void visitArrayInit(ArrayInit* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.writesArray = true; // Traps when the destination is null, when out of bounds in source or // destination, or when the source segment has been dropped. - parent.implicitTrap = true; + set(Bits::WritesArray | Bits::ImplicitTrap); } void visitArrayInitData(ArrayInitData* curr) { visitArrayInit(curr); } void visitArrayInitElem(ArrayInitElem* curr) { visitArrayInit(curr); } void visitArrayRMW(ArrayRMW* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.readsArray = true; - parent.writesArray = true; - // traps when the arg is null or the index out of bounds - parent.implicitTrap = true; + // Traps when the arg is null or the index out of bounds. + set(Bits::ReadsArray | Bits::WritesArray | Bits::ImplicitTrap | + Bits::IsAtomic); assert(curr->order != MemoryOrder::Unordered); - parent.isAtomic = true; } void visitArrayCmpxchg(ArrayCmpxchg* curr) { if (curr->ref->type.isNull()) { - parent.trap = true; + set(Bits::Trap); return; } - parent.readsArray = true; - parent.writesArray = true; - // traps when the arg is null or the index out of bounds - parent.implicitTrap = true; + // Traps when the arg is null or the index out of bounds. + set(Bits::ReadsArray | Bits::WritesArray | Bits::ImplicitTrap | + Bits::IsAtomic); assert(curr->order != MemoryOrder::Unordered); - parent.isAtomic = true; } void visitRefAs(RefAs* curr) { if (curr->op == AnyConvertExtern || curr->op == ExternConvertAny) { @@ -1095,7 +1090,7 @@ class EffectAnalyzer { return; } // traps when the arg is not valid - parent.implicitTrap = true; + set(Bits::ImplicitTrap); // Note: We could be more precise here and report the lack of a possible // trap if the input is non-nullable (and also of the right kind for // RefAsFunc etc.). However, we have optimization passes that will @@ -1104,104 +1099,93 @@ class EffectAnalyzer { // only help until the next time those optimizations run. As a tradeoff, // we keep the code here simpler, but it does mean another optimization // cycle may be needed in some cases. + // TODO: This seems like an unnecessary tradeoff. } void visitStringNew(StringNew* curr) { - // traps when ref is null - parent.implicitTrap = true; + // Traps when ref is null. + set(Bits::ImplicitTrap); if (curr->op != StringNewFromCodePoint) { - parent.readsArray = true; + set(Bits::ReadsArray); } } void visitStringConst(StringConst* curr) {} void visitStringMeasure(StringMeasure* curr) { - // traps when ref is null. - parent.implicitTrap = true; + // Traps when ref is null. + set(Bits::ImplicitTrap); } void visitStringEncode(StringEncode* curr) { - // traps when ref is null or we write out of bounds. - parent.implicitTrap = true; - parent.writesArray = true; + // Traps when ref is null or we write out of bounds. + set(Bits::ImplicitTrap | Bits::WritesArray); } void visitStringConcat(StringConcat* curr) { - // traps when an input is null. - parent.implicitTrap = true; + // Traps when an input is null. + set(Bits::ImplicitTrap); } void visitStringEq(StringEq* curr) { if (curr->op == StringEqCompare) { - // traps when either input is null. + // Traps when either input is null. if (curr->left->type.isNullable() || curr->right->type.isNullable()) { - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } } } void visitStringTest(StringTest* curr) {} void visitStringWTF16Get(StringWTF16Get* curr) { - // traps when ref is null. - parent.implicitTrap = true; + // Traps when ref is null. + set(Bits::ImplicitTrap); } void visitStringSliceWTF(StringSliceWTF* curr) { - // traps when ref is null. - parent.implicitTrap = true; + // Traps when ref is null. + set(Bits::ImplicitTrap); } void visitContNew(ContNew* curr) { - // traps when curr->func is null ref. - parent.implicitTrap = true; + // Traps when curr->func is null ref. + set(Bits::ImplicitTrap); } void visitContBind(ContBind* curr) { - // traps when curr->cont is null ref. - parent.implicitTrap = true; - - // The input continuation is modified, as it will trap if resumed. This is - // a globally-noticeable effect, which we model as a call for now, but we - // could in theory use something more refined here (|modifiesContinuation| - // perhaps, to parallel |writesMemory| etc.). - parent.calls = true; + // Traps when curr->cont is null ref. The input continuation is modified, + // as it will trap if resumed. This is a globally-noticeable effect, which + // we model as a call for now, but we could in theory use something more + // refined here (|ModifiesContinuation| perhaps, to parallel + // |WritesMemory| etc.). + set(Bits::ImplicitTrap | Bits::Calls); } void visitSuspend(Suspend* curr) { // Similar to resume/call: Suspending means that we execute arbitrary // other code before we may resume here. - parent.calls = true; + set(Bits::Calls); if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } // A suspend may go unhandled and therefore trap. - parent.implicitTrap = true; + set(Bits::ImplicitTrap); } void visitResume(Resume* curr) { - // This acts as a kitchen sink effect. - parent.calls = true; - - // resume instructions accept nullable continuation references and trap - // on null. - parent.implicitTrap = true; + // The call acts as a kitchen sink effect. `resume` instructions accept + // nullable continuation references and trap on null. + set(Bits::Calls | Bits::ImplicitTrap); if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } } void visitResumeThrow(ResumeThrow* curr) { - // This acts as a kitchen sink effect. - parent.calls = true; - - // resume_throw instructions accept nullable continuation - // references and trap on null. - parent.implicitTrap = true; + // The call acts as a kitchen sink effect. `resume_throw` instructions + // accept nullable continuation references and trap on null. + set(Bits::Calls | Bits::ImplicitTrap); if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } } void visitStackSwitch(StackSwitch* curr) { - // This acts as a kitchen sink effect. - parent.calls = true; - - // switch instructions accept nullable continuation references - // and trap on null. - parent.implicitTrap = true; + // The call acts as a kitchen sink effect. `switch` instructions accept + // nullable continuation references and trap on null. + set(Bits::Calls | Bits::ImplicitTrap); if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws_ = true; + set(Bits::Throws); } } }; @@ -1242,10 +1226,10 @@ class EffectAnalyzer { }; uint32_t getSideEffects() const { uint32_t effects = 0; - if (branchesOut || hasExternalBreakTargets()) { + if (get(Bits::BranchesOut) || hasExternalBreakTargets()) { effects |= SideEffects::Branches; } - if (calls) { + if (get(Bits::Calls)) { effects |= SideEffects::Calls; } if (localsRead.size() > 0) { @@ -1260,31 +1244,31 @@ class EffectAnalyzer { if (globalsWritten.size() > 0) { effects |= SideEffects::WritesGlobal; } - if (readsMemory) { + if (get(Bits::ReadsMemory)) { effects |= SideEffects::ReadsMemory; } - if (writesMemory) { + if (get(Bits::WritesMemory)) { effects |= SideEffects::WritesMemory; } - if (readsTable) { + if (get(Bits::ReadsTable)) { effects |= SideEffects::ReadsTable; } - if (writesTable) { + if (get(Bits::WritesTable)) { effects |= SideEffects::WritesTable; } - if (implicitTrap) { + if (get(Bits::ImplicitTrap)) { effects |= SideEffects::ImplicitTrap; } if (trapsNeverHappen) { effects |= SideEffects::TrapsNeverHappen; } - if (isAtomic) { + if (get(Bits::IsAtomic)) { effects |= SideEffects::IsAtomic; } - if (throws_) { + if (get(Bits::Throws)) { effects |= SideEffects::Throws; } - if (danglingPop) { + if (get(Bits::DanglingPop)) { effects |= SideEffects::DanglingPop; } return effects; @@ -1298,9 +1282,8 @@ class EffectAnalyzer { // This function matches transfersControlFlow(), that is, after calling this // method transfersControlFlow() will always return false. void ignoreControlFlowTransfers() { - branchesOut = false; + set(Bits::BranchesOut | Bits::Throws, false); breakTargets.clear(); - throws_ = false; delegateTargets.clear(); assert(!transfersControlFlow()); } @@ -1310,9 +1293,9 @@ class EffectAnalyzer { assert(tryDepth == 0); if (ignoreImplicitTraps) { - implicitTrap = false; - } else if (implicitTrap) { - trap = true; + set(Bits::ImplicitTrap, false); + } else if (get(Bits::ImplicitTrap)) { + set(Bits::Trap); } } }; diff --git a/src/passes/CodeFolding.cpp b/src/passes/CodeFolding.cpp index 1fc738a377d..a07b9f7c4b5 100644 --- a/src/passes/CodeFolding.cpp +++ b/src/passes/CodeFolding.cpp @@ -324,7 +324,7 @@ struct CodeFolding // simulate its behavior. We cannot move expressions containing pops if // they are not enclosed in a 'catch' body, because a pop instruction // should follow right after 'catch'. - if (effects.danglingPop) { + if (effects.get(EffectAnalyzer::Bits::DanglingPop)) { return false; } // When an expression can throw and it is within a try/try_table scope, diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index fbfb278e93f..6cfab63d46b 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -53,16 +53,16 @@ struct GenerateGlobalEffects : public Pass { // Gather the effects. funcInfo.effects.emplace(getPassOptions(), *module, func); - if (funcInfo.effects->calls) { + if (funcInfo.effects->get(EffectAnalyzer::Bits::Calls)) { // There are calls in this function, which we will analyze in detail. // Clear the |calls| field first, and we'll handle calls of all sorts // below. - funcInfo.effects->calls = false; + funcInfo.effects->set(EffectAnalyzer::Bits::Calls, false); // Clear throws as well, as we are "forgetting" calls right now, and // want to forget their throwing effect as well. If we see something // else that throws, below, then we'll note that there. - funcInfo.effects->throws_ = false; + funcInfo.effects->set(EffectAnalyzer::Bits::Throws, false); struct CallScanner : public PostWalkerdynCast()) { // Note the direct call. funcInfo.calledFunctions.insert(call->target); - } else if (effects.calls) { + } else if (effects.get(EffectAnalyzer::Bits::Calls)) { // This is an indirect call of some sort, so we must assume the // worst. To do so, clear the effects, which indicates nothing // is known (so anything is possible). @@ -89,8 +89,9 @@ struct GenerateGlobalEffects : public Pass { // No call here, but update throwing if we see it. (Only do so, // however, if we have effects; if we cleared it - see before - // then we assume the worst anyhow, and have nothing to update.) - if (effects.throws_ && funcInfo.effects) { - funcInfo.effects->throws_ = true; + if (effects.get(EffectAnalyzer::Bits::Throws) && + funcInfo.effects) { + funcInfo.effects->set(EffectAnalyzer::Bits::Throws); } } } @@ -153,7 +154,7 @@ struct GenerateGlobalEffects : public Pass { for (auto& [func, info] : analysis.map) { if (callers[func->name].count(func->name)) { if (info.effects) { - info.effects->trap = true; + info.effects->set(EffectAnalyzer::Bits::Trap); } } } diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 5ecc9ebae0a..93a7e716208 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -567,8 +567,8 @@ struct GlobalTypeOptimization : public Pass { operands[newIndex] = old[i]; } else { ++removed; - if (!func && - EffectAnalyzer(getPassOptions(), *getModule(), old[i]).trap) { + if (!func && EffectAnalyzer(getPassOptions(), *getModule(), old[i]) + .traps()) { removedTrappingInits.push_back(old[i]); } } diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp index 73720bed430..afaa324553c 100644 --- a/src/passes/LocalCSE.cpp +++ b/src/passes/LocalCSE.cpp @@ -485,7 +485,7 @@ struct Checker // must happen twice. That is, removing trapping from (curr) in the // example above has no effect as (ORIGINAL) never has global write // effects. - effects.trap = false; + effects.set(EffectAnalyzer::Bits::Trap, false); auto idempotent = isIdempotent(curr, *getModule()); @@ -539,7 +539,7 @@ struct Checker // anyhow. (The other checks we perform here, including invalidation and // determinism, will ensure that either all of the appearances trap, or // none of them.) - effects.trap = false; + effects.set(EffectAnalyzer::Bits::Trap, false); // Note that we've already checked above that this has no side effects or // generativity: if we got here, then it is good to go from the diff --git a/src/passes/Monomorphize.cpp b/src/passes/Monomorphize.cpp index 9f02d00402e..7d46b794e29 100644 --- a/src/passes/Monomorphize.cpp +++ b/src/passes/Monomorphize.cpp @@ -461,7 +461,8 @@ struct CallContext { // Pretty much everything can be moved into the context if we can copy it // between functions, such as constants, globals, etc. The things we cannot // copy are now checked for. - if (effects.branchesOut || effects.hasExternalBreakTargets()) { + if (effects.get(EffectAnalyzer::Bits::BranchesOut) || + effects.hasExternalBreakTargets()) { // This branches or returns. We can't move control flow between functions. return false; } @@ -469,7 +470,7 @@ struct CallContext { // Reads/writes to local state cannot be moved around. return false; } - if (effects.calls) { + if (effects.get(EffectAnalyzer::Bits::Calls)) { // We can in principle move calls, but for simplicity we avoid such // situations (which might involve recursion etc.). return false; diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index 84cf29be89f..b2515980c23 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -397,7 +397,7 @@ struct ConstantGlobalApplier // forget everything, but in some cases we can do better, if we have a call // and have computed function effects for it. ShallowEffectAnalyzer effects(getPassOptions(), *getModule(), curr); - if (effects.calls) { + if (effects.get(EffectAnalyzer::Bits::Calls)) { // Forget everything. currConstantGlobals.clear(); } else { diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index cf0b7495011..3f3c7fef57d 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp @@ -452,7 +452,7 @@ struct SimplifyLocals FeatureSet features = this->getModule()->features; if (features.hasExceptionHandling() && EffectAnalyzer(this->getPassOptions(), *this->getModule(), set->value) - .danglingPop) { + .get(EffectAnalyzer::Bits::DanglingPop)) { return false; } // if in the first cycle, or not allowing tees, then we cannot sink if >1 diff --git a/src/passes/Unsubtyping.cpp b/src/passes/Unsubtyping.cpp index d648635b4af..ef2315d44a1 100644 --- a/src/passes/Unsubtyping.cpp +++ b/src/passes/Unsubtyping.cpp @@ -1081,7 +1081,8 @@ struct Unsubtyping : Pass, Noter { // nested inside it. In that case we need to preserve the trap by // moving this descriptor to a new global. if (curr->desc->is() && - EffectAnalyzer(getPassOptions(), *getModule(), curr->desc).trap) { + EffectAnalyzer(getPassOptions(), *getModule(), curr->desc) + .traps()) { removedTrappingInits.push_back(curr->desc); } } diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index 2b5ec3f191b..f12c552b8ba 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -187,8 +187,10 @@ struct Vacuum : public WalkerPass> { // We also cannot remove a pop as it is necessary for structural // reasons. EffectAnalyzer effects(getPassOptions(), *getModule(), list[i]); - if (effects.transfersControlFlow() || effects.calls || - effects.mayNotReturn || effects.danglingPop) { + if (effects.transfersControlFlow() || + effects.get(EffectAnalyzer::Bits::Calls) || + effects.get(EffectAnalyzer::Bits::MayNotReturn) || + effects.get(EffectAnalyzer::Bits::DanglingPop)) { headingToTrap = false; continue; } @@ -494,7 +496,7 @@ struct Vacuum : public WalkerPass> { // emit 0 / 0 for a logical trap, rather than an Unreachable. We would // remove that 0 / 0 if we saw it, and the trap would not propagate. // (But other passes would handle it, if they saw it first.) - if (effects.trap) { + if (effects.traps()) { // The code is removable, so the trap is the only effect it has, and // we are considering removing it because TNH is enabled. assert(getPassOptions().trapsNeverHappen); From e59a80c8d8841c0a7302ea116408507fb840d582 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 27 Feb 2026 23:30:50 -0800 Subject: [PATCH 2/2] update example test --- test/example/cpp-unit.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/example/cpp-unit.cpp b/test/example/cpp-unit.cpp index 13e055b20e6..0875b1c108e 100644 --- a/test/example/cpp-unit.cpp +++ b/test/example/cpp-unit.cpp @@ -597,11 +597,11 @@ void test_effects() { // Unreachables trap. Unreachable unreachable; - assert_equal(EffectAnalyzer(options, module, &unreachable).trap, true); + assert_equal(EffectAnalyzer(options, module, &unreachable).traps(), true); // Nops... do not. Nop nop; - assert_equal(EffectAnalyzer(options, module, &nop).trap, false); + assert_equal(EffectAnalyzer(options, module, &nop).traps(), false); // ArrayCopy can trap, reads arrays, and writes arrays (but not structs). { @@ -631,11 +631,11 @@ void test_effects() { EffectAnalyzer effects(options, module); effects.visit(&arrayCopy); - assert_equal(effects.trap, true); - assert_equal(effects.readsArray, true); - assert_equal(effects.writesArray, true); - assert_equal(effects.readsMutableStruct, false); - assert_equal(effects.writesStruct, false); + assert_equal(effects.traps(), true); + assert_equal(effects.get(EffectAnalyzer::Bits::ReadsArray), true); + assert_equal(effects.get(EffectAnalyzer::Bits::WritesArray), true); + assert_equal(effects.get(EffectAnalyzer::Bits::ReadsMutableStruct), false); + assert_equal(effects.get(EffectAnalyzer::Bits::WritesStruct), false); } {