The-OpenROAD-Project / OpenROAD

OpenROAD's unified application implementing an RTL-to-GDS Flow. Documentation at https://openroad.readthedocs.io/en/latest/
https://theopenroadproject.org/
BSD 3-Clause "New" or "Revised" License
1.4k stars 491 forks source link

Fix the issue : "mpl2 crashes with mock-array-big #3490" #3512

Closed ZhiangWang033 closed 10 months ago

ZhiangWang033 commented 12 months ago

Please update the your configuration file as follows. export PLACE_DENSITY = 0.30 export RTLMP_FLOW = True export SYNTH_HIERARCHICAL = 1 export MACRO_PLACE_HALO = 12 12

github-actions[bot] commented 12 months ago

clang-tidy review says "All clean, LGTM! :+1:"

ZhiangWang033 commented 12 months ago

Please go ahead.

On Fri, Jun 23, 2023 at 1:02 PM Ethan Mahintorabi @.***> wrote:

@.**** commented on this pull request.

In src/mpl2/src/hier_rtlmp.cpp https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*discussion_r1240315698__;Iw!!Mih3wA!Dj55sk9fHOccz79cK2yyawjXUCTHCqpwIJq-cD8lwZgvL7HrqfJsaShEvcW39FO2QOflngcQq_V4BpKJ6HZXGE-FDJo$ :

@@ -4994,9 +5007,9 @@ void HierRTLMP::callBusPlanning(std::vector& shaped_macros, void HierRTLMP::hardMacroClusterMacroPlacement(Cluster* cluster) { // Check if the cluster is a HardMacroCluster

  • if (cluster->getClusterType() != HardMacroCluster) {
  • return;
  • }
  • //if (cluster->getClusterType() != StdCellCluster) {
  • // return;

Would you mind deleting the commented lines?

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*pullrequestreview-1495913697__;Iw!!Mih3wA!Dj55sk9fHOccz79cK2yyawjXUCTHCqpwIJq-cD8lwZgvL7HrqfJsaShEvcW39FO2QOflngcQq_V4BpKJ6HZXdvt_vcU$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQR3KJVBRJYT5GI6BUL5U23XMXY47ANCNFSM6AAAAAAZR66M4Q__;!!Mih3wA!Dj55sk9fHOccz79cK2yyawjXUCTHCqpwIJq-cD8lwZgvL7HrqfJsaShEvcW39FO2QOflngcQq_V4BpKJ6HZXMuzz768$ . You are receiving this because you authored the thread.Message ID: @.***>

oharboe commented 12 months ago

This is the default macro placer. It figures out that it is an array, but then places them helter skelter in the middle:

image

This is manual placement on master:

image

This is with this PR and some changes to configuration:

$ git diff
diff --git a/flow/designs/asap7/mock-array/config.mk b/flow/designs/asap7/mock-array/config.mk
index c27c216d..6f20bdeb 100644
--- a/flow/designs/asap7/mock-array/config.mk
+++ b/flow/designs/asap7/mock-array/config.mk
@@ -26,7 +26,12 @@ BLOCKS                       = Element

 export GDS_ALLOW_EMPTY       = Element

-export MACRO_PLACEMENT_TCL   = ./designs/asap7/mock-array/macro-placement.tcl
+# export MACRO_PLACEMENT_TCL   = ./designs/asap7/mock-array/macro-placement.tcl
+
+export RTLMP_FLOW = True
+export SYNTH_HIERARCHICAL = 1
+export MACRO_PLACE_HALO = 2 2
+

 export IO_CONSTRAINTS        = designs/asap7/mock-array/io.tcl

flow/settings.mk:

export DESIGN_CONFIG=designs/asap7/mock-array/config.mk
# Routing by abutment parameters
export MOCK_ARRAY_TABLE         = 8 8 4 4 6 6
make floorplan
[deleted]
HierRTLMP Flow enabled...
[deleted]
Elapsed time: 0:01.81[h:]min:sec. CPU time: user 1.75 sys 0.05 (99%). Peak memory: 233500KB.
cp results/asap7/mock-array/base/2_6_floorplan_pdn.odb results/asap7/mock-array/base/2_floorplan.odb

image

So that looks spot on, I'd say :-)

Though.... there are some problems in detailed route. Will post details here.

There are max 15 iterations in mock-array:

[INFO DRT-0195] Start 15th optimization iteration.
[deleted]
[INFO DRT-0199]   Number of violations = 5689.

Something strage is going on...

image

Aha.... Looks like IO pin constraints for the element are not respected. Could that be because of export SYNTH_HIERARCHICAL = 1?

These routes should be straight horizontal lines:

image

ZhiangWang033 commented 12 months ago

Please use this "export MACRO_PLACE_HALO = 12 12" instead of 2

oharboe commented 12 months ago

Please use this "export MACRO_PLACE_HALO = 12 12" instead of 2

@ZhiangWang033 Then I get another error:

(/usr/bin/time -f 'Elapsed time: %E[h:]min:sec. CPU time: user %U sys %S (%P). Peak memory: %MKB.' /home/oyvind/OpenROAD-flow-scripts/tools/install/OpenROAD/bin/openroad -exit -no_init  ./scripts/macro_place.tcl -metrics ./logs/asap7/mock-array/base/2_4_mplace.json) 2>&1 | tee ./logs/asap7/mock-array/base/2_4_mplace.log
OpenROAD v2.0-8837-ga3d77f4d0 
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
HierRTLMP Flow enabled...
Call Macro Placer  -halo_width 12 -report_directory ./objects/asap7/mock-array/base/rtlmp
Floorplan Outline: (0.0, 0.0) (112.32, 112.32),  Core Outline: (2, 2) (110, 110)
Traversed logical hierarchy
    Number of std cell instances : 1
    Area of std cell instances : 0.04
    Number of macros : 64
    Area of macros : 0.00
    Total area : 0.04
    Design Utilization : 0.00
    Core Utilization: 0.00
    Manufacturing Grid: 1

Reset number of levels to 1
Reset macro_blockage_weight : 50.0
num level: 1, max_macro: 5, min_macro: 1, max_inst:5000, min_inst:1000
area_weight_ = 0.1
outline_weight_ = 100.0
wirelength_weight_ = 100.0
guidance_weight_ = 10.0
fence_weight_ = 10.0
boundary_weight_ = 50.0
notch_weight_ = 10.0
macro_blockage_weight_ = 50.0
halo_width_ = 12.0
bus_planning_flag_ = false

Create Data Flow

Perform Clustering..

Break mixed clusters into std cell and macro clusters.

Print Physical Hierachy Tree after splitting std cell and macros in leaf clusters

root  (0)  num_macro :  64   num_std_cell :  1  macro_area :  0.0  std_cell_area : 0.04374

+---L0  (1)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---L1  (2)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---L2  (3)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---T0  (4)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---T1  (5)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---T2  (6)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---R0  (7)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---R1  (8)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---R2  (9)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---B0  (10)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---B1  (11)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

+---B2  (12)  num_macro :  0   num_std_cell :  0  macro_area :  0.0  std_cell_area : 0.0

The design only has macros.

[Hier-RTLMP::HardMacroClusterMacroPlacement] Place macros in cluster: root
[ERROR MPL-0010] Cannot find valid macro placement for hard macro cluster: root
Error: macro_place.tcl, 141 MPL-0010
Command exited with non-zero status 1
Elapsed time: 0:07.53[h:]min:sec. CPU time: user 60.70 sys 0.05 (806%). Peak memory: 192976KB.
make: *** [Makefile:485: results/asap7/mock-array/base/2_4_floorplan_macro.odb] Error 1
ZhiangWang033 commented 12 months ago

I updated one line of code. Please check again. It succeeded on my side. image

github-actions[bot] commented 12 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 11 months ago

@ZhiangWang033 I tried this but I get

[ERROR MPL-0010] Cannot find valid macro placement for hard macro cluster: root
Error: macro_place.tcl, 141 MPL-0010
ZhiangWang033 commented 11 months ago

Did you update the config file ?

On Fri, Jun 23, 2023 at 6:47 PM Matt Liberty @.***> wrote:

@ZhiangWang033 https://urldefense.com/v3/__https://github.com/ZhiangWang033__;!!Mih3wA!HhNtgi30P9rkjpnZe5TW3hwyj973SresDwK2BGolyz1LRHeBroOyalC8QO9hqbq5GIk1aPvRbsOSx1mVufQBGdevKSw$ I tried this but I get

[ERROR MPL-0010] Cannot find valid macro placement for hard macro cluster: root Error: macro_place.tcl, 141 MPL-0010

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*issuecomment-1605229267__;Iw!!Mih3wA!HhNtgi30P9rkjpnZe5TW3hwyj973SresDwK2BGolyz1LRHeBroOyalC8QO9hqbq5GIk1aPvRbsOSx1mVufQBGaATeog$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQR3KJVILFHX6CWTRXQAX6DXMZBKBANCNFSM6AAAAAAZR66M4Q__;!!Mih3wA!HhNtgi30P9rkjpnZe5TW3hwyj973SresDwK2BGolyz1LRHeBroOyalC8QO9hqbq5GIk1aPvRbsOSx1mVufQBomVRFDA$ . You are receiving this because you were mentioned.Message ID: @.***>

maliberty commented 11 months ago

Yes

+export RTLMP_FLOW = True
+export SYNTH_HIERARCHICAL = 1
+export MACRO_PLACE_HALO = 12 12
+
 export DESIGN_NAME            = MockArray
 export DESIGN_NICKNAME        = mock-array

@@ -26,7 +30,7 @@ BLOCKS                       = Element

 export GDS_ALLOW_EMPTY       = Element

-export MACRO_PLACEMENT_TCL   = ./designs/asap7/mock-array/macro-placement.tcl
+#export MACRO_PLACEMENT_TCL   = ./designs/asap7/mock-array/macro-placement.tcl
ZhiangWang033 commented 11 months ago

include designs/asap7/mock-array/defaults.mk

export DESIGN_NAME = MockArray export DESIGN_NICKNAME = mock-array

export VERILOG_FILES_BLACKBOX = designs/src/mock-array/Element.v export VERILOG_FILES = designs/src/mock-array/*.v

export SDC_FILE = designs/asap7/mock-array/constraints.sdc

export PLATFORM = asap7

export PLACE_DENSITY = 0.30 export RTLMP_FLOW = True export SYNTH_HIERARCHICAL = 1 export MACRO_PLACE_HALO = 12 12

export CORE_AREA = $(shell \ export MOCK_ARRAY_HEIGHT=$(MOCK_ARRAY_HEIGHT) && \ export MOCK_ARRAY_WIDTH=$(MOCK_ARRAY_WIDTH) && \ export MOCK_ARRAY_PITCH_SCALE=$(MOCK_ARRAY_PITCH_SCALE) && \ cd designs/asap7/mock-array && \ python3 -c "import config; print(f'{config.margin_x} {config.margin_y} {config.core_width + config.margin_x} {config.core_height + config.margin_y}')") export DIE_AREA = $(shell \ export MOCK_ARRAY_HEIGHT=$(MOCK_ARRAY_HEIGHT) && \ export MOCK_ARRAY_WIDTH=$(MOCK_ARRAY_WIDTH) && \ export MOCK_ARRAY_PITCH_SCALE=$(MOCK_ARRAY_PITCH_SCALE) && \ cd designs/asap7/mock-array && \ python3 -c "import config; print(f'0 0 {config.die_width} {config.die_height}')")

BLOCKS = Element

export GDS_ALLOW_EMPTY = Element

export MACRO_PLACEMENT_TCL = ./designs/asap7/mock-array/macro-placement.tcl

export IO_CONSTRAINTS = designs/asap7/mock-array/io.tcl

export PDN_TCL = designs/asap7/mock-array/pdn.tcl export TNS_END_PERCENT = 100

Target to force generation of Verilog per user settings

MOCK_ARRAY_WIDTH and MOCK_ARRAY_HEIGHT

verilog: ./designs/asap7/mock-array/verilog.sh

If this design isn't quickly done in detailed routing, something is wrong.

At time of adding this option, only 12 iterations were needed for 0

violations.

export DETAILED_ROUTE_ARGS=-bottom_routing_layer M2 -top_routing_layer M7 -save_guide_updates -verbose 1 -droute_end_iter 15

since we are specifying DETAILED_ROUTE_ARGS, we need to communicate the

same information to other stages in the flow.

export MIN_ROUTING_LAYER = M2 export MAX_ROUTING_LAYER = M7

ZhiangWang033 commented 11 months ago

I have verified it on my side again. I did not have any problem ....

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 11 months ago

@ZhiangWang033 Your config.mk is not from the head of ORFS. I still get the same error with this PR. Please update ORFS and try again.

maliberty commented 11 months ago

@ZhiangWang033 the macro is 8.64um on a side. A halo of 12 will bloat the size so large there is no way the array could possibly fit in the core area.

maliberty commented 11 months ago

I set the halo to 1.0 and it completes but the pins are not on the routing grid: image

Is there something to ensure that?

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 11 months ago

I discussed the pin alignment issue with @ZhiangWang033 today and he will make some further updates to this PR.

ZhiangWang033 commented 11 months ago

I have updated the codes. Please take a look.

Thanks, Zhiang

On Wed, Jun 28, 2023 at 8:32 PM Matt Liberty @.***> wrote:

I discussed the pin alignment issue with @ZhiangWang033 https://urldefense.com/v3/__https://github.com/ZhiangWang033__;!!Mih3wA!EeYManXijD0_9ifkXLxblaO6Nk1_66bdlXsMqI9QNp2FcTV7MoCbmRV8d9qLi8k1B5hnAm-0jgcpjtxvFeUAQEnXK0k$ today and he will make some further updates to this PR.

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*issuecomment-1612383983__;Iw!!Mih3wA!EeYManXijD0_9ifkXLxblaO6Nk1_66bdlXsMqI9QNp2FcTV7MoCbmRV8d9qLi8k1B5hnAm-0jgcpjtxvFeUAX9c74qY$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQR3KJRLJFQHFMTMJSIPXH3XNTZNTANCNFSM6AAAAAAZR66M4Q__;!!Mih3wA!EeYManXijD0_9ifkXLxblaO6Nk1_66bdlXsMqI9QNp2FcTV7MoCbmRV8d9qLi8k1B5hnAm-0jgcpjtxvFeUAzgb7zrk$ . You are receiving this because you were mentioned.Message ID: @.***>

maliberty commented 11 months ago

@ZhiangWang033 I ran with your update and

export MACRO_PLACE_HALO = 1 1

but I still see the pins are off track: image image

maliberty commented 11 months ago

You are relying on the layer's getPitch/Offset but those are not always the ones used to generate the track pattern. In asap7 we use https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/master/flow/platforms/asap7/openRoad/make_tracks.tcl instead (due to the complex pattern on m2).

I think it would be best to see if a dbTrackGrid exists for the layer in question and take its origin & step. If more than one pattern exists then I would just ignore such a layer for this purpose (its unlikely to happen on signal pins).

If none exists then you can fall back to your current method.

ZhiangWang033 commented 11 months ago

Hi Matt,

Could you please check the latest update ?

On Thu, Jun 29, 2023 at 9:25 AM Matt Liberty @.***> wrote:

You are relying on the layer's getPitch/Offset but those are not always the ones used to generate the track pattern. In asap7 we use https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/master/flow/platforms/asap7/openRoad/make_tracks.tcl https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/master/flow/platforms/asap7/openRoad/make_tracks.tcl__;!!Mih3wA!CXLeOJ7aMIoyb3bLcSW5N0Xh5jL8VMR6m-5hVMWdkIIL4in_duombF5FabUpfVgtGBclDhx2SKO_COVJz1ONqkD0SQ4$ instead (due to the complex pattern on m2).

I think it would be best to see if a dbTrackGrid exists for the layer in question and take its origin & step. If more than one pattern exists then I would just ignore such a layer for this purpose (its unlikely to happen on signal pins).

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*issuecomment-1613492176__;Iw!!Mih3wA!CXLeOJ7aMIoyb3bLcSW5N0Xh5jL8VMR6m-5hVMWdkIIL4in_duombF5FabUpfVgtGBclDhx2SKO_COVJz1ONZo4w7KQ$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQR3KJQPGGK2254UO5XKW3DXNWT63ANCNFSM6AAAAAAZR66M4Q__;!!Mih3wA!CXLeOJ7aMIoyb3bLcSW5N0Xh5jL8VMR6m-5hVMWdkIIL4in_duombF5FabUpfVgtGBclDhx2SKO_COVJz1ONjSxmZqU$ . You are receiving this because you were mentioned.Message ID: @.***>

maliberty commented 11 months ago

@ZhiangWang033 are you unable to test your changes? Your logic is reversed in

 if (grid == nullptr) {

should be

 if (grid) {

with that fix the pins are still off: image

ZhiangWang033 commented 11 months ago

Hi Matt,

I did check the results on my side.

First, It finished the entire flow. [image: image.png]

Second, you can see the pin is on the track ...

Did I check the wrong items ?

[image: image.png] [image: image.png]

On Fri, Jun 30, 2023 at 9:17 PM Matt Liberty @.***> wrote:

@ZhiangWang033 https://urldefense.com/v3/__https://github.com/ZhiangWang033__;!!Mih3wA!CMmQJf7zvOh2VWXqVXDKgf7J8jRfh5hh7pr_xNFnc9cJddWDgHxS9Q3PdDFf-7WQh9b0Ex7LcaE4oGE_eDPxIA3ZtbY$ are you unable to test your changes? Your logic is reversed in

if (grid == nullptr) {

should be

if (grid) {

with that fix the pins are still off: [image: image] https://urldefense.com/v3/__https://user-images.githubusercontent.com/761514/250229590-4a710b24-5ea3-4f0f-89f7-59cc31eae303.png__;!!Mih3wA!CMmQJf7zvOh2VWXqVXDKgf7J8jRfh5hh7pr_xNFnc9cJddWDgHxS9Q3PdDFf-7WQh9b0Ex7LcaE4oGE_eDPxC5aaoIM$

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*issuecomment-1615458507__;Iw!!Mih3wA!CMmQJf7zvOh2VWXqVXDKgf7J8jRfh5hh7pr_xNFnc9cJddWDgHxS9Q3PdDFf-7WQh9b0Ex7LcaE4oGE_eDPxyYvL2aw$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQR3KJWZOSYAHICXTZOC4ITXN6QFVANCNFSM6AAAAAAZR66M4Q__;!!Mih3wA!CMmQJf7zvOh2VWXqVXDKgf7J8jRfh5hh7pr_xNFnc9cJddWDgHxS9Q3PdDFf-7WQh9b0Ex7LcaE4oGE_eDPxYln5IlQ$ . You are receiving this because you were mentioned.Message ID: @.***>

maliberty commented 11 months ago

@ZhiangWang033 I see no images just "[image: image.png]". I don't see how your code could have worked with the bug I mentioned.

github-actions[bot] commented 11 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 11 months ago

@ZhiangWang033 I see the pins looks better now. However a new issue has arisen since the test case has changed somewhat recently. The macros are abutting in X but not in Y. However mpl2 only has a single halo width which doesn't support this. The older placer had separate X & Y halos. Is that easy for you to implement?

If you prefer can merge this and start a new PR. Let me know your preference.

ZhiangWang033 commented 11 months ago

Hi Matt,

I will be working on supporting separate X & Y halos today. Then you can merge it.

On Fri, Jul 14, 2023 at 11:22 PM Matt Liberty @.***> wrote:

@ZhiangWang033 https://urldefense.com/v3/__https://github.com/ZhiangWang033__;!!Mih3wA!CkJAYemVVHZcsuguLZSYmEOHcTlwsY-ut4Q6AgcJerfl1aPY16rpWbbabXgOvLXnYRIc4VeqNGeXelmfsOcCVkpko2g$ I see the pins looks better now. However a new issue has arisen since the test case has changed somewhat recently. The macros are abutting in X but not in Y. However mpl2 only has a single halo width which doesn't support this. The older placer had separate X & Y halos. Is that easy for you to implement?

If you prefer can merge this and start a new PR. Let me know your preference.

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3512*issuecomment-1636682361__;Iw!!Mih3wA!CkJAYemVVHZcsuguLZSYmEOHcTlwsY-ut4Q6AgcJerfl1aPY16rpWbbabXgOvLXnYRIc4VeqNGeXelmfsOcCegI9b5w$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQR3KJQUGDH64FAMGJNSRK3XQIZLBANCNFSM6AAAAAAZR66M4Q__;!!Mih3wA!CkJAYemVVHZcsuguLZSYmEOHcTlwsY-ut4Q6AgcJerfl1aPY16rpWbbabXgOvLXnYRIc4VeqNGeXelmfsOcCTJ7CISU$ . You are receiving this because you were mentioned.Message ID: @.***>

maliberty commented 10 months ago

There is still work to be done to make this fully solve the mock array issue. I'm going to merge this as a base for further progress.