Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/analysis/lattices/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
namespace wasm::analysis {

// A lattice whose elements are N-tuples of elements of L. Also written as L^N.
// N is supplied at compile time rather than run time like it is for Vector.
template<Lattice L, size_t N> struct Array {
using Element = std::array<typename L::Element, N>;

Expand Down
138 changes: 138 additions & 0 deletions src/analysis/lattices/vector.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
* Copyright 2023 WebAssembly Community Group participants
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef wasm_analysis_lattices_vector_h
#define wasm_analysis_lattices_vector_h

#include <vector>

#include "../lattice.h"
#include "bool.h"
#include "flat.h"

namespace wasm::analysis {

// A lattice whose elements are N-tuples of elements of L. Also written as L^N.
// N is supplied at run time rather than compile time like it is for Array.
template<Lattice L> struct Vector {
using Element = std::vector<typename L::Element>;

L lattice;
const size_t size;

Vector(L&& lattice, size_t size) : lattice(std::move(lattice)), size(size) {}

Element getBottom() const noexcept {
return Element(size, lattice.getBottom());
}

Element getTop() const noexcept
#if __cplusplus >= 202002L
requires FullLattice<L>
#endif
{
return Element(size, lattice.getTop());
}

// `a` <= `b` if their elements are pairwise <=, etc. Unless we determine
// that there is no relation, we must check all the elements.
LatticeComparison compare(const Element& a, const Element& b) const noexcept {
assert(a.size() == size);
assert(b.size() == size);
auto result = EQUAL;
for (size_t i = 0; i < size; ++i) {
switch (lattice.compare(a[i], b[i])) {
case NO_RELATION:
return NO_RELATION;
case EQUAL:
continue;
case LESS:
if (result == GREATER) {
// Cannot be both less and greater.
return NO_RELATION;
}
result = LESS;
continue;
case GREATER:
if (result == LESS) {
// Cannot be both greater and less.
return NO_RELATION;
}
result = GREATER;
continue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That we copy-paste this switch logic is kind of disappointing. I'd have hoped with all the template magic there would be a way to avoid duplication? The same for the functions below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the copy-paste between the array and vector lattices? I could probably try to use even more template magic to reduce the duplication, but I think at that point we could start calling it template obfuscation 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can they both inherit from something?

Or perhaps they can both simply call a standalone utility class that does these operations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could perhaps have a CRTP parent class, but I don't think it's worth it. These methods are simple enough that I think there's more value in being able to read their implementations directly without an extra layer of source indirection than there would be in deduplicating them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the harm in adding a utility function (not a CRTP parent class) "compareSingleLatticeElement" that contains the switch, and is called from those loops? Those functions would become much smaller and more readable, and we'd avoid the duplication.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types are different, so at least that utility would have to be another template. It would also have to go in some new header file but would not be part of any public API, which is unfortunate. I don't anticipate the utility would be used anywhere except these two places, either.

I can do it if you feel strongly about it, but I don't think the extra complexity is worth the deduplication.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel that strongly about it, I suppose.

}
return result;
}

// Pairwise join on the elements.
bool join(Element& joinee, const Element& joiner) const noexcept {
assert(joinee.size() == size);
assert(joiner.size() == size);
bool result = false;
for (size_t i = 0; i < size; ++i) {
if constexpr (std::is_same_v<typename L::Element, bool>) {
// The vector<bool> specialization does not expose references to the
// individual bools because they might be in a bitmap, so we need a
// workaround.
bool e = joinee[i];
if (lattice.join(e, joiner[i])) {
joinee[i] = e;
result = true;
}
} else {
result |= lattice.join(joinee[i], joiner[i]);
}
}

return result;
}

// Pairwise meet on the elements.
bool meet(Element& meetee, const Element& meeter) const noexcept
#if __cplusplus >= 202002L
requires FullLattice<L>
#endif
{
assert(meetee.size() == size);
assert(meeter.size() == size);
bool result = false;
for (size_t i = 0; i < size; ++i) {
if constexpr (std::is_same_v<typename L::Element, bool>) {
// The vector<bool> specialization does not expose references to the
// individual bools because they might be in a bitmap, so we need a
// workaround.
bool e = meetee[i];
if (lattice.meet(e, meeter[i])) {
meetee[i] = e;
result = true;
}
} else {
result |= lattice.meet(meetee[i], meeter[i]);
}
}
return result;
}
};

#if __cplusplus >= 202002L
static_assert(FullLattice<Vector<Bool>>);
static_assert(Lattice<Vector<Flat<bool>>>);
#endif

} // namespace wasm::analysis

#endif // wasm_analysis_lattices_vector_h
74 changes: 63 additions & 11 deletions src/tools/wasm-fuzz-lattices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "analysis/lattices/inverted.h"
#include "analysis/lattices/lift.h"
#include "analysis/lattices/stack.h"
#include "analysis/lattices/vector.h"
#include "analysis/liveness-transfer-function.h"
#include "analysis/reaching-definitions-transfer-function.h"
#include "analysis/transfer-function.h"
Expand Down Expand Up @@ -151,33 +152,38 @@ static_assert(Lattice<RandomLattice>);
using ArrayFullLattice = analysis::Array<RandomFullLattice, 2>;
using ArrayLattice = analysis::Array<RandomLattice, 2>;

struct RandomFullLattice::L
: std::variant<Bool, UInt32, Inverted<RandomFullLattice>, ArrayFullLattice> {
};
struct RandomFullLattice::L : std::variant<Bool,
UInt32,
Inverted<RandomFullLattice>,
ArrayFullLattice,
Vector<RandomFullLattice>> {};

struct RandomFullLattice::ElementImpl
: std::variant<typename Bool::Element,
typename UInt32::Element,
typename Inverted<RandomFullLattice>::Element,
typename ArrayFullLattice::Element> {};
typename ArrayFullLattice::Element,
typename Vector<RandomFullLattice>::Element> {};

struct RandomLattice::L : std::variant<RandomFullLattice,
Flat<uint32_t>,
Lift<RandomLattice>,
ArrayLattice> {};
ArrayLattice,
Vector<RandomLattice>> {};

struct RandomLattice::ElementImpl
: std::variant<typename RandomFullLattice::Element,
typename Flat<uint32_t>::Element,
typename Lift<RandomLattice>::Element,
typename ArrayLattice::Element> {};
typename ArrayLattice::Element,
typename Vector<RandomLattice>::Element> {};

RandomFullLattice::RandomFullLattice(Random& rand,
size_t depth,
std::optional<uint32_t> maybePick)
: rand(rand) {
// TODO: Limit the depth once we get lattices with more fan-out.
uint32_t pick = maybePick ? *maybePick : rand.upTo(4);
uint32_t pick = maybePick ? *maybePick : rand.upTo(5);
switch (pick) {
case 0:
lattice = std::make_unique<L>(L{Bool{}});
Expand All @@ -193,30 +199,39 @@ RandomFullLattice::RandomFullLattice(Random& rand,
lattice = std::make_unique<L>(
L{ArrayFullLattice{RandomFullLattice{rand, depth + 1}}});
return;
case 4:
lattice = std::make_unique<L>(
L{Vector{RandomFullLattice{rand, depth + 1}, rand.upTo(4)}});
return;
}
WASM_UNREACHABLE("unexpected pick");
}

RandomLattice::RandomLattice(Random& rand, size_t depth) : rand(rand) {
// TODO: Limit the depth once we get lattices with more fan-out.
uint32_t pick = rand.upTo(7);
uint32_t pick = rand.upTo(9);
switch (pick) {
case 0:
case 1:
case 2:
case 3:
case 4:
lattice = std::make_unique<L>(L{RandomFullLattice{rand, depth, pick}});
return;
case 4:
case 5:
lattice = std::make_unique<L>(L{Flat<uint32_t>{}});
return;
case 5:
case 6:
lattice = std::make_unique<L>(L{Lift{RandomLattice{rand, depth + 1}}});
return;
case 6:
case 7:
lattice =
std::make_unique<L>(L{ArrayLattice{RandomLattice{rand, depth + 1}}});
return;
case 8:
lattice = std::make_unique<L>(
L{Vector{RandomLattice{rand, depth + 1}, rand.upTo(4)}});
return;
}
WASM_UNREACHABLE("unexpected pick");
}
Expand All @@ -235,6 +250,14 @@ RandomFullLattice::Element RandomFullLattice::makeElement() const noexcept {
return ElementImpl{typename ArrayFullLattice::Element{
l->lattice.makeElement(), l->lattice.makeElement()}};
}
if (const auto* l = std::get_if<Vector<RandomFullLattice>>(lattice.get())) {
std::vector<typename RandomFullLattice::Element> elem;
elem.reserve(l->size);
for (size_t i = 0; i < l->size; ++i) {
elem.push_back(l->lattice.makeElement());
}
return ElementImpl{std::move(elem)};
}
WASM_UNREACHABLE("unexpected lattice");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't ask for a change here, but I really do think virtual would be a nicer way to write code like this, rather than have to keep modifying RandomFullLattice for each type. That is, I would personally gone the other route and had a virtual method that this class calls, each new lattice just implements a "makeRandom" interface, so it all plugs in.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to at least get rid of the distinction between RandomFullLattice and RandomLattice, which should simplify things.

I can also clean up this giant if-else chain by using std::visit combined with overloaded free functions, which will be closer to what you're envisioning, I think. I don't think that going all the way to creating an inheritance-based wrapper for each of the lattices would be worth it, though.

}

Expand All @@ -261,6 +284,14 @@ RandomLattice::Element RandomLattice::makeElement() const noexcept {
return ElementImpl{typename ArrayLattice::Element{
l->lattice.makeElement(), l->lattice.makeElement()}};
}
if (const auto* l = std::get_if<Vector<RandomLattice>>(lattice.get())) {
std::vector<typename RandomLattice::Element> elem;
elem.reserve(l->size);
for (size_t i = 0; i < l->size; ++i) {
elem.push_back(l->lattice.makeElement());
}
return ElementImpl{std::move(elem)};
}
WASM_UNREACHABLE("unexpected lattice");
}

Expand Down Expand Up @@ -293,6 +324,17 @@ void printFullElement(std::ostream& os,
printFullElement(os, e->back(), depth + 1);
indent(os, depth);
os << "]\n";
} else if (const auto* vec =
std::get_if<typename Vector<RandomFullLattice>::Element>(
&*elem)) {
os << "Vector[\n";
for (const auto& e : *vec) {
printFullElement(os, e, depth + 1);
}
indent(os, depth);
os << "]\n";
} else {
WASM_UNREACHABLE("unexpected element");
}
}

Expand Down Expand Up @@ -332,6 +374,16 @@ void printElement(std::ostream& os,
printElement(os, e->back(), depth + 1);
indent(os, depth);
os << ")\n";
} else if (const auto* vec =
std::get_if<typename Vector<RandomLattice>::Element>(&*elem)) {
os << "Vector[\n";
for (const auto& e : *vec) {
printElement(os, e, depth + 1);
}
indent(os, depth);
os << "]\n";
} else {
WASM_UNREACHABLE("unexpected element");
}
}

Expand Down
Loading