[XLS] Switch from external-adapter channel legalization to internal arbitration #2148
Conversation
This file contains hidden or 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
![copybara-service[bot]](https://avatars.githubusercontent.com/in/44061?s=60&v=4){kind=small}
copybara-service
bot
force-pushed
the
test_747607750
branch
3 times, most recently
from
2b3ea9d to
2813bf1
Compare
copybara-service
bot
force-pushed
the
test_747607750
branch
2 times, most recently
from
c8d2236 to
2da1275
Compare
…rbitration To support relaxed channel strictness settings (allowing multiple operations per channel per activation), we previously translated all operations to take place on their own channels, then inserted an external arbitrating adapter to mux the channels back together. This resulted in duplicated control-flow logic, and was difficult to optimize well without full-powered proc inlining - which ran into its own problems. Instead, we now implement relaxed strictnesses by implementing additional legalizations within the proc: adding token-typed state to avoid cross-activation conflicts, and adding scheduling constraints to avoid having multiple (non-mutually-exclusive) operations in the same activation trigger at the same time. To support this, we also modify codegen to allow multiple operations per channel, relying on earlier compilation steps to ensure that we never see more than one trigger in a given cycle. This passes all existing tests, except for a few where it revealed issues that were previously hidden by our choices in implementing the external adapter. We modify those tests accordingly. As best as I can tell, this should be no worse for performance than the previous version. NOTE: This will also allow us to support proven-mutually-exclusive channels with mutually-exclusive operations that can't be merged, which reduces merging to an optimization; we'll remove the forced-merge logic for this case in a followup CL. Fixes #2180 PiperOrigin-RevId: 760612461
copybara-service
bot
force-pushed
the
test_747607750
branch
from
2da1275 to
b0a0b50
Compare
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.
[XLS] Switch from external-adapter channel legalization to internal arbitration
To support relaxed channel strictness settings (allowing multiple operations per channel per activation), we previously translated all operations to take place on their own channels, then inserted an external arbitrating adapter to mux the channels back together. This resulted in duplicated control-flow logic, and was difficult to optimize well without full-powered proc inlining - which ran into its own problems.
Instead, we now implement relaxed strictnesses by implementing additional legalizations within the proc: adding token-typed state to avoid cross-activation conflicts, and adding scheduling constraints to avoid having multiple (non-mutually-exclusive) operations in the same activation trigger at the same time. To support this, we also modify codegen to allow multiple operations per channel, relying on earlier compilation steps to ensure that we never see more than one trigger in a given cycle.
This passes all existing tests, except for a few where it revealed issues that were previously hidden by our choices in implementing the external adapter. We modify those tests accordingly. As best as I can tell, this should be no worse for performance than the previous version.
NOTE: This will also allow us to support proven-mutually-exclusive channels with mutually-exclusive operations that can't be merged, which reduces merging to an optimization; we'll remove the forced-merge logic for this case in a followup CL.
Fixes #2180