Skip to content

[NFC] Convert LocalGraph influences accesses to function calls #6899

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 4, 2024
Merged

[NFC] Convert LocalGraph influences accesses to function calls #6899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
34a434c
start
kripken Sep 3, 2024
251133f
work
kripken Sep 3, 2024
8759860
work
kripken Sep 4, 2024
ca3aded
Merge remote-tracking branch 'origin/main' into nfc.lg.influences
kripken Sep 4, 2024
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
30 changes: 25 additions & 5 deletions src/ir/local-graph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ struct LocalGraph {
bool equivalent(LocalGet* a, LocalGet* b);

// Optional: compute the influence graphs between sets and gets (useful for
// algorithms that propagate changes).
// algorithms that propagate changes). Set influences are the gets that can
// read from it; get influences are the sets that can (directly) read from it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// read from it; get influences are the sets that can (directly) read from it.
// read from it; get influences are the sets from which the get can (directly) read.

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC what you wrote, that is actually getSets, not getInfluences. Example:

x = 5;       // line 0
foo(x);      // line 1
y = x + 20;  // line 2
  • getSets of the x on line 1 is the set of x on line 0.
  • setInfluences of the set on line 0 are the gets of x on lines 1 and 2.
  • getInfluences of the x on line 2 is the set of y on line 2.

getSets is the basic property of "what value is being read here?" We go backwards to find the right sets.

*Influences are used for forward propagation (hence the comment says "useful for algorithms that propagate changes"). Imagine that a set is given a new value, then setInfluences is the gets that read it. And, from each of those gets, we want to know which other sets may be influenced by it in turn, to keep propagating forward.

Copy link
Member

Choose a reason for hiding this comment

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

I see. I was confused by the explanation that sets were doing reads and thought that was surely a typo. Perhaps instead of talking about sets doing reads, we can describe the sets' values as using the gets.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps. Maybe "propagates" can also make more sense rather than "influences", as influence isn't as clearly directional. I'm not sure what's best here, but let's keep thinking about it.

void computeSetInfluences();
void computeGetInfluences();

Expand All @@ -83,11 +84,27 @@ struct LocalGraph {
computeGetInfluences();
}

// for each get, the sets whose values are influenced by that get
using GetInfluences = std::unordered_set<LocalSet*>;
std::unordered_map<LocalGet*, GetInfluences> getInfluences;
using SetInfluences = std::unordered_set<LocalGet*>;
std::unordered_map<LocalSet*, SetInfluences> setInfluences;
using GetInfluences = std::unordered_set<LocalSet*>;

const SetInfluences& getSetInfluences(LocalSet* set) const {
auto iter = setInfluences.find(set);
if (iter == setInfluences.end()) {
// Use a canonical constant empty set to avoid allocation.
static const SetInfluences empty;
return empty;
}
return iter->second;
}
const GetInfluences& getGetInfluences(LocalGet* get) const {
auto iter = getInfluences.find(get);
if (iter == getInfluences.end()) {
// Use a canonical constant empty set to avoid allocation.
static const GetInfluences empty;
return empty;
}
return iter->second;
}

// Optional: Compute the local indexes that are SSA, in the sense of
// * a single set for all the gets for that local index
Expand Down Expand Up @@ -132,6 +149,9 @@ struct LocalGraph {
// with that. It could alternatively be a shared_ptr, but that runs into what
// seems to be a false positive of clang's (but not gcc's) UBSan.
std::unique_ptr<LocalGraphFlower> flower;

std::unordered_map<LocalSet*, SetInfluences> setInfluences;
std::unordered_map<LocalGet*, GetInfluences> getInfluences;
};

} // namespace wasm
Expand Down
20 changes: 4 additions & 16 deletions src/passes/Heap2Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,8 @@ struct EscapeAnalyzer {
sets.insert(set);

// We must also look at how the value flows from those gets.
if (auto* getsReached = getGetsReached(set)) {
for (auto* get : *getsReached) {
flows.push({get, parents.getParent(get)});
}
for (auto* get : localGraph.getSetInfluences(set)) {
flows.push({get, parents.getParent(get)});
}
}

Expand Down Expand Up @@ -479,14 +477,6 @@ struct EscapeAnalyzer {
return ParentChildInteraction::Mixes;
}

const LocalGraph::SetInfluences* getGetsReached(LocalSet* set) {
auto iter = localGraph.setInfluences.find(set);
if (iter != localGraph.setInfluences.end()) {
return &iter->second;
}
return nullptr;
}

const BranchUtils::NameSet branchesSentByParent(Expression* child,
Expression* parent) {
BranchUtils::NameSet names;
Expand All @@ -508,10 +498,8 @@ struct EscapeAnalyzer {
// Find all the relevant gets (which may overlap between the sets).
std::unordered_set<LocalGet*> gets;
for (auto* set : sets) {
if (auto* getsReached = getGetsReached(set)) {
for (auto* get : *getsReached) {
gets.insert(get);
}
for (auto* get : localGraph.getSetInfluences(set)) {
gets.insert(get);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/passes/MergeLocals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct MergeLocals
for (auto* copy : copies) {
auto* trivial = copy->value->cast<LocalSet>();
bool canOptimizeToCopy = false;
auto& trivialInfluences = preGraph.setInfluences[trivial];
auto& trivialInfluences = preGraph.getSetInfluences(trivial);
if (!trivialInfluences.empty()) {
canOptimizeToCopy = true;
for (auto* influencedGet : trivialInfluences) {
Expand Down Expand Up @@ -156,7 +156,7 @@ struct MergeLocals

// if the trivial set we added has influences, it means $y lives on
if (!trivialInfluences.empty()) {
auto& copyInfluences = preGraph.setInfluences[copy];
auto& copyInfluences = preGraph.getSetInfluences(copy);
if (!copyInfluences.empty()) {
bool canOptimizeToTrivial = true;
for (auto* influencedGet : copyInfluences) {
Expand Down Expand Up @@ -198,7 +198,7 @@ struct MergeLocals
LocalGraph postGraph(func, getModule());
postGraph.computeSetInfluences();
for (auto& [copy, trivial] : optimizedToCopy) {
auto& trivialInfluences = preGraph.setInfluences[trivial];
auto& trivialInfluences = preGraph.getSetInfluences(trivial);
for (auto* influencedGet : trivialInfluences) {
// verify the set
auto& sets = postGraph.getSets(influencedGet);
Expand All @@ -212,7 +212,7 @@ struct MergeLocals
}
}
for (auto& [copy, trivial] : optimizedToTrivial) {
auto& copyInfluences = preGraph.setInfluences[copy];
auto& copyInfluences = preGraph.getSetInfluences(copy);
for (auto* influencedGet : copyInfluences) {
// verify the set
auto& sets = postGraph.getSets(influencedGet);
Expand Down
2 changes: 1 addition & 1 deletion src/passes/OptimizeAddedConstants.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ struct OptimizeAddedConstants
if (add->left->is<Const>() || add->right->is<Const>()) {
// Looks like this might be relevant, check all uses.
bool canPropagate = true;
for (auto* get : localGraph->setInfluences[set]) {
for (auto* get : localGraph->getSetInfluences(set)) {
auto* parent = parents.getParent(get);
// if this is at the top level, it's the whole body - no set can
// exist!
Expand Down
4 changes: 2 additions & 2 deletions src/passes/Precompute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ struct Precompute
}
setValues[set] = values;
if (values.isConcrete()) {
for (auto* get : localGraph.setInfluences[set]) {
for (auto* get : localGraph.getSetInfluences(set)) {
work.push(get);
}
}
Expand Down Expand Up @@ -861,7 +861,7 @@ struct Precompute
if (values.isConcrete()) {
// we did!
getValues[get] = values;
for (auto* set : localGraph.getInfluences[get]) {
for (auto* set : localGraph.getGetInfluences(get)) {
work.push(set);
}
propagated = true;
Expand Down
2 changes: 1 addition & 1 deletion src/passes/SSAify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ struct SSAify : public Pass {
}

bool hasMerges(LocalSet* set, LocalGraph& graph) {
for (auto* get : graph.setInfluences[set]) {
for (auto* get : graph.getSetInfluences(set)) {
if (graph.getSets(get).size() > 1) {
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/passes/Souperify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ struct UseFinder {
return;
}
// Find all the uses of that set.
auto& gets = localGraph.setInfluences[set];
auto& gets = localGraph.getSetInfluences(set);
if (debug() >= 2) {
std::cout << "addSetUses for " << set << ", " << gets.size() << " gets\n";
}
for (auto* get : gets) {
// Each of these relevant gets is either
// (1) a child of a set, which we can track, or
// (2) not a child of a set, e.g., a call argument or such
auto& sets = localGraph.getInfluences[get]; // TODO: iterator
auto& sets = localGraph.getGetInfluences(get); // TODO: iterator
// In flat IR, each get can influence at most 1 set.
assert(sets.size() <= 1);
if (sets.size() == 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/wasm/wasm-stack-opts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ void StackIROptimizer::local2Stack() {
// used by this get and nothing else, check that.
auto& sets = localGraph.getSets(get);
if (sets.size() == 1 && *sets.begin() == set) {
auto& setInfluences = localGraph.setInfluences[set];
auto& setInfluences = localGraph.getSetInfluences(set);
// If this has the proper value of 1, also do the potentially-
// expensive check of whether we can remove this pair at all.
if (setInfluences.size() == 1 &&
Expand Down
Loading