ConstraintAnalysis: Return a bool from approximateOr

src/ir/constraint.cpp

@@ -159,32 +159,33 @@ void AndedConstraintSet::approximateAnd(const Constraint& c) {
   //       useful to implement that).
 }
 
-void AndedConstraintSet::approximateOr(const AndedConstraintSet& other) {
+bool AndedConstraintSet::approximateOr(const AndedConstraintSet& other) {
   // If one proves everything, the only thing that matters is the other.
+  if (other.provesEverything()) {
+    return false;
+  }
   if (provesEverything()) {
     *this = other;
-    return;
-  }
-  if (other.provesEverything()) {
-    return;
+    return true;
   }
 
   // If this is already implied by current constraints, then it is redundant.
   // E.g. if we are { x = 10 } and other is { x >= 0 } then all we need is
   // { x >= 0 } as the result of the OR.
+  if (other.proves(*this) == True) {
+    return false;
+  }
   if (proves(other) == True) {
     *this = other;
-    return;
-  }
-  if (other.proves(*this) == True) {
-    return;
+    return true;
   }
 
   // TODO smarts: handle <= > and so forth
 
   // Otherwise, we don't know how to nicely OR these things, and expand to the
   // trivial set of no constraints.
   clear();
+  return true;
 }
 
 std::optional<LocalConstraint> LocalConstraint::parse(Expression* curr) {
@@ -303,30 +304,37 @@ void BasicBlockConstraintMap::setProvesNothing(Index index) {
   map.erase(index);
 }
 
-void BasicBlockConstraintMap::approximateOr(
+bool BasicBlockConstraintMap::approximateOr(
   const BasicBlockConstraintMap& other) {
   // If one is unreachable, it adds nothing to the other.
   if (other.unreachable) {
-    return;
+    return false;
   }
   if (unreachable) {
     *this = other;
-    return;
+    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).
+  bool changed = false;
   for (auto& [local, constraints] : map) {
-    constraints.approximateOr(other.get(local));
+    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());
-    return constraints.provesNothing();
+    if (constraints.provesNothing()) {
+      changed = true;
+      return true;
+    }
+    return false;
   });
+
+  return changed;
 }
 
 void BasicBlockConstraintMap::approximateAndInternal(Index index,

src/ir/constraint.h

@@ -177,7 +177,9 @@ struct AndedConstraintSet : inplace_vector<Constraint, MaxConstraints> {
   //   { x >= 0 }
   //
   // If we become too imprecise, we lose the ability to imply anything useful.
-  void approximateOr(const AndedConstraintSet& other);
+  //
+  // Returns whether we changed anything.
+  bool approximateOr(const AndedConstraintSet& other);
 
   // Set a constraint, replacing all previous state.
   void set(const Constraint& c) {
@@ -266,7 +268,9 @@ struct BasicBlockConstraintMap {
   // Perform an OR as above. When a local only appears in one map, we treat it
   // as if it contains a contradiction there, that is, as if the code is
   // unreachable.
-  void approximateOr(const BasicBlockConstraintMap& other);
+  //
+  // Returns whether we changed anything.
+  bool approximateOr(const BasicBlockConstraintMap& other);
 
   // Perform an AND as above, on a particular index.
   void approximateAnd(Index index, const Constraint& c) {

src/passes/ConstraintAnalysis.cpp

@@ -171,9 +171,7 @@ struct ConstraintAnalysis
         }
 
         // If anything changed at the start of the target block, flow onwards.
-        auto old = outStartConstraints;
-        outStartConstraints.approximateOr(sentConstraints);
-        if (outStartConstraints != old) {
+        if (outStartConstraints.approximateOr(sentConstraints)) {
           work.push(out);
         }
       }