[NFC] Use a sorted vector instead of a map in ConstraintAnalysis

src/cfg/liveness-traversal.h

@@ -37,7 +37,7 @@ namespace wasm {
 // may be a great many potential elements but actual sets
 // may be fairly small. Specifically, we use a sorted
 // vector.
-using SetOfLocals = SortedVector;
+using SetOfLocals = SortedVector<Index>;
 
 // A liveness-relevant action. Supports a get, a set, or an
 // "other" which can be used for other purposes, to mark

src/ir/constraint.cpp

@@ -1051,14 +1051,12 @@ void BasicBlockConstraintMap::set(Index index,
   // We should not set values in unreachable code.
   assert(!unreachable);
 
-  // Clear the old state.
+  // Clear the old state, making us prove nothing.
   eraseStaleRefs(index);
   map.erase(index);
 
   // Apply the constraints, if there are any.
-  if (constraints.provesNothing()) {
-    setProvesNothing(index);
-  } else {
+  if (!constraints.provesNothing()) {
     for (auto& c : constraints) {
       approximateAnd(index, c);
     }
@@ -1199,24 +1197,27 @@ bool BasicBlockConstraintMap::approximateOr(
     return true;
   }
 
-  // We only need to loop on our locals, as any local that is missing in us is
-  // one that would end up proving nothing (and get removed).
+  // Both maps are sorted by local Index. Intersect in place: for us to be able
+  // to prove something (for us to have an entry in the ORed map), there must
+  // have been an entry in both original maps.
   bool changed = false;
-  for (auto& [local, constraints] : map) {
-    changed |= constraints.approximateOr(other.get(local));
-  }
-
-  // Anything that became trivial after the OR must be removed.
-  std::erase_if(map, [&](const auto& item) {
-    const auto& [local, constraints] = item;
-    // We do not store contradictions.
-    assert(!constraints.provesEverything());
-    if (constraints.provesNothing()) {
-      changed = true;
-      return true;
-    }
-    return false;
+  auto oldSize = map.size();
+  map.intersectAndFilter(other.map, [&](auto& self, const auto& other) {
+    changed |= self.value.approximateOr(other.value);
+    assert(!self.value.provesEverything());
+    // Keep only entries that prove things, as others should not be in the map.
+    return !self.value.provesNothing();
   });
+  if (map.size() != oldSize) {
+    changed = true;
+  }
+
+  // We could more precisely find which locals were removed from the map, but
+  // stale refs have low overhead and no correctness cost, so just handle the
+  // common, simple case of nothing remaining, so no refs are needed.
+  if (map.empty()) {
+    refs.clear();
+  }
 
   return changed;
 }
@@ -1246,18 +1247,19 @@ void BasicBlockConstraintMap::approximateAndInternal(Index index,
     // If we are applying a constraint to another local, and we know that
     // local's value, propagate it. That is, if x == 42, then if we try to apply
     // y < x we instead apply y < 42, which is better.
-    auto otherConstraints = get(*other);
-    if (auto lit = otherConstraints.getLiteral()) {
-      actual.term = Term{*lit};
+    if (auto iter = map.find(*other); iter != map.end()) {
+      if (auto lit = iter->value.getLiteral()) {
+        actual.term = Term{*lit};
+      }
     }
   }
 
   // Refer to the constraints for this index. If this is the first access of
   // the local, then we insert a new item into the map, which has a default of
-  // proxesEverything, which we need to flip (provesEverything cannot otherwise
+  // provesNothing, which we need to populate (provesNothing cannot otherwise
   // be found in the map, as we never store it).
-  auto [iter, _] = map.insert({index, AndedConstraintSet::makeProvesNothing()});
-  auto& indexConstraints = iter->second;
+  auto& indexConstraints =
+    map.insert({index, AndedConstraintSet::makeProvesNothing()}).value;
   // As in ::set(), this makes the map temporarily invalid until the
   // approximateAnd, as we don't store proves-nothing in the map, normally.
 
@@ -1267,6 +1269,7 @@ void BasicBlockConstraintMap::approximateAndInternal(Index index,
     // We just proved we are in unreachable code.
     unreachable = true;
     map.clear();
+    refs.clear();
     return;
   }
 
@@ -1310,18 +1313,22 @@ Result BasicBlockConstraintMap::proves(LocalConstraint condition) const {
   // about, propagate it. TODO: even without equality, we can add more
   // constraints here (e.g. x < y and y < 10 can lead to proving x < 10)
   if (auto* other = std::get_if<Index>(&condition.constraint.term)) {
-    auto otherConstraints = get(*other);
-    if (auto lit = otherConstraints.getLiteral()) {
-      condition.constraint.term = Term{*lit};
+    if (auto iter = map.find(*other); iter != map.end()) {
+      if (auto lit = iter->value.getLiteral()) {
+        condition.constraint.term = Term{*lit};
+      }
     }
   }
 
-  return get(condition.local).proves(condition.constraint);
+  if (auto iter = map.find(condition.local); iter != map.end()) {
+    return iter->value.proves(condition.constraint);
+  }
+  return Unknown;
 }
 
 void BasicBlockConstraintMap::noteRefs(Index index, const Constraint& c) {
   if (auto* i = std::get_if<Index>(&c.term)) {
-    refs[*i].insert(index);
+    refs.insert({*i, {}}).value.insert(index);
   }
 }
 
@@ -1331,11 +1338,12 @@ void BasicBlockConstraintMap::eraseStaleRefs(Index index) {
     return;
   }
 
-  auto& refIndexes = iter->second;
+  auto refIndexes = std::move(iter->value);
+  refs.erase(iter);
 
   for (auto refIndex : refIndexes) {
     if (auto iter = map.find(refIndex); iter != map.end()) {
-      auto& refConstraints = iter->second;
+      auto& refConstraints = iter->value;
       std::erase_if(refConstraints, [&](const auto& c) {
         if (auto* i = std::get_if<Index>(&c.term)) {
           if (*i == index) {

src/ir/constraint.h

@@ -33,6 +33,7 @@
 #include "ir/abstract.h"
 #include "support/inplace_vector.h"
 #include "support/small_vector.h"
+#include "support/sorted_vector.h"
 #include "support/span.h"
 #include "support/utilities.h"
 #include "wasm.h"
@@ -342,7 +343,7 @@ struct BasicBlockConstraintMap {
     assert(!unreachable);
 
     if (auto iter = map.find(index); iter != map.end()) {
-      auto& constraints = iter->second;
+      auto& constraints = iter->value;
       // If we can prove nothing, we should have removed it from the map.
       assert(!constraints.provesNothing());
       // If we can prove everything, we should be entirely unreachable.
@@ -367,30 +368,49 @@ struct BasicBlockConstraintMap {
   // Check a condition on a local, given all we know about all other locals.
   Result proves(LocalConstraint condition) const;
 
-  bool operator!=(const BasicBlockConstraintMap& other) {
+  bool operator!=(const BasicBlockConstraintMap& other) const {
     return unreachable != other.unreachable || map != other.map;
   }
 
   friend std::ostream& operator<<(std::ostream& o,
                                   const BasicBlockConstraintMap& map);
 
 private:
-  std::unordered_map<Index, AndedConstraintSet> map;
+  // Wrap a combination of an index and a value, and sort using only the index.
+  template<typename T> struct Indexed {
+    Index index;
+    T value;
+
+    bool operator<(const Indexed& other) const { return index < other.index; }
+    bool operator<(Index otherIndex) const { return index < otherIndex; }
+    bool operator==(const Indexed& other) const {
+      return index == other.index && value == other.value;
+    }
+    bool operator==(const Index& otherIndex) const {
+      return index == otherIndex;
+    }
+  };
+
+  // Sorted by local Index for fast contiguous copying and linear-time merge in
+  // approximateOr.
+  SortedVector<Indexed<AndedConstraintSet>> map;
 
-  // Maps an index to the locals that have constraints referring to it. When a
-  // local is modified, we need to wipe all those constraints, which become
-  // stale.
+  // Maps an index to the locals that have constraints referring to it, sorted
+  // by index. When a local is modified, we need to wipe all those constraints,
+  // which become stale.
   //
   // It is ok (but unoptimal in efficiency) if we have stale refs here, e.g. due
   // to approximation removing a constraint. Whenever there is a reference,
   // however, it must be noted here, so that when things get stale we can remove
   // them.
-  std::unordered_map<Index, std::unordered_set<Index>> refs;
+  SortedVector<Indexed<SortedVector<Index>>> refs;
 
   // Given a constraint on a local, note refs.
   void noteRefs(Index index, const Constraint& c);
 
-  // Given an index, erase constraints referring to it.
+  // Given an index, erase constraints referring to it. This is called when the
+  // information for this index is wiped out, so we clear the refs and the
+  // constraints referred to.
   void eraseStaleRefs(Index index);
 
   // Internal version, with a flag to flip the constraint. Whenever we apply

src/passes/DeadArgumentElimination.cpp

@@ -71,7 +71,7 @@ struct DAEFunctionInfo {
   // computation, and we reset it every time we touch the function.
   bool stale = true;
   // The unused parameters, if any.
-  SortedVector unusedParams;
+  SortedVector<Index> unusedParams;
   // Maps a function name to the calls going to it.
   std::unordered_map<Name, std::vector<Call*>> calls;
   // Map of all calls that are dropped, to their drops' locations (so that

src/passes/SignaturePruning.cpp

@@ -276,7 +276,7 @@ struct SignaturePruning : public Pass {
 
       // We found possible work! Find the specific params that are unused & try
       // to prune them.
-      SortedVector unusedParams;
+      SortedVector<Index> unusedParams;
       for (Index i = 0; i < numParams; i++) {
         if (!usedParams.contains(i)) {
           unusedParams.insert(i);

src/passes/param-utils.cpp

@@ -188,9 +188,9 @@ RemovalOutcome removeParameter(const std::vector<Function*>& funcs,
   return Success;
 }
 
-std::pair<SortedVector, RemovalOutcome>
+std::pair<SortedVector<Index>, RemovalOutcome>
 removeParameters(const std::vector<Function*>& funcs,
-                 SortedVector indexes,
+                 SortedVector<Index> indexes,
                  const std::vector<Call*>& calls,
                  const std::vector<CallRef*>& callRefs,
                  Module* module,
@@ -210,7 +210,7 @@ removeParameters(const std::vector<Function*>& funcs,
   // Iterate downwards, as we may remove more than one, and going forwards would
   // alter the indexes after us.
   Index i = first->getNumParams() - 1;
-  SortedVector removed;
+  SortedVector<Index> removed;
   while (1) {
     if (indexes.has(i)) {
       auto outcome = removeParameter(funcs, i, calls, callRefs, module, runner);
@@ -230,10 +230,10 @@ removeParameters(const std::vector<Function*>& funcs,
   return {removed, finalOutcome};
 }
 
-SortedVector applyConstantValues(const std::vector<Function*>& funcs,
-                                 const std::vector<Call*>& calls,
-                                 const std::vector<CallRef*>& callRefs,
-                                 Module* module) {
+SortedVector<Index> applyConstantValues(const std::vector<Function*>& funcs,
+                                        const std::vector<Call*>& calls,
+                                        const std::vector<CallRef*>& callRefs,
+                                        Module* module) {
   assert(funcs.size() > 0);
   auto* first = funcs[0];
 #ifndef NDEBUG
@@ -242,7 +242,7 @@ SortedVector applyConstantValues(const std::vector<Function*>& funcs,
   }
 #endif
 
-  SortedVector optimized;
+  SortedVector<Index> optimized;
   auto numParams = first->getNumParams();
   for (Index i = 0; i < numParams; i++) {
     PossibleConstantValues value;

src/passes/param-utils.h

@@ -87,9 +87,9 @@ RemovalOutcome removeParameter(const std::vector<Function*>& funcs,
 // we return Success if we removed any index, Failure if we removed none, and
 // FailureDueToEffects if at least one index could have been removed but for
 // effects).
-std::pair<SortedVector, RemovalOutcome>
+std::pair<SortedVector<Index>, RemovalOutcome>
 removeParameters(const std::vector<Function*>& funcs,
-                 SortedVector indexes,
+                 SortedVector<Index> indexes,
                  const std::vector<Call*>& calls,
                  const std::vector<CallRef*>& callRefs,
                  Module* module,
@@ -102,10 +102,10 @@ removeParameters(const std::vector<Function*>& funcs,
 // which allows other optimizations to remove it.
 //
 // Returns the indexes that were optimized.
-SortedVector applyConstantValues(const std::vector<Function*>& funcs,
-                                 const std::vector<Call*>& calls,
-                                 const std::vector<CallRef*>& callRefs,
-                                 Module* module);
+SortedVector<Index> applyConstantValues(const std::vector<Function*>& funcs,
+                                        const std::vector<Call*>& calls,
+                                        const std::vector<CallRef*>& callRefs,
+                                        Module* module);
 
 // Helper that localizes all calls to a set of targets, in an entire module.
 // This basically calls ChildLocalizer in each function, on the relevant calls.