Skip to content

Reorganize place timing files #2829

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

Merged
merged 43 commits into from
Jan 23, 2025
Merged

Conversation

soheilshahrouz
Copy link
Contributor

@soheilshahrouz soheilshahrouz commented Nov 29, 2024

vpr/src/place directory has too many files, making it difficult to locate related files. This PR organizes timing related files and classes under a new directory vpr/src/place/timing.

  • Moved placement delay model classes to place/timing/delay_model.
  • Removed timing_place.cpp/.h and created seperate files for PlacerCriticalities, PlacerSetupSlacks, and PlacerTimingCosts.
  • Some functions in physical_types_util.cpp are now implemented as member functions of structs declared in physical_types.h.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Nov 29, 2024
@github-actions github-actions bot added the libarchfpga Library for handling FPGA Architecture descriptions label Nov 30, 2024
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just some minor comments.

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks generally good. A few suggestions on commenting.
Also, src/place/timing/delay/.cpp seems like an excessively long path name.
I suggest src/place/delay/
.cpp instead; there's no real reason delay has to go under timing, and I think code is harder to discovery when the directory hierarchy gets too deep.

@soheilshahrouz
Copy link
Contributor Author

regression_tests/vtr_reg_nightly_test3/vtr_reg_qor_chain_large

Metric master.txt feature.txt
vtr_flow_elapsed_time 1 0.998737754327542
odin_synth_time
parmys_synth_time 1 1.02392185673139
abc_depth 1 1
abc_synth_time 1 0.984601844021888
num_clb 1 1
num_memories 1 1
num_mult 1 1
max_vpr_mem 1 0.993918915469413
num_pre_packed_blocks 1 1
num_post_packed_blocks 1 1
device_grid_tiles 1 1
pack_time 1 0.99066781822591
placed_wirelength_est 1 1
place_time 1 1.00280780264552
placed_CPD_est 1 1
min_chan_width 1 1
routed_wirelength 1 1
min_chan_width_route_time 1 0.995261447731316
crit_path_routed_wirelength 1 1
critical_path_delay 1 1
geomean_nonvirtual_intradomain_critical_path_delay 1 1
crit_path_route_time 1 0.990615820824036

@soheilshahrouz soheilshahrouz changed the title [WIP] Reorganize place timing files Reorganize place timing files Jan 20, 2025
@vaughnbetz
Copy link
Contributor

Looks like NO_GRAPHICS has a compile error. Otherwise good to merge once that is fixed.

@soheilshahrouz
Copy link
Contributor Author

@vaughnbetz
The compilation error in NO_GRAPHICS is fixed. Ready to merge into master.

@vaughnbetz
Copy link
Contributor

Will merge once CI finishes.

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@soheilshahrouz soheilshahrouz merged commit 9761055 into master Jan 23, 2025
36 checks passed
@soheilshahrouz soheilshahrouz deleted the temp_organize_place_timng branch January 23, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants