[AP][Solver][3D] 3D AP Solver Support #3265
Conversation
Upgraded the solver in the AP flow to support a "Z" dimension representing the layer. This code only kicks on when the architecture has more than one layer, since in the single layer case there is no point to compute the Z dimension.
github-actions
bot
added
VPR
vaughnbetz
left a comment
vaughnbetz
left a comment
There was a problem hiding this comment.
LGTM, but a few comments embedded.
Should also add a before and after runtime etc. comparison on the 2D case.
...sks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/constraints/mm9a_io_constraint.xml
Show resolved
Hide resolved
|
I think this one is almost good to go ... some comments to address, and long term at least the cost scaling factors should come from rr_graph computations / refactored annealing code that gets the scaling factors based on routing supply. |
The channel cost factor code is useful outside of the net cost handler code. Separated the code out into its own class so it can be used in AP.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request adds 3D/multi-die support to VPR's analytical placement solver. A new ChanCostHandler class centralizes channel cost factor computations across x, y, and z dimensions. The B2B solver now manages a third dimension (z-layer) alongside x/y coordinates through extended linear system construction, dimension-aware solving, and per-dimension result storage. Initial placement and cost calculations are adapted to incorporate layer information while maintaining backward compatibility with 2D placements. Changes
Sequence DiagramsequenceDiagram
participant Solver as B2B Solver
participant CostHandler as ChanCostHandler
participant LinSys as Linear System (CG)
participant Storage as Placement Storage
Note over Solver: 2D Flow
Solver->>LinSys: Build A_sparse_x, b_x
Solver->>CostHandler: get_chanx_cost_fac(bb)
Solver->>LinSys: Solve CG (x-dimension)
Solver->>Storage: store_solution_into_placement(x_soln, block_x_locs, x_max)
Solver->>LinSys: Build A_sparse_y, b_y
Solver->>CostHandler: get_chany_cost_fac(bb)
Solver->>LinSys: Solve CG (y-dimension)
Solver->>Storage: store_solution_into_placement(y_soln, block_y_locs, y_max)
Note over Solver,Storage: 3D Flow (if multi_die)
alt is_multi_die() == true
Solver->>Solver: Initialize A_sparse_z, b_z
Solver->>CostHandler: get_chanz_cost_fac(bb)
rect rgb(100, 150, 200)
Solver->>LinSys: Build z linear system
Solver->>LinSys: Solve CG (z-dimension)
end
Solver->>Storage: store_solution_into_placement(z_soln, block_z_locs, z_max)
end
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
vpr/src/place/chan_cost_handler.h (1)
89-100: Minor inconsistency: usingfloatcast but returningdouble.On line 99, the division uses
static_cast<float>but the function returnsdouble. For consistency with the other cost factor methods, consider usingdoublethroughout.🔎 Proposed fix
int bb_num_tiles = (bb.xmax - bb.xmin + 1) * (bb.ymax - bb.ymin + 1); - return bb_num_tiles / static_cast<float>(num_inter_dir_conn); + return static_cast<double>(bb_num_tiles) / num_inter_dir_conn;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
vpr/src/analytical_place/analytical_solver.cpp(25 hunks)vpr/src/analytical_place/analytical_solver.h(5 hunks)vpr/src/place/chan_cost_handler.cpp(1 hunks)vpr/src/place/chan_cost_handler.h(1 hunks)vpr/src/place/initial_placement.cpp(1 hunks)vpr/src/place/net_cost_handler.cpp(3 hunks)vpr/src/place/net_cost_handler.h(2 hunks)vtr_flow/tasks/regression_tests/vtr_reg_strong/basic_ap/config/golden_results.txt(1 hunks)vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/config/config.txt(2 hunks)vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/config/golden_results.txt(1 hunks)vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/constraints/mm9a_io_constraint.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
vpr/src/analytical_place/analytical_solver.h (1)
vpr/src/analytical_place/analytical_solver.cpp (6)
solve_linear_system(804-821)solve_linear_system(804-806)store_solution_into_placement(506-536)store_solution_into_placement(506-508)store_solution_into_placement(1369-1392)store_solution_into_placement(1369-1371)
🪛 LanguageTool
vtr_flow/tasks/regression_tests/vtr_reg_strong/basic_ap/config/golden_results.txt
[grammar] ~5-~5: After the number ‘5’, use a plural noun. Did you mean “successes”?
Context: ... 37940 -1 -1 45 162 0 5 success v8.0.0-14549-gf2054f2de-dirty relea...
(CD_NNU)
vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/config/golden_results.txt
[grammar] ~2-~2: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ... router_lookahead_computation_time 3d_k4_N4_90nm.xml s820.blif common_--a...
(THREE_D)
[grammar] ~3-~3: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...8 0.0107952 28.4 MiB -1 0.02 3d_k4_N4_90nm.xml s820.blif common_--a...
(THREE_D)
[grammar] ~4-~4: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...97 0.011807 28.5 MiB -1 0.02 3d_k4_N4_90nm.xml s820.blif common_--a...
(THREE_D)
[grammar] ~5-~5: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...2 0.0118082 28.4 MiB -1 0.02 3d_k4_N4_90nm.xml s838.1.blif common_-...
(THREE_D)
[grammar] ~6-~6: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...9 0.0113223 28.5 MiB -1 0.02 3d_k4_N4_90nm.xml s838.1.blif common_-...
(THREE_D)
[grammar] ~7-~7: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...1 0.0162105 28.4 MiB -1 0.03 3d_k4_N4_90nm.xml s838.1.blif common_-...
(THREE_D)
[grammar] ~8-~8: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...3 0.0186984 28.6 MiB -1 0.03 3d_k4_N4_90nm.xml bw.blif common_--ap_...
(THREE_D)
[grammar] ~9-~9: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...7 0.0191361 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml bw.blif common_--ap_...
(THREE_D)
[grammar] ~10-~10: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...8 0.0222217 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml bw.blif common_--ap_...
(THREE_D)
[grammar] ~11-~11: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...8 0.0218322 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml rd84.blif common_--a...
(THREE_D)
[grammar] ~12-~12: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...6 0.0301007 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml rd84.blif common_--a...
(THREE_D)
[grammar] ~13-~13: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...1 0.0301413 28.4 MiB -1 0.03 3d_k4_N4_90nm.xml rd84.blif common_--a...
(THREE_D)
[grammar] ~14-~14: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...8 0.0299277 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml s832.blif common_--a...
(THREE_D)
[grammar] ~15-~15: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...2 0.0267563 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml s832.blif common_--a...
(THREE_D)
[grammar] ~16-~16: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...1 0.0253975 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml s832.blif common_--a...
(THREE_D)
[grammar] ~17-~17: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...9 0.0247701 28.8 MiB -1 0.03 3d_k4_N4_90nm.xml mm9a.blif common_--a...
(THREE_D)
[grammar] ~18-~18: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...3 0.0303441 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml mm9a.blif common_--a...
(THREE_D)
[grammar] ~19-~19: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...3 0.0307423 28.8 MiB -1 0.03 3d_k4_N4_90nm.xml mm9a.blif common_--a...
(THREE_D)
[grammar] ~20-~20: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...5 0.0357478 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml alu2.blif common_--a...
(THREE_D)
[grammar] ~21-~21: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...4 0.0365584 28.4 MiB -1 0.03 3d_k4_N4_90nm.xml alu2.blif common_--a...
(THREE_D)
[grammar] ~22-~22: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...7 0.0437696 28.8 MiB -1 0.03 3d_k4_N4_90nm.xml alu2.blif common_--a...
(THREE_D)
[grammar] ~23-~23: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...7 0.0387797 28.6 MiB -1 0.03 3d_k4_N4_90nm.xml x1.blif common_--ap_...
(THREE_D)
[grammar] ~24-~24: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...8 0.0279211 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml x1.blif common_--ap_...
(THREE_D)
[grammar] ~25-~25: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...74 0.028147 28.4 MiB -1 0.03 3d_k4_N4_90nm.xml x1.blif common_--ap_...
(THREE_D)
[grammar] ~26-~26: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...4 0.0289236 28.6 MiB -1 0.03 3d_k4_N4_90nm.xml t481.blif common_--a...
(THREE_D)
[grammar] ~27-~27: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...3 0.0427232 28.6 MiB -1 0.03 3d_k4_N4_90nm.xml t481.blif common_--a...
(THREE_D)
[grammar] ~28-~28: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...2 0.0474837 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml t481.blif common_--a...
(THREE_D)
[grammar] ~29-~29: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...7 0.0428633 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml mm9b.blif common_--a...
(THREE_D)
[grammar] ~30-~30: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...7 0.0445339 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml mm9b.blif common_--a...
(THREE_D)
[grammar] ~31-~31: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...4 0.0448781 28.4 MiB -1 0.03 3d_k4_N4_90nm.xml mm9b.blif common_--a...
(THREE_D)
[grammar] ~32-~32: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...6 0.0450357 28.4 MiB -1 0.03 3d_k4_N4_90nm.xml styr.blif common_--a...
(THREE_D)
[grammar] ~33-~33: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...1 0.0451644 28.8 MiB -1 0.03 3d_k4_N4_90nm.xml styr.blif common_--a...
(THREE_D)
[grammar] ~34-~34: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...6 0.0448722 28.5 MiB -1 0.03 3d_k4_N4_90nm.xml styr.blif common_--a...
(THREE_D)
[grammar] ~35-~35: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...2 0.0445067 28.6 MiB -1 0.03 3d_k4_N4_90nm.xml s953.blif common_--a...
(THREE_D)
[grammar] ~36-~36: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...4 0.0482236 28.3 MiB -1 0.03 3d_k4_N4_90nm.xml s953.blif common_--a...
(THREE_D)
[grammar] ~37-~37: Did you mean “3D”(= three-dimensional) or “3rd” (= third)?
Context: ...2 0.0467707 28.8 MiB -1 0.03 3d_k4_N4_90nm.xml s953.blif common_--a...
(THREE_D)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Image
🔇 Additional comments (36)
vtr_flow/tasks/regression_tests/vtr_reg_strong/basic_ap/config/golden_results.txt (1)
2-5: Verify that golden results updates have been validated against 3D AP Solver changes.The updated rows (lines 2–5) reflect changes to vpr_revision, vpr_build_info, vpr_compiled timestamps, and numerous numeric metrics (ap_time, place_time, routing results, timing analysis, etc.). While golden results updates are expected when solver logic changes, these specific numeric values should be validated.
Please confirm:
- That these golden results have been validated against actual test runs with the new 3D AP Solver.
- That the numeric changes (ap_time, place_time, wirelength, memory usage, etc.) represent expected improvements or expected performance trade-offs from the 3D solver implementation, not regressions.
- That the test suite has passed cleanly with these updated golden values.
Additionally, the AI-generated summary references code changes in
src/calculator.py(method additions, signature changes, variable renaming) that do not appear to be part of the 3D AP Solver PR scope. Please clarify whether these references are erroneous or if there are additional related changes not visible in the provided file.vpr/src/place/initial_placement.cpp (1)
663-672: LGTM! Clean 3D extension of centroid projection.The layer projection logic correctly follows the same pattern as the x and y dimensions. The distance metric now properly accounts for layer displacement using Manhattan distance in 3D (dx + dy + dlayer), which aligns with the PR's objective to support multi-layer architectures.
vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/config/config.txt (2)
30-33: LGTM! IO constraint binding for 3D AP testing.The TODO comment appropriately documents the temporary nature of this single-circuit constraint binding. The relative path is correct for the VTR task directory structure.
50-51: LGTM! New test configuration exercises the 3D solver.This configuration omits
--ap_analytical_solver identity(unlike lines 47 and 49), which means it will use the default B2B solver to verify 3D support works correctly with the actual solver implementation.vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/constraints/mm9a_io_constraint.xml (1)
1-70: Overall structure looks good for 3D AP testing.The constraint file properly defines partitioned IO regions on layer 1 with inputs on the left edge (x=0) and outputs on the right edge (x=9). The layer constraints (
layer_low="1" layer_high="1") correctly target the IO layer for 3D placement testing.vtr_flow/tasks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/config/golden_results.txt (1)
1-37: LGTM! Golden results capture expected 3D AP test outcomes.The results file correctly reflects the three test configurations from
config.txt, with each of the 12 circuits tested against all three parameter sets. All tests reportsuccessstatus, confirming the 3D AP flow functions correctly.Note: The static analysis hints about "3d" vs "3D" are false positives—
3d_k4_N4_90nm.xmlis a filename identifier, not prose text.vpr/src/place/chan_cost_handler.cpp (2)
1-10: LGTM!File header and includes are well-structured. The documentation is clear.
24-46: LGTM!The prefix-sum initialization for
acc_chanx_width_andacc_chany_width_correctly handles the zero-width case by substituting 1 to avoid division-by-zero in the cost factor calculations.vpr/src/place/net_cost_handler.cpp (3)
93-97: LGTM!The constructor properly initializes
chan_cost_handler_with the required device context parameters. The initialization order in the member initializer list is correct.
1250-1257: LGTM!The cost factor lookups are cleanly delegated to
chan_cost_handler_. The 3D cost factor (chanz_cost_fac) is correctly only computed whenis_multi_layer_is true.
1296-1300: LGTM!Per-layer bounding box cost calculation correctly uses the
chan_cost_handler_for cost factor retrieval.vpr/src/place/net_cost_handler.h (2)
8-8: LGTM!Appropriate include for the new
ChanCostHandlerclass.
235-236: LGTM!The
chan_cost_handler_member is appropriately placed in the private section and well-documented. This cleanly encapsulates channel cost factor computations.vpr/src/place/chan_cost_handler.h (3)
1-14: LGTM!Header documentation and forward declarations are well-structured.
20-42: LGTM!The class design with a deleted default constructor and a parameterized constructor ensures proper initialization. The constructor parameters are well-documented.
53-74: LGTM!The templated cost factor getters for x and y channels cleanly support both
t_bbandt_2D_bbbounding box types. The formulas correctly compute the inverse of average channel width.vpr/src/analytical_place/analytical_solver.h (8)
13-15: LGTM!Appropriate includes added for
ChanCostHandlerandglobals.h(needed forg_vpr_ctxaccess in the constructor).
550-554: LGTM!The
chan_cost_handler_is properly initialized in the constructor initializer list using device context data. This aligns with the approach inNetCostHandler.
708-733: LGTM!The refactored method signatures are well-documented. The per-dimension
store_solution_into_placementmethod provides a clean abstraction for handling x, y, and z dimensions uniformly.
740-742: LGTM!The
is_multi_die()helper provides a clean abstraction for the multi-die check. This addresses the previous review comment about documenting when z values are used.
755-762: LGTM!The z-dimension matrices (
A_sparse_z,b_z) are appropriately added alongside the existing x and y dimension matrices.
769-778: Documentation addresses prior review feedback.The comments on lines 769 and 777 noting that z vectors are unused for single-die devices addresses the previous review comment from vaughnbetz requesting this clarification.
803-806: LGTM!The
chan_cost_handler_member is appropriately placed and documented.
731-733: The code is correct. All uses of the loop variable instore_solution_into_placementconsistently userow_id_idx(lines 1385–1386 showdim_soln[row_id_idx] = dim_max_pos - epsilon;). There is no typo, and the variablerow_idxdoes not appear in this file.Likely an incorrect or invalid review comment.
vpr/src/analytical_place/analytical_solver.cpp (12)
22-22: LGTM! Channel cost handler inclusion aligns with past review feedback.This include enables per-dimension channel cost factors as discussed in the previous review comments, addressing the concern about how cost factors are set.
598-607: LGTM! Safe multi-die initialization.The z-dimension initialization follows the same pattern as x and y, placing blocks at the device center when there are no fixed blocks. The
is_multi_die()guard ensures backward compatibility.
626-635: LGTM! Consistent z-dimension solution storage.The code properly stores both legalized and solved z-positions, maintaining symmetry with x/y handling and preserving 2D behavior when multi-die is disabled.
Also applies to: 644-646
693-701: LGTM! Proper z-dimension integration into B2B solve loop.The z-dimension guess initialization, solving, and updates follow the same pattern as x and y dimensions, with appropriate guards for backward compatibility.
Also applies to: 742-752, 774-776
787-789: LGTM! Disconnected blocks handled consistently.Disconnected blocks are placed at the device center (first iteration) or restored to their legalized positions (subsequent iterations), matching x/y behavior.
Also applies to: 797-799
804-821: LGTM! Good refactoring for per-dimension solving.Extracting the linear system solve logic into a dedicated helper reduces code duplication and centralizes iteration tracking and logging. This improves maintainability as the same logic is now used for x, y, and z dimensions.
836-839: LGTM! Clean extension of net bounds to z-dimension.The z-dimension bounds tracking follows the same pattern as x and y dimensions, including proper tie-breaking logic and validation assertions. This maintains consistency across all three dimensions.
Also applies to: 864-896, 909-912, 921-921
984-985: LGTM! Appropriate safeguards for unsupported 3D timing paths.These assertions properly prevent silent failures in timing-driven code paths that haven't been extended to support multi-die architectures yet. The error messages are clear and consistent.
Also applies to: 1118-1120, 1272-1273
1205-1262: LGTM! Per-dimension channel cost factors properly integrated.The code correctly computes and applies per-dimension channel cost factors for both wirelength and timing connections. The normalization strategy using device-wide averages is sound, and the z-dimension handling is properly guarded.
Also applies to: 1322-1326
1361-1365: LGTM! Z-dimension anchors properly integrated.The anchor weights for the z-dimension follow the same pattern as x and y, connecting moveable blocks to their legalized layer positions. This maintains consistency across all dimensions.
1369-1392: LGTM! Good refactoring of solution storage.The updated signature makes
store_solution_into_placementdimension-agnostic by accepting the target vector and max position as parameters. This reduces code duplication and improves maintainability. The epsilon-based clamping prevents round-off errors.
1178-1234: Remove this comment —chan_cost_handler_is properly initialized in the constructor's member initializer list in the header file (lines 551–554 of analytical_solver.h), using values from the global VPR context and constructor parameters.
...sks/regression_tests/vtr_reg_strong/strong_ap/basic_3d_ap/constraints/mm9a_io_constraint.xml
Show resolved
Hide resolved
|
@amin1377 Can you please review when you have a moment. I changed a bit of the code in the placer to expose the channel cost factors to Analytical Placement. |
2 participants
Upgraded the solver in the AP flow to support a "Z" dimension representing the layer. This code only kicks on when the architecture has more than one layer, since in the single layer case there is no point to compute the Z dimension.
Summary by CodeRabbit
Release Notes
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.