Skip to content

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.

4 participants