Clean up and Document Placement #2317
Conversation
|
@vaughnbetz can you please publish your review so I can take actions? |
| @@ -666,6 +666,8 @@ static void SetupPlacerOpts(const t_options& Options, t_placer_opts* PlacerOpts) | |||
| PlacerOpts->place_constraint_subtile = Options.place_constraint_subtile; | |||
| PlacerOpts->floorplan_num_horizontal_partitions = Options.floorplan_num_horizontal_partitions; | |||
| PlacerOpts->floorplan_num_vertical_partitions = Options.floorplan_num_vertical_partitions; | |||
|
|
|||
| PlacerOpts->seed = Options.Seed; | |||
There was a problem hiding this comment.
- Please check if we can remove the PlacerOpts->seed line from the placer now (should be able to). Best to remove it in that case to avoid confusion.
- ToDo: document:
- place_cost_exp: wiring cost is divided by the average channel width over a net's bounding box taken to this exponent. Default: 1. Only impacts devices with different channel widths in different directions or regions.
- Document RLPlace options if they aren't already.
There was a problem hiding this comment.
@saaramahmoudi : placerOpts->seed setting line is now removed from the placer. If the two documentation updates can be made we can merge this. Or if that will take a while please create an issue to track them and I'll merge this.
There was a problem hiding this comment.
@vaughnbetz will fix the documentation today and will mention you when it is done.
|
@vaughnbetz The RLPlace command-line options are now added to the documentation. I also added NoC options to documentation since they weren't available either and fixed some traffic flow typos. |
There was a problem hiding this comment.
Looks good, except for a few wording nits.
Also, @Srivat97 has simultaneously been making a PR for NoC documentation so you'll need to sync up with him on that part.
| @@ -920,6 +980,82 @@ The following options are only valid when the placement engine is in timing-driv | |||
|
|
|||
| Name of the post-placement timing report file to generate (not generated if unspecfied). | |||
|
|
|||
|
|
|||
|
@Srivat97 : probably you haven't made a PR yet (@saaramahmoudi : Srivatsan showed me the documentation in our meeting today, but I don't see the PR either). Can you make a PR @Srivat97 ? |
|
@vaughnbetz I just did a small clean-up on NoC router description in vpr.out, I moved printing of router connections in the architecture to echo file instead of printing 100 lines in the vpr.out. Could you please take a look at it to see if that sounds good to you? |
github-actions
bot
added
the
libarchfpga
|
Looks good, thanks. Will merge after talking to Srivatsan today. |
|
@Srivat97 : I don't see a PR for the NoC documentation yet. I am going to merge this shortly; it might produce a conflict with your NoC command line option documentation but that shouldn't be too hard to resolve. |
|
@vaughnbetz ready to be merged. |
|
@vaughnbetz , @saaramahmoudi : Sorry for the delayed response. You can go ahead and merge this PR first. I'll handle the conflicts on my end. |
Description
Small PR; placer seed was always printed as 0 in vpr.out, since It was printed before It was set. Hence, the default value regardless of the command-line was printed. Set it before printing.
Types of changes
Checklist: