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.

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

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 4, 2024

This replaces direct access of the data structure graph.*influences[foo] with a call
graph.get*influences(foo). This will allow a later PR to make those calls optionally
lazy.

@kripken kripken requested a review from tlively September 4, 2024 16:22
@@ -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.

@kripken kripken merged commit 0812ad3 into WebAssembly:main Sep 4, 2024
13 checks passed
@kripken kripken deleted the nfc.lg.influences branch September 4, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants