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

Inverse of the average number of tracks per channel #2741

Merged
merged 20 commits into from
Oct 11, 2024

Conversation

soheilshahrouz
Copy link
Contributor

@soheilshahrouz soheilshahrouz commented Sep 22, 2024

Before this PR, calls to std::min() and std::max() clipped bounding box coordinates between 1 and grid width/height - 2. This PR removes all these calls and initalizes chanx_place_cost_fac_ and chany_place_cost_fac_ in a way that is compatible with the new index range: [-1, grid width/height - 1].

@soheilshahrouz soheilshahrouz changed the title Inverse of the average number of tracks per channel [WIP] Inverse of the average number of tracks per channel Sep 22, 2024
@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code libvtrutil labels Sep 22, 2024
@soheilshahrouz
Copy link
Contributor Author

soheilshahrouz commented Sep 23, 2024

QoR is compared with #2676

This is measured before inlining operator() of ChanPlaceCostFacContainer class.

Titan Quick QoR Pack Time Place Time Place WL Place CPD Place Mem N Swaps
net_cost_handler 653.0400675 1268.961831 2496205.323 17.12522422 5491.752447 18117190.22
chan_fac 655.37998 1270.253766 2496205.323 17.12522422 5491.853519 18117190.22
ratio 1.0036 1.001 1 1 1.000018 1

@soheilshahrouz
Copy link
Contributor Author

soheilshahrouz commented Sep 23, 2024

/vtr_reg_nightly_test3/vtr_reg_qor_chain_large
QoR is compared with #2676
This is measured after inlining operator() of ChanPlaceCostFacContainer class.

VTR Large Pack Time Place Time Place WL Place CPD Place Mem N Swaps
vtr_net 6.43E+01 69.57212354 231158.1631 20.42328639 685.6120645 2080613.564
vtr_chanxy 6.49E+01 69.04388726 234105.7188 19.79841244 685.3785724 2090490.813
1.01E+00 0.9924 1.01275 0.9694 0.9997 1.0047

@soheilshahrouz soheilshahrouz changed the title [WIP] Inverse of the average number of tracks per channel Inverse of the average number of tracks per channel Sep 23, 2024
@soheilshahrouz
Copy link
Contributor Author

QoR is compared with #2676

This is measured after inlining operator() of ChanPlaceCostFacContainer class.

Test Case Pack Time Place Time Place WL Place CPD Place Memory Number of Swaps
titan_net_inline 650.514155 1269.200989 2496205.323 17.12522422 5491.663657 18117190.22
titan_chanxy_inline 653.6507028 1260.628876 2496205.323 17.12522422 5491.771652 18117190.22
ratio 1.004821644 0.993246056 1 1 1.000019665 1

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.

The code all looks good -- I have no comments to address :).

@vaughnbetz
Copy link
Contributor

Whoops, I lied. I think the [-1, ...] indexed Matrix you want can just use the VtrNdOffsetMatrix, and doesn't need a new class.

@vaughnbetz
Copy link
Contributor

Ah, I see. Maybe then the new class you've created should be pulled out into a new utility class that allows -ve offsets on top of a VtrNdMatrix?

@vaughnbetz
Copy link
Contributor

Thanks! The changes to generalize and use nd_offset_matrix look good.

@vaughnbetz
Copy link
Contributor

Looks like there are some memory faults / assertion failures. From the valgrind run:

Command being timed: "valgrind --leak-check=full --suppressions=/home/runner/work/vtr-verilog-to-routing/vtr-verilog-to-routing/vpr/valgrind.supp --error-exitcode=1 --errors-for-leak-kinds=none --track-origins=yes --log-file=valgrind.log --error-limit=no /home/runner/work/vtr-verilog-to-routing/vtr-verilog-to-routing/vtr_flow/scripts/blackbox_latches.pl --input 1_stereovision3.abc.blif --output stereovision3.abc.blif --vanilla"

2024-09-26T17:27:08.5689907Z User time (seconds): 0.48
2024-09-26T17:27:08.5690506Z System time (seconds): 0.03
2024-09-26T17:27:08.5691076Z Percent of CPU this job got: 99%
2024-09-26T17:27:08.5691747Z Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.52
2024-09-26T17:27:08.5692402Z Average shared text size (kbytes): 0
2024-09-26T17:27:08.5692996Z Average unshared data size (kbytes): 0
2024-09-26T17:27:08.5693605Z Average stack size (kbytes): 0
2024-09-26T17:27:08.5694154Z Average total size (kbytes): 0
2024-09-26T17:27:08.5694725Z Maximum resident set size (kbytes): 161656
2024-09-26T17:27:08.5695506Z Average resident set size (kbytes): 0
2024-09-26T17:27:08.5696097Z Major (requiring I/O) page faults: 0
2024-09-26T17:27:08.5696702Z Minor (reclaiming a frame) page faults: 7996
2024-09-26T17:27:08.5697324Z Voluntary context switches: 1
2024-09-26T17:27:08.5697839Z Involuntary context switches: 4
2024-09-26T17:27:08.5698355Z Swaps: 0
2024-09-26T17:27:08.5698725Z File system inputs: 0
2024-09-26T17:27:08.5699186Z File system outputs: 80
2024-09-26T17:27:08.5699745Z Socket messages sent: 0
2024-09-26T17:27:08.5700229Z Socket messages received: 0
2024-09-26T17:27:08.5700720Z Signals delivered: 0
2024-09-26T17:27:08.5701175Z Page size (bytes): 4096
2024-09-26T17:27:08.5701620Z Exit status: 0
2024-09-26T17:27:17.4065810Z /home/runner/work/vtr-verilog-to-routing/vtr-verilog-to-routing/libs/libvtrutil/src/vtr_ndoffsetmatrix.h:389 operator[]: Assertion 'this->dim_size(0) > 0' failed (Can not index into size zero dimension).
2024-09-26T17:27:17.6700617Z Command terminated by signal 6
2024-09-26T17:27:17.6706561Z Command being timed: "valgrind --leak-check=full --suppressions=/home/runner/work/vtr-verilog-to-routing/vtr-verilog-to-routing/vpr/valgrind.supp --error-exitcode=1 --errors-for-leak-kinds=none --track-origins=yes --log-file=valgrind.log --error-limit=no /home/runner/work/vtr-verilog-to-routing/vtr-verilog-to-routing/vpr/vpr k6_frac_N10_frac_chain_mem32K_40nm.xml ch_intrinsics --circuit_file ch_intrinsics.pre-vpr.blif --min_route_chan_width_hint 44"

@vaughnbetz
Copy link
Contributor

Update: seg fault fixed, @soheilshahrouz gathering updated QoR to be safe.

@vaughnbetz
Copy link
Contributor

@soheilshahrouz : if you post the QoR data (and it looks good) we can merge this.

@soheilshahrouz
Copy link
Contributor Author

titan_quick_qor pack-time place time place WL place cpd place_mem n_swaps
master 501.2210495 1183.176432 2496483.069 17.05656962 5501.755819 18137699.29
PR 510.2875014 1206.429456 2496483.069 17.05656962 5501.687095 18137699.29
ratio 1.018 1.019 1 1 0.9999875 1

@vaughnbetz
Copy link
Contributor

LGTM!

@vaughnbetz vaughnbetz merged commit f337eb3 into master Oct 11, 2024
37 of 53 checks passed
@vaughnbetz vaughnbetz deleted the temp_chanx_place_cost_fac branch October 11, 2024 21:48
@AlexandreSinger
Copy link
Contributor

AlexandreSinger commented Oct 12, 2024

@vaughnbetz Sorry for all the messages; but I think this PR was the one that is causing the CI to fail now. The CI run for this PR was a week old and since then there have been golden result updates and other minor changes which seems to have been revealed by this PR.

@fkosar-ql Sorry for wrongly accusing your PR!

I am currently regenerating the golden results for the Strong tests since that is easy to fix. The issue with the basic test is probably something I cannot easily fix. Issue #2754 is tracking the issue.

Edit:
see PR #2768

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code libvtrutil VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants