-
Notifications
You must be signed in to change notification settings - Fork 400
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
Specify different switch type to drive wires with different directions #2647
Conversation
…x_dec for specifiying different switches for wires with different direction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; one commenting change suggested.
You should also add a documentation update to this PR.
Finally, you should add at least one quick-running CI test that uses the feature (you could modify an existing arch file or add a new test with a small circuit and put it in vtr_reg_strong or some equivalent).
FYI, the self-hosted runners are not launching jobs right now ... not sure why. Hopefully they start soon but that's why the various _nightly runs are failing. |
@vaughnbetz Thanks for the review, I update the documentation based on your suggestion. I also added two tasks, one for strong and the other for strong_odin. Please let me know if you have more suggestions for me. |
@AlexandreSinger has a theory about what is killing CI: this PR had a bunch of commits pushed to the branch in rapid succession which would have launched CI runs that each got cancelled by the new one. @AlexandreSinger was doing a similar thing on another PR at the same time. It seems that having a lot of rapid starts & cancellations might make the CI runners unreachable (some limit in the google cloud? some race condition?) so it would be good to try to avoid a lot of rapid pushes to the same branch that is in a PR, to see if that makes this issue go away. |
@vaughnbetz @AlexandreSinger Sorry about that, will avoid pushing multiple commits in a branch that is a PR in the future. |
No problem, and it might be a red herring. Alex is trying to track this down, but it is a weird one. |
@vaughnbetz Can you please merge this when the CI turned green? |
Thanks. The google cloud runners are down again, so some runs aren't going to start. Can you run vtr_reg_strong locally (I think it is on the cloud runners) and post the results to show that it passed, including the new test? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two more minor commenting updates suggested.
@vaughnbetz I added your new suggestions, and CIs are green (Nightly tests are also green). Could you please merge this PR? |
Thanks! |
This PR aims to allow the architecture to specify more than one switch type to drive a pair of wires (assuming wires are uni-directional) based on the wire's direction.
This feature allows putting different delay numbers on the switch that drives the segment. For instance, an L4 wire in x-axis can go either right (incremental wire) or left (decremental wire), and we can put one mux for the first category and another with a different delay for the second.
Description
The current segment definition in the architecture file is shown below:
This PR introduces new tags called <mux_inc> and <mux_dec> to allow the user to specify different muxes for different directions (only meaningful for unidirectional segments). The usage for the same L4 wire would be:
This way we can exploit the VPR data structures for RR graph generation to capture timing more accurately. These changes are backward compatible and we don't have to necessarily use the new feature.
The architecture parser will look for either a general "mux" tag or both "mux_inc" and "mux_dec" should be defined. Otherwise, it will throw an error for the user. RR graph generation would consider the switch index while creating segments, and will continue as usual.
How Has This Been Tested?
Types of changes