From 53929e67ce78ee2dfb57b73e1e23f29c5e9b7b4d Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 20 Oct 2023 17:31:15 -0700 Subject: [PATCH] [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. --- src/analysis/lattice.h | 2 +- src/analysis/lattices/powerset-impl.h | 13 ++-- src/analysis/lattices/powerset.h | 14 ++-- src/analysis/lattices/stack.h | 92 +++++++++++++-------------- src/analysis/monotone-analyzer-impl.h | 2 +- test/gtest/cfg.cpp | 4 +- 6 files changed, 66 insertions(+), 61 deletions(-) diff --git a/src/analysis/lattice.h b/src/analysis/lattice.h index 7865de20b90..c45b0b97767 100644 --- a/src/analysis/lattice.h +++ b/src/analysis/lattice.h @@ -55,7 +55,7 @@ concept Lattice = requires(const L& lattice, } noexcept -> std::same_as; // 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; + { lattice.join(elem, constElem) } noexcept -> std::same_as; }; #else // __cplusplus >= 202002L diff --git a/src/analysis/lattices/powerset-impl.h b/src/analysis/lattices/powerset-impl.h index 3390934d3a7..e210a9e85bf 100644 --- a/src/analysis/lattices/powerset-impl.h +++ b/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; diff --git a/src/analysis/lattices/powerset.h b/src/analysis/lattices/powerset.h index 92159da12c8..08bcacd9f1c 100644 --- a/src/analysis/lattices/powerset.h +++ b/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 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 diff --git a/src/analysis/lattices/stack.h b/src/analysis/lattices/stack.h index 988da036b19..59a179010a7 100644 --- a/src/analysis/lattices/stack.h +++ b/src/analysis/lattices/stack.h @@ -127,51 +127,6 @@ template 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 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 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 diff --git a/src/analysis/monotone-analyzer-impl.h b/src/analysis/monotone-analyzer-impl.h index e52ac57595e..1f818290877 100644 --- a/src/analysis/monotone-analyzer-impl.h +++ b/src/analysis/monotone-analyzer-impl.h @@ -45,7 +45,7 @@ inline void MonotoneCFGAnalyzer::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()); } } diff --git a/test/gtest/cfg.cpp b/test/gtest/cfg.cpp index 9c6e8573aee..9f037e64439 100644 --- a/test/gtest/cfg.cpp +++ b/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);