-
Notifications
You must be signed in to change notification settings - Fork 47.4k
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
[compiler] ReactiveIR: add Let/Assign/Branch/Join nodes #32013
Open
josephsavona
wants to merge
6
commits into
gh/josephsavona/65/base
Choose a base branch
from
gh/josephsavona/65/head
base: gh/josephsavona/65/base
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The implementation ensures writes are ordered, but is missing the logic to ensure that reads are ordered. The easy thing would be to order reads the same as writes, but we want to be able to order reads relative to each other — the real ordering constraint is that an instruction read the correct write. That will take a bit more work, for a follow-up. [ghstack-poisoned]
josephsavona
added a commit
that referenced
this pull request
Jan 7, 2025
The implementation ensures writes are ordered, but is missing the logic to ensure that reads are ordered. The easy thing would be to order reads the same as writes, but we want to be able to order reads relative to each other — the real ordering constraint is that an instruction read the correct write. That will take a bit more work, for a follow-up. ghstack-source-id: 07c8efedc4300c311c946eef07326a8b0d20973b Pull Request resolved: #32013
facebook-github-bot
added
the
React Core Team
Opened by a member of the React Core Team
label
Jan 7, 2025
The implementation ensures writes are ordered, but is missing the logic to ensure that reads are ordered. The easy thing would be to order reads the same as writes, but we want to be able to order reads relative to each other — the real ordering constraint is that an instruction read the correct write. That will take a bit more work, for a follow-up. [ghstack-poisoned]
josephsavona
added a commit
that referenced
this pull request
Jan 8, 2025
The implementation ensures writes are ordered, but is missing the logic to ensure that reads are ordered. The easy thing would be to order reads the same as writes, but we want to be able to order reads relative to each other — the real ordering constraint is that an instruction read the correct write. That will take a bit more work, for a follow-up. ghstack-source-id: 8fa68d6cbba00670d59ea72cdd4f4b6669c33297 Pull Request resolved: #32013
Further exploration of the new ReactiveIR sea-of-nodes approach, adding Let/Assign/Branch/Join nodes. See the doc for more about how these work. Notable things in the implementation is that this ensures that reads and writes of variables are always ordered correctly: * Reads after writes: reads have a control dependency that ensures they come after the write they should be reading * Writes after reads: subsequent writes come after any reads of previous writes, or if no reads, after the previous write In both cases, the ordering only applies within consecutive control flow. The first read or write of a variable within a given branch of control flow will only take a dependency on the block's entry node (the default control). Not implemented yet is the logic to ensure that if there is a read within a branch, that we order the branch itself after the value that would be read (and similarly for writes), and also to construct phis for conditionally reassigned values and set the Join as the last write. [ghstack-poisoned]
josephsavona
added a commit
that referenced
this pull request
Jan 8, 2025
The implementation ensures writes are ordered, but is missing the logic to ensure that reads are ordered. The easy thing would be to order reads the same as writes, but we want to be able to order reads relative to each other — the real ordering constraint is that an instruction read the correct write. That will take a bit more work, for a follow-up. ghstack-source-id: 9ec23981561fe2ac1e5428d31c5bbcabb20699e3 Pull Request resolved: #32013
Further exploration of the new ReactiveIR sea-of-nodes approach, adding Load/Store/Branch/Join nodes. This has evolved a bit relative to the doc, but the doc still gives a decent idea (Let/Const/Assign are all collapsed to Store now), and we have an explicit Load to ensure loads are sequenced correctly. Notable things in the implementation are the ordering of variable loads/stores and scope mutations. ## Ordering Loads/Stores The rules are: * Reads after writes: reads have a control dependency that ensures they come after the write they should be reading * Writes after reads: subsequent writes come after any reads of previous writes, or if no reads, after the previous write In both cases, the ordering only applies within consecutive control flow. The first read or write of a variable within a given branch of control flow will only take a dependency on the block's entry node (the default control). This ordering extends to conditional control flow. For Join nodes, we look at all the reads/writes which occurred in any of the branches, and then consider the join itself to be either a write or read of those variables. So in the following: ```js let x = 0; if (cond) { read(x); } x = 1; ``` We establish two ordering conditions: * The Branch node for the if has a control dependency on `let x = 0`, since one of the branches needs to read that version of `x` * The Join node for the if acts as a read of `x`, such that the `x = 1` has a control dependency on the if. This ensures the write happens after the read. ## Ordering Scope Mutations The only rule here is that all the mutations affecting a particular scope must remain in their original order. Within linear control flow we accomplish this by tracking the last node which mutated each scope, and setting the previous such node as the control of the next one. For conditional control flow, we take the union of all the scopes mutated within any of the branches. The Branch node takes a control on the previous mutating node for each scope (ensuring the branch comes after those earlier mutations), and then the Join node is set as the last mutation of each scope (ensuring any subsequent mutations of those scopes stay after). ## Todos (for follow-ups) The main things not completed yet wrt to the branch/join infra are: * handling other terminal types * phis (though i'm actually wondering about converting directly to ReactiveFunction and not needing to preserve them!) [ghstack-poisoned]
Further exploration of the new ReactiveIR sea-of-nodes approach, adding Load/Store/Branch/Join nodes. This has evolved a bit relative to the doc, but the doc still gives a decent idea (Let/Const/Assign are all collapsed to Store now), and we have an explicit Load to ensure loads are sequenced correctly. Notable things in the implementation are the ordering of variable loads/stores and scope mutations. ## Ordering Loads/Stores The rules are: * Reads after writes: reads have a control dependency that ensures they come after the write they should be reading * Writes after reads: subsequent writes come after any reads of previous writes, or if no reads, after the previous write In both cases, the ordering only applies within consecutive control flow. The first read or write of a variable within a given branch of control flow will only take a dependency on the block's entry node (the default control). This ordering extends to conditional control flow. For Join nodes, we look at all the reads/writes which occurred in any of the branches, and then consider the join itself to be either a write or read of those variables. So in the following: ```js let x = 0; if (cond) { read(x); } x = 1; ``` We establish two ordering conditions: * The Branch node for the if has a control dependency on `let x = 0`, since one of the branches needs to read that version of `x` * The Join node for the if acts as a read of `x`, such that the `x = 1` has a control dependency on the if. This ensures the write happens after the read. ## Ordering Scope Mutations The only rule here is that all the mutations affecting a particular scope must remain in their original order. Within linear control flow we accomplish this by tracking the last node which mutated each scope, and setting the previous such node as the control of the next one. For conditional control flow, we take the union of all the scopes mutated within any of the branches. The Branch node takes a control on the previous mutating node for each scope (ensuring the branch comes after those earlier mutations), and then the Join node is set as the last mutation of each scope (ensuring any subsequent mutations of those scopes stay after). ## Todos (for follow-ups) The main things not completed yet wrt to the branch/join infra are: * handling other terminal types * phis (though i'm actually wondering about converting directly to ReactiveFunction and not needing to preserve them!) [ghstack-poisoned]
Further exploration of the new ReactiveIR sea-of-nodes approach, adding Load/Store/Branch/Join nodes. This has evolved a bit relative to the doc, but the doc still gives a decent idea (Let/Const/Assign are all collapsed to Store now), and we have an explicit Load to ensure loads are sequenced correctly. Notable things in the implementation are the ordering of variable loads/stores and scope mutations. ## Ordering Loads/Stores The rules are: * Reads after writes: reads have a control dependency that ensures they come after the write they should be reading * Writes after reads: subsequent writes come after any reads of previous writes, or if no reads, after the previous write In both cases, the ordering only applies within consecutive control flow. The first read or write of a variable within a given branch of control flow will only take a dependency on the block's entry node (the default control). This ordering extends to conditional control flow. For Join nodes, we look at all the reads/writes which occurred in any of the branches, and then consider the join itself to be either a write or read of those variables. So in the following: ```js let x = 0; if (cond) { read(x); } x = 1; ``` We establish two ordering conditions: * The Branch node for the if has a control dependency on `let x = 0`, since one of the branches needs to read that version of `x` * The Join node for the if acts as a read of `x`, such that the `x = 1` has a control dependency on the if. This ensures the write happens after the read. ## Ordering Scope Mutations The only rule here is that all the mutations affecting a particular scope must remain in their original order. Within linear control flow we accomplish this by tracking the last node which mutated each scope, and setting the previous such node as the control of the next one. For conditional control flow, we take the union of all the scopes mutated within any of the branches. The Branch node takes a control on the previous mutating node for each scope (ensuring the branch comes after those earlier mutations), and then the Join node is set as the last mutation of each scope (ensuring any subsequent mutations of those scopes stay after). ## Todos (for follow-ups) The main things not completed yet wrt to the branch/join infra are: * handling other terminal types * phis (though i'm actually wondering about converting directly to ReactiveFunction and not needing to preserve them!) [ghstack-poisoned]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Further exploration of the new ReactiveIR sea-of-nodes approach, adding Load/Store/Branch/Join nodes. This has evolved a bit relative to the doc, but the doc still gives a decent idea (Let/Const/Assign are all collapsed to Store now), and we have an explicit Load to ensure loads are sequenced correctly.
Notable things in the implementation are the ordering of variable loads/stores and scope mutations.
Ordering Loads/Stores
The rules are:
In both cases, the ordering only applies within consecutive control flow. The first read or write of a variable within a given branch of control flow will only take a dependency on the block's entry node (the default control).
This ordering extends to conditional control flow. For Join nodes, we look at all the reads/writes which occurred in any of the branches, and then consider the join itself to be either a write or read of those variables.
So in the following:
We establish two ordering conditions:
let x = 0
, since one of the branches needs to read that version ofx
x
, such that thex = 1
has a control dependency on the if. This ensures the write happens after the read.Ordering Scope Mutations
The only rule here is that all the mutations affecting a particular scope must remain in their original order. Within linear control flow we accomplish this by tracking the last node which mutated each scope, and setting the previous such node as the control of the next one.
For conditional control flow, we take the union of all the scopes mutated within any of the branches. The Branch node takes a control on the previous mutating node for each scope (ensuring the branch comes after those earlier mutations), and then the Join node is set as the last mutation of each scope (ensuring any subsequent mutations of those scopes stay after).
Todos (for follow-ups)
The main things not completed yet wrt to the branch/join infra are: