Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC] Refactor LocalGraph's core getSets API #6877

Merged
merged 6 commits into from
Aug 28, 2024
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
10 changes: 5 additions & 5 deletions src/analysis/reaching-definitions-transfer-function.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class ReachingDefinitionsTransferFunction
std::unordered_map<Index, SmallVector<LocalSet*, 2>> indexSetses;

// LocalGraph members we need to update.
LocalGraph::GetSetses& getSetses;
LocalGraph::GetSetsMap& getSetsMap;

// Fictitious LocalSet objects to reprsent a local index obtaining its value
// from its default initial value or parameter value.
Expand Down Expand Up @@ -86,9 +86,9 @@ class ReachingDefinitionsTransferFunction
// are working with doesn't contain the correct Expression**s, but this is
// left in for future improvements. TODO.
ReachingDefinitionsTransferFunction(Function* func,
LocalGraph::GetSetses& getSetses,
LocalGraph::GetSetsMap& getSetsMap,
LocalGraph::Locations& locations)
: numLocals(func->getNumLocals()), getSetses(getSetses),
: numLocals(func->getNumLocals()), getSetsMap(getSetsMap),
lattice(listLocalSets(func, fakeInitialValueSets, fakeSetPtrs)) {

// Map every local index to a set of all the local sets which affect it.
Expand Down Expand Up @@ -129,9 +129,9 @@ class ReachingDefinitionsTransferFunction
if (lattice.exists(currState, setInstance)) {
// If a pointer to a real LocalSet, add it, otherwise add a nullptr.
if (fakeSetPtrs.find(setInstance) == fakeSetPtrs.end()) {
getSetses[curr].insert(setInstance);
getSetsMap[curr].insert(setInstance);
} else {
getSetses[curr].insert(nullptr);
getSetsMap[curr].insert(nullptr);
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions src/ir/LocalGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ struct Info {
// flow helper class. flows the gets to their sets

struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> {
LocalGraph::GetSetses& getSetses;
LocalGraph::GetSetsMap& getSetsMap;
LocalGraph::Locations& locations;

Flower(LocalGraph::GetSetses& getSetses,
Flower(LocalGraph::GetSetsMap& getSetsMap,
LocalGraph::Locations& locations,
Function* func,
Module* module)
: getSetses(getSetses), locations(locations) {
: getSetsMap(getSetsMap), locations(locations) {
setFunction(func);
setModule(module);
// create the CFG by walking the IR
Expand Down Expand Up @@ -183,7 +183,7 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> {
auto* set = action->cast<LocalSet>();
auto& gets = allGets[set->index];
for (auto* get : gets) {
getSetses[get].insert(set);
getSetsMap[get].insert(set);
}
gets.clear();
}
Expand All @@ -206,7 +206,7 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> {
// confusing when debugging, but it does not have any downside for
// optimization (since unreachable code should be removed anyhow).
for (auto* get : gets) {
getSetses[get].insert(nullptr);
getSetsMap[get].insert(nullptr);
}
continue;
}
Expand All @@ -222,7 +222,7 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> {
if (curr == entryFlowBlock) {
// These receive a param or zero init value.
for (auto* get : gets) {
getSetses[get].insert(nullptr);
getSetsMap[get].insert(nullptr);
}
}
} else {
Expand All @@ -241,7 +241,7 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> {
if (lastSet != pred->lastSets.end()) {
// There is a set here, apply it, and stop the flow.
for (auto* get : gets) {
getSetses[get].insert(lastSet->second);
getSetsMap[get].insert(lastSet->second);
}
} else {
// Keep on flowing.
Expand All @@ -261,11 +261,11 @@ struct Flower : public CFGWalker<Flower, Visitor<Flower>, Info> {
// LocalGraph implementation

LocalGraph::LocalGraph(Function* func, Module* module) : func(func) {
LocalGraphInternal::Flower flower(getSetses, locations, func, module);
LocalGraphInternal::Flower flower(getSetsMap, locations, func, module);

#ifdef LOCAL_GRAPH_DEBUG
std::cout << "LocalGraph::dump\n";
for (auto& [get, sets] : getSetses) {
for (auto& [get, sets] : getSetsMap) {
std::cout << "GET\n" << get << " is influenced by\n";
for (auto* set : sets) {
std::cout << set << '\n';
Expand All @@ -276,8 +276,8 @@ LocalGraph::LocalGraph(Function* func, Module* module) : func(func) {
}

bool LocalGraph::equivalent(LocalGet* a, LocalGet* b) {
auto& aSets = getSetses[a];
auto& bSets = getSetses[b];
auto& aSets = getSetsMap[a];
auto& bSets = getSetsMap[b];
// The simple case of one set dominating two gets easily proves that they must
// have the same value. (Note that we can infer dominance from the fact that
// there is a single set: if the set did not dominate one of the gets then
Expand Down Expand Up @@ -315,7 +315,7 @@ bool LocalGraph::equivalent(LocalGet* a, LocalGet* b) {
void LocalGraph::computeSetInfluences() {
for (auto& [curr, _] : locations) {
if (auto* get = curr->dynCast<LocalGet>()) {
for (auto* set : getSetses[get]) {
for (auto* set : getSetsMap[get]) {
setInfluences[set].insert(get);
}
}
Expand All @@ -335,7 +335,7 @@ void LocalGraph::computeGetInfluences() {

void LocalGraph::computeSSAIndexes() {
std::unordered_map<Index, std::set<LocalSet*>> indexSets;
for (auto& [get, sets] : getSetses) {
for (auto& [get, sets] : getSetsMap) {
for (auto* set : sets) {
indexSets[get->index].insert(set);
}
Expand Down
47 changes: 29 additions & 18 deletions src/ir/local-graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,37 +39,42 @@ namespace wasm {
// code will be removed anyhow).
//
struct LocalGraph {
// main API

// The constructor computes getSetses, the sets affecting each get.
//
// If a module is passed in, it is used to find which features are needed in
// the computation (for example, if exception handling is disabled, then we
// can generate a simpler CFG, as calls cannot throw).
LocalGraph(Function* func, Module* module = nullptr);

// The local.sets relevant for an index or a get. The most common case is to
// have a single set; after that, to be a phi of 2 items, so we use a small
// set of size 2 to avoid allocations there.
// Get the sets relevant for a local.get.
//
// A nullptr set means there is no local.set for that value, which means it is
// the initial value from the function entry: 0 for a var, the received value
// for a param.
//
// Often there is a single set, or a phi or two items, so we use a small set.
using Sets = SmallSet<LocalSet*, 2>;
const Sets& getSets(LocalGet* get) const {
// When we return an empty result, use a canonical constant empty set to
// avoid allocation.
static const Sets empty;
auto iter = getSetsMap.find(get);
if (iter == getSetsMap.end()) {
return empty;
}
return iter->second;
}

using GetSetses = std::unordered_map<LocalGet*, Sets>;

// Where each get and set is. We compute this while doing the main computation
// and make it accessible for users, for easy replacing of things without
// extra work.
using Locations = std::map<Expression*, Expression**>;

// externally useful information
GetSetses getSetses; // the sets affecting each get. a nullptr set means the
// initial value (0 for a var, the received value for a
// param)
Locations locations; // where each get and set is (for easy replacing)
Locations locations;

// Checks if two gets are equivalent, that is, definitely have the same
// value.
bool equivalent(LocalGet* a, LocalGet* b);

// Optional: compute the influence graphs between sets and gets
// (useful for algorithms that propagate changes).

// Optional: compute the influence graphs between sets and gets (useful for
// algorithms that propagate changes).
void computeSetInfluences();
void computeGetInfluences();

Expand Down Expand Up @@ -109,9 +114,15 @@ struct LocalGraph {

bool isSSA(Index x);

// Defined publicly as other utilities need similar data layouts.
using GetSetsMap = std::unordered_map<LocalGet*, Sets>;

private:
Function* func;
std::set<Index> SSAIndexes;

// A map of each get to the sets relevant to it.
GetSetsMap getSetsMap;
};

} // namespace wasm
Expand Down
9 changes: 7 additions & 2 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1245,15 +1245,20 @@ struct InfoCollector
// the type must be the same for all gets of that local.)
LocalGraph localGraph(func, getModule());

for (auto& [get, setsForGet] : localGraph.getSetses) {
for (auto& [curr, _] : localGraph.locations) {
auto* get = curr->dynCast<LocalGet>();
if (!get) {
continue;
}

auto index = get->index;
auto type = func->getLocalType(index);
if (!isRelevant(type)) {
continue;
}

// Each get reads from its relevant sets.
for (auto* set : setsForGet) {
for (auto* set : localGraph.getSets(get)) {
for (Index i = 0; i < type.size(); i++) {
Location source;
if (set) {
Expand Down
2 changes: 1 addition & 1 deletion src/passes/AvoidReinterprets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ static Load* getSingleLoad(LocalGraph* localGraph,
std::set<LocalGet*> seen;
seen.insert(get);
while (1) {
auto& sets = localGraph->getSetses[get];
auto& sets = localGraph->getSets(get);
if (sets.size() != 1) {
return nullptr;
}
Expand Down
4 changes: 1 addition & 3 deletions src/passes/Heap2Local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,7 @@ struct EscapeAnalyzer {

// Check that the gets can only read from the specific known sets.
for (auto* get : gets) {
auto iter = localGraph.getSetses.find(get);
assert(iter != localGraph.getSetses.end());
for (auto* set : iter->second) {
for (auto* set : localGraph.getSets(get)) {
if (sets.count(set) == 0) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/passes/LoopInvariantCodeMotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ struct LoopInvariantCodeMotion
bool hasGetDependingOnLoopSet(Expression* curr, LoopSets& loopSets) {
FindAll<LocalGet> gets(curr);
for (auto* get : gets.list) {
auto& sets = localGraph->getSetses[get];
auto& sets = localGraph->getSets(get);
for (auto* set : sets) {
// nullptr means a parameter or zero-init value;
// no danger to us.
Expand Down
14 changes: 8 additions & 6 deletions src/passes/MergeLocals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,10 @@ struct MergeLocals
// however, it may depend on other writes too, if there is a
// merge/phi, and in that case we can't do anything
assert(influencedGet->index == trivial->index);
if (preGraph.getSetses[influencedGet].size() == 1) {
auto& sets = preGraph.getSets(influencedGet);
if (sets.size() == 1) {
// this is ok
assert(*preGraph.getSetses[influencedGet].begin() == trivial);
assert(*sets.begin() == trivial);
// If local types are different (when one is a subtype of the
// other), don't optimize
if (func->getLocalType(copy->index) != influencedGet->type) {
Expand Down Expand Up @@ -161,9 +162,10 @@ struct MergeLocals
for (auto* influencedGet : copyInfluences) {
// as above, avoid merges/phis
assert(influencedGet->index == copy->index);
if (preGraph.getSetses[influencedGet].size() == 1) {
auto& sets = preGraph.getSets(influencedGet);
if (sets.size() == 1) {
// this is ok
assert(*preGraph.getSetses[influencedGet].begin() == copy);
assert(*sets.begin() == copy);
// If local types are different (when one is a subtype of the
// other), don't optimize
if (func->getLocalType(trivial->index) != influencedGet->type) {
Expand Down Expand Up @@ -199,7 +201,7 @@ struct MergeLocals
auto& trivialInfluences = preGraph.setInfluences[trivial];
for (auto* influencedGet : trivialInfluences) {
// verify the set
auto& sets = postGraph.getSetses[influencedGet];
auto& sets = postGraph.getSets(influencedGet);
if (sets.size() != 1 || *sets.begin() != copy) {
// not good, undo all the changes for this copy
for (auto* undo : trivialInfluences) {
Expand All @@ -213,7 +215,7 @@ struct MergeLocals
auto& copyInfluences = preGraph.setInfluences[copy];
for (auto* influencedGet : copyInfluences) {
// verify the set
auto& sets = postGraph.getSetses[influencedGet];
auto& sets = postGraph.getSets(influencedGet);
if (sets.size() != 1 || *sets.begin() != trivial) {
// not good, undo all the changes for this copy
for (auto* undo : copyInfluences) {
Expand Down
2 changes: 1 addition & 1 deletion src/passes/OptimizeAddedConstants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ template<typename P, typename T> class MemoryAccessOptimizer {
//
// This is only valid if y does not change in the middle!
if (auto* get = curr->ptr->template dynCast<LocalGet>()) {
auto& sets = localGraph->getSetses[get];
auto& sets = localGraph->getSets(get);
if (sets.size() == 1) {
auto* set = *sets.begin();
// May be a zero-init (in which case, we can ignore it). Must also be
Expand Down
2 changes: 1 addition & 1 deletion src/passes/Precompute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ struct Precompute
// for this get to have constant value, all sets must agree
Literals values;
bool first = true;
for (auto* set : localGraph.getSetses[get]) {
for (auto* set : localGraph.getSets(get)) {
Literals curr;
if (set == nullptr) {
if (getFunction()->isVar(get->index)) {
Expand Down
4 changes: 2 additions & 2 deletions src/passes/SSAify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ struct SSAify : public Pass {

bool hasMerges(LocalSet* set, LocalGraph& graph) {
for (auto* get : graph.setInfluences[set]) {
if (graph.getSetses[get].size() > 1) {
if (graph.getSets(get).size() > 1) {
return true;
}
}
Expand All @@ -131,7 +131,7 @@ struct SSAify : public Pass {
void computeGetsAndPhis(LocalGraph& graph) {
FindAll<LocalGet> gets(func->body);
for (auto* get : gets.list) {
auto& sets = graph.getSetses[get];
auto& sets = graph.getSets(get);
if (sets.size() == 0) {
continue; // unreachable, ignore
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/wasm-fuzz-lattices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ struct LivenessChecker {
// Struct to set up and check reaching definitions analysis lattice and transfer
// function.
struct ReachingDefinitionsChecker {
LocalGraph::GetSetses getSetses;
LocalGraph::GetSetsMap getSetsMap;
LocalGraph::Locations locations;
ReachingDefinitionsTransferFunction txfn;
AnalysisChecker<FinitePowersetLattice<LocalSet*>,
Expand All @@ -838,7 +838,7 @@ struct ReachingDefinitionsChecker {
ReachingDefinitionsChecker(Function* func,
uint64_t latticeElementSeed,
Name funcName)
: txfn(func, getSetses, locations),
: txfn(func, getSetsMap, locations),
checker(txfn.lattice,
txfn,
"FinitePowersetLattice<LocalSet*>",
Expand Down
2 changes: 1 addition & 1 deletion src/wasm/wasm-stack-opts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ void StackIROptimizer::local2Stack() {
if (set->index == get->index) {
// This might be a proper set-get pair, where the set is
// used by this get and nothing else, check that.
auto& sets = localGraph.getSetses[get];
auto& sets = localGraph.getSets(get);
if (sets.size() == 1 && *sets.begin() == set) {
auto& setInfluences = localGraph.setInfluences[set];
// If this has the proper value of 1, also do the potentially-
Expand Down
Loading
Loading