diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp index d32e0266363..ffc0baaa850 100644 --- a/src/passes/Heap2Local.cpp +++ b/src/passes/Heap2Local.cpp @@ -170,8 +170,16 @@ namespace { // Core analysis that provides an escapes() method to check if an allocation // escapes in a way that prevents optimizing it away as described above. It also // stashes information about the relevant expressions as it goes, which helps -// optimization later (|reached|). +// optimization later (|seen| and |reached|). struct EscapeAnalyzer { + // All the expressions that have already been seen by the optimizer, see the + // comment above on exclusivity: once we have seen something when analyzing + // one allocation, if we reach it again then we can exit early since seeing it + // a second time proves we lost exclusivity. We must track this across + // multiple instances of EscapeAnalyzer as each handles a particular + // allocation. + std::unordered_set& seen; + // To find what escapes, we need to follow where values flow, both up to // parents, and via branches, and through locals. // TODO: for efficiency, only scan reference types in LocalGraph @@ -182,12 +190,13 @@ struct EscapeAnalyzer { const PassOptions& passOptions; Module& wasm; - EscapeAnalyzer(const LocalGraph& localGraph, + EscapeAnalyzer(std::unordered_set& seen, + const LocalGraph& localGraph, const Parents& parents, const BranchUtils::BranchTargets& branchTargets, const PassOptions& passOptions, Module& wasm) - : localGraph(localGraph), parents(parents), + : seen(seen), localGraph(localGraph), parents(parents), branchTargets(branchTargets), passOptions(passOptions), wasm(wasm) {} // We must track all the local.sets that write the allocation, to verify @@ -252,6 +261,28 @@ struct EscapeAnalyzer { assert(interaction == ParentChildInteraction::FullyConsumes || interaction == ParentChildInteraction::Flows); + // If we've already seen an expression, stop since we cannot optimize + // things that overlap in any way (see the notes on exclusivity, above). + // Note that we use a nonrepeating queue here, so we already do not visit + // the same thing more than once; what this check does is verify we don't + // look at something that another allocation reached, which would be in a + // different call to this function and use a different queue (any overlap + // between calls would prove non-exclusivity). + // + // It is ok, however, to see a parent more than once, if the allocation + // flows to it from two children, as is the case for ref.eq. Likewise, for + // struct.set, where we also have different considerations for the two + // children (the reference does not escape, but the value does), and in + // that case there may be different allocations for the children. + if (interaction == ParentChildInteraction::Flows) { + auto seenBefore = !seen.emplace(parent).second; + auto reachedInThisAllocation = reached.count(parent) > 0; + if (seenBefore && !reachedInThisAllocation) { + // XXX struct.set can be from different allocations! + return true; + } + } + // We can proceed, as the parent interacts with us properly, and we are // the only allocation to get here. @@ -1031,6 +1062,10 @@ struct Heap2Local { // flow to. localGraph.computeSetInfluences(); + // All the expressions we have already looked at. We use this to avoid + // repeated work, see above. + std::unordered_set seen; + // Find all the relevant allocations in the function: StructNew, ArrayNew, // ArrayNewFixed. struct AllocationFinder : public PostWalker { @@ -1090,7 +1125,7 @@ struct Heap2Local { } EscapeAnalyzer analyzer( - localGraph, parents, branchTargets, passOptions, wasm); + seen, localGraph, parents, branchTargets, passOptions, wasm); if (!analyzer.escapes(allocation)) { // Convert the allocation and all its uses into a struct. Then convert // the struct into locals. @@ -1110,7 +1145,7 @@ struct Heap2Local { // Check for escaping, noting relevant information as we go. If this does // not escape, optimize it into locals. EscapeAnalyzer analyzer( - localGraph, parents, branchTargets, passOptions, wasm); + seen, localGraph, parents, branchTargets, passOptions, wasm); if (!analyzer.escapes(allocation)) { Struct2Local(allocation, analyzer, func, wasm); }