Skip to content
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

Placer class #2813

Merged
merged 35 commits into from
Nov 28, 2024
Merged

Placer class #2813

merged 35 commits into from
Nov 28, 2024

Conversation

soheilshahrouz
Copy link
Contributor

Add Placer class that encapsulates all objects and variable that are placement instance specific. Shared objects like delay model are still constructed outside this class and shared across all Placer instances.

Don't look at the code yet; it's a mess! :)

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions lang-cpp C/C++ code external_libs labels Nov 18, 2024
/* Allocated here because it goes into timing critical code where each memory allocation is expensive */
IntraLbPbPinLookup pb_gpin_lookup(device_ctx.logical_block_types);
//Enables fast look-up of atom pins connect to CLB pins
ClusteredPinAtomPinsLookup netlist_pin_lookup(cluster_ctx.clb_nlist, atom_ctx.nlist, pb_gpin_lookup);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vaughnbetz
pb_gpin_lookup and netlist_pin_lookup are initialized after the placement timer starts in line 215. Is their initialization times intentionally included in the total placement time?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably just what was convenient. They can be moved outside placement if it cleans up the code, but please measure how long they take and if it can be significant they should get their own timers (either on their own or together).

@soheilshahrouz
Copy link
Contributor Author

titan_quick_qor

Metric master.txt feature.txt
vtr_flow_elapsed_time 1 0.9991044425
num_LAB 1 1
num_DSP 1 1
num_M9K 1 1
num_M144K 1 1
max_vpr_mem 1 1.000029452
num_pre_packed_blocks 1 1
num_post_packed_blocks 1 1
device_grid_tiles 1 1
pack_time 1 0.9947880721
placed_wirelength_est 1 1
place_time 1 1.000120706
placed_CPD_est 1 1
routed_wirelength 1 1
critical_path_delay 1 1
geomean_nonvirtual_intradomain_critical_path_delay 1 1
crit_path_route_time 1 0.9949575309

@soheilshahrouz soheilshahrouz changed the title [WIP] Placer class Placer class Nov 22, 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.

Hi @soheilshahrouz these changes look great to me! I left some minor comments. I am excited to see this PR merged!

vpr/src/place/place.cpp Outdated Show resolved Hide resolved
vpr/src/place/place.cpp Outdated Show resolved Hide resolved
vpr/src/place/place.cpp Outdated Show resolved Hide resolved
vpr/src/place/place_log_util.cpp Outdated Show resolved Hide resolved
vpr/src/place/place_log_util.cpp Outdated Show resolved Hide resolved
vpr/src/place/placer.h Outdated Show resolved Hide resolved
vpr/src/place/placer.h Outdated Show resolved Hide resolved
vpr/src/place/placer.cpp Show resolved Hide resolved
vpr/src/place/placer.cpp Outdated Show resolved Hide resolved
vpr/src/place/placer.cpp Outdated Show resolved Hide resolved
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 good; some suggested commenting and a few minor tweaks.

vpr/src/place/place.cpp Outdated Show resolved Hide resolved
vpr/src/place/place.cpp Show resolved Hide resolved
vpr/src/place/place.cpp Show resolved Hide resolved
vpr/src/place/place_delay_model.h Outdated Show resolved Hide resolved
vpr/src/place/place_log_util.h Outdated Show resolved Hide resolved
vpr/src/place/placer.cpp Outdated Show resolved Hide resolved
vpr/src/place/placer.cpp Show resolved Hide resolved
vpr/src/place/placer.cpp Outdated Show resolved Hide resolved
vpr/src/place/placer.cpp Outdated Show resolved Hide resolved
vpr/src/place/timing_place_lookup.h Outdated Show resolved Hide resolved
@soheilshahrouz
Copy link
Contributor Author

@vaughnbetz
This is ready to merge into master

@vaughnbetz vaughnbetz merged commit ec23fea into master Nov 28, 2024
37 checks passed
@vaughnbetz vaughnbetz deleted the tmep_placer_class branch November 28, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external_libs lang-cpp C/C++ code libarchfpga Library for handling FPGA Architecture descriptions libvtrutil VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants