[analysis][NFC] Rename makeLeastUpperBound to join and move it to lattice

In general, the operation of taking the least upper bound of two lattice
elements may depend on some state stored in the lattice object rather than in
the elements themselves. To avoid forcing the elements to be larger and more
complicated in that case (by storing a parent pointer back to the lattice), move
the least upper bound operation to make it a method of the lattice rather than
the lattice element. This is also more consistent with where we put e.g. the
`compare` method.

While we are at it, rename `makeLeastUpperBound` to `join`, which is much
shorter and nicer. Usually we avoid using esoteric mathematical jargon like
this, but "join" as a normal verb actually describes the operation nicely, so I
think it is ok in this case.

Author: tlively

src/analysis/lattice.h

@@ -55,7 +55,7 @@ concept Lattice = requires(const L& lattice,
   } noexcept -> std::same_as<LatticeComparison>;
   // We need to be able to get the least upper bound of two elements and know
   // whether any change was made.
-  { elem.makeLeastUpperBound(constElem) } noexcept -> std::same_as<bool>;
+  { lattice.join(elem, constElem) } noexcept -> std::same_as<bool>;
 };
 
 #else // __cplusplus >= 202002L

src/analysis/lattices/powerset-impl.h

@@ -78,17 +78,18 @@ inline size_t FiniteIntPowersetLattice::Element::count() const {
 // Least upper bound is implemented as a logical OR between the bitvectors on
 // both sides. We return true if a bit is flipped in-place on the left so the
 // worklist algorithm will know if when to enqueue more work.
-inline bool FiniteIntPowersetLattice::Element::makeLeastUpperBound(
-  const FiniteIntPowersetLattice::Element& other) noexcept {
+inline bool
+FiniteIntPowersetLattice::join(Element& self,
+                               const Element& other) const noexcept {
   // Both must be from powerset lattice of the same set.
-  assert(other.bitvector.size() == bitvector.size());
+  assert(other.bitvector.size() == self.bitvector.size());
 
   bool modified = false;
-  for (size_t i = 0; i < bitvector.size(); ++i) {
+  for (size_t i = 0; i < self.bitvector.size(); ++i) {
     // Bit is flipped on self only if self is false and other is true when self
     // and other are OR'ed together.
-    modified |= (!bitvector[i] && other.bitvector[i]);
-    bitvector[i] = bitvector[i] || other.bitvector[i];
+    modified |= (!self.bitvector[i] && other.bitvector[i]);
+    self.bitvector[i] = self.bitvector[i] || other.bitvector[i];
   }
 
   return modified;

src/analysis/lattices/powerset.h

@@ -71,11 +71,6 @@ class FiniteIntPowersetLattice {
     bool isTop() const { return count() == bitvector.size(); }
     bool isBottom() const { return count() == 0; }
 
-    // Calculates the LUB of this element with some other element and sets
-    // this element to the LUB in place. Returns true if this element before
-    // this method call was different than the LUB.
-    bool makeLeastUpperBound(const Element& other) noexcept;
-
     // Prints out the bits in the bitvector for a lattice element.
     void print(std::ostream& os);
 
@@ -89,6 +84,11 @@ class FiniteIntPowersetLattice {
 
   // Returns an instance of the bottom lattice element.
   Element getBottom() const noexcept;
+
+  // Calculates the LUB of this element with some other element and sets
+  // this element to the LUB in place. Returns true if this element before
+  // this method call was different than the LUB.
+  bool join(Element& self, const Element& other) const noexcept;
 };
 
 // A layer of abstraction over FiniteIntPowersetLattice which maps
@@ -149,6 +149,10 @@ template<typename T> class FinitePowersetLattice {
   }
 
   Element getBottom() const noexcept { return intLattice.getBottom(); }
+
+  bool join(Element& self, const Element& other) const noexcept {
+    return intLattice.join(self, other);
+  }
 };
 
 } // namespace wasm::analysis

src/analysis/lattices/stack.h

@@ -127,51 +127,6 @@ template<Lattice L> class StackLattice {
       return value;
     }
 
-    // When taking the LUB, we take the LUBs of the elements of each stack
-    // starting from the top of the stack. So, LUB([b, a], [b', a']) is
-    // [LUB(b, b'), LUB(a, a')]. If one stack is higher than the other,
-    // the bottom of the higher stack will be kept, while the LUB of the
-    // corresponding tops of each stack will be taken. For instance,
-    // LUB([d, c, b, a], [b', a']) is [d, c, LUB(b, b'), LUB(a, a')].
-    //
-    // We start at the top because it makes taking the LUB of stacks with
-    // different scope easier, as mentioned at the top of the file. It also
-    // fits with the conception of the stack starting at the top and having
-    // an infinite bottom, which allows stacks of different height and scope
-    // to be easily joined.
-    bool makeLeastUpperBound(const Element& other) noexcept {
-      // Top element cases, since top elements don't actually have the stack
-      // value.
-      if (isTop()) {
-        return false;
-      } else if (other.isTop()) {
-        stackValue.reset();
-        return true;
-      }
-
-      bool modified = false;
-
-      // Merge the shorter height stack with the top of the longer height
-      // stack. We do this by taking the LUB of each pair of matching elements
-      // from both stacks.
-      auto otherIt = other.stackValue->crbegin();
-      auto thisIt = stackValue->rbegin();
-      for (;
-           thisIt != stackValue->rend() && otherIt != other.stackValue->crend();
-           ++thisIt, ++otherIt) {
-        modified |= thisIt->makeLeastUpperBound(*otherIt);
-      }
-
-      // If the other stack is higher, append the bottom of it to our current
-      // stack.
-      for (; otherIt != other.stackValue->crend(); ++otherIt) {
-        stackValue->push_front(*otherIt);
-        modified = true;
-      }
-
-      return modified;
-    }
-
     void print(std::ostream& os) {
       if (isTop()) {
         os << "TOP STACK" << std::endl;
@@ -188,6 +143,8 @@ template<Lattice L> class StackLattice {
     friend StackLattice;
   };
 
+  Element getBottom() const noexcept { return Element{}; }
+
   // Like in computing the LUB, we compare the tops of the two stacks, as it
   // handles the case of stacks of different scopes. Comparisons are done by
   // element starting from the top.
@@ -258,7 +215,50 @@ template<Lattice L> class StackLattice {
     }
   }
 
-  Element getBottom() const noexcept { return Element{}; }
+  // When taking the LUB, we take the LUBs of the elements of each stack
+  // starting from the top of the stack. So, LUB([b, a], [b', a']) is
+  // [LUB(b, b'), LUB(a, a')]. If one stack is higher than the other,
+  // the bottom of the higher stack will be kept, while the LUB of the
+  // corresponding tops of each stack will be taken. For instance,
+  // LUB([d, c, b, a], [b', a']) is [d, c, LUB(b, b'), LUB(a, a')].
+  //
+  // We start at the top because it makes taking the LUB of stacks with
+  // different scope easier, as mentioned at the top of the file. It also
+  // fits with the conception of the stack starting at the top and having
+  // an infinite bottom, which allows stacks of different height and scope
+  // to be easily joined.
+  bool join(Element& self, const Element& other) const noexcept {
+    // Top element cases, since top elements don't actually have the stack
+    // value.
+    if (self.isTop()) {
+      return false;
+    } else if (other.isTop()) {
+      self.stackValue.reset();
+      return true;
+    }
+
+    bool modified = false;
+
+    // Merge the shorter height stack with the top of the longer height
+    // stack. We do this by taking the LUB of each pair of matching elements
+    // from both stacks.
+    auto selfIt = self.stackValue->rbegin();
+    auto otherIt = other.stackValue->crbegin();
+    for (; selfIt != self.stackValue->rend() &&
+           otherIt != other.stackValue->crend();
+         ++selfIt, ++otherIt) {
+      modified |= lattice.join(*selfIt, *otherIt);
+    }
+
+    // If the other stack is higher, append the bottom of it to our current
+    // stack.
+    for (; otherIt != other.stackValue->crend(); ++otherIt) {
+      self.stackValue->push_front(*otherIt);
+      modified = true;
+    }
+
+    return modified;
+  }
 };
 
 } // namespace wasm::analysis

src/analysis/monotone-analyzer-impl.h

@@ -45,7 +45,7 @@ inline void MonotoneCFGAnalyzer<L, TxFn>::evaluate() {
     for (auto& dep : txfn.getDependents(cfg[i])) {
       // If the input state for the dependent block changes, we need to
       // re-analyze it.
-      if (states[dep.getIndex()].makeLeastUpperBound(state)) {
+      if (lattice.join(states[dep.getIndex()], state)) {
         worklist.push(dep.getIndex());
       }
     }

test/gtest/cfg.cpp

@@ -164,7 +164,7 @@ TEST_F(CFGTest, FinitePowersetLatticeFunctioning) {
   element2.print(ss);
   EXPECT_EQ(ss.str(), "100101");
   ss.str(std::string());
-  element2.makeLeastUpperBound(element1);
+  lattice.join(element2, element1);
   element2.print(ss);
   EXPECT_EQ(ss.str(), "101101");
 }
@@ -496,7 +496,7 @@ TEST_F(CFGTest, StackLatticeFunctioning) {
   EXPECT_EQ(stackLattice.compare(thirdStack, expectedStack),
             LatticeComparison::EQUAL);
 
-  EXPECT_EQ(thirdStack.makeLeastUpperBound(secondStack), true);
+  EXPECT_TRUE(stackLattice.join(thirdStack, secondStack));
 
   {
     expectedStack.stackTop().set(0, true);