verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
1.02k stars 392 forks source link

Specify different switch type to drive wires with different directions #2647

Closed saaramahmoudi closed 2 months ago

saaramahmoudi commented 4 months ago

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:

<segment name="L4" freq="0.80" length="4" type="unidir" Rmetal="0" Cmetal="0">
   <mux name="L4_mux"/>
   <sb type="pattern">1 1 1 1 1</sb>
   <cb type="pattern">1 1 1 1</cb>
</segment>

This PR introduces new tags called and 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:

<segment name="L4" freq="0.80" length="4" type="unidir" Rmetal="0" Cmetal="0">
   <mux_inc name="L4_mux_inc"/>
   <mux_dec name="L4_mux_dec"/>
   <sb type="pattern">1 1 1 1 1</sb>
   <cb type="pattern">1 1 1 1</cb>
</segment>

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?

  1. I ran VTR benchmarks and generated the RR graph with both the master branch and my branch, and they looked identical. Hence, without changing the architecture file, we expect no QoR change.
  2. I used my syntax to define different muxes for incremental and decremental wires, and generated the RR graph. I used a script to check whether the switch id within each RR edge matches the one defined in the architecture file based on direction.

Types of changes

vaughnbetz commented 4 months ago

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.

saaramahmoudi commented 4 months ago

@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.

vaughnbetz commented 4 months ago

@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.

saaramahmoudi commented 4 months ago

@vaughnbetz @AlexandreSinger Sorry about that, will avoid pushing multiple commits in a branch that is a PR in the future.

vaughnbetz commented 4 months ago

No problem, and it might be a red herring. Alex is trying to track this down, but it is a weird one.

saaramahmoudi commented 3 months ago

@vaughnbetz Can you please merge this when the CI turned green?

vaughnbetz commented 3 months ago

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?

saaramahmoudi commented 2 months ago

@vaughnbetz I added your new suggestions, and CIs are green (Nightly tests are also green). Could you please merge this PR?

vaughnbetz commented 2 months ago

Thanks!