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.35k stars 481 forks source link

par & mpl2: get inst area from .lef instead of .lib #5036

Open AcKoucher opened 2 weeks ago

AcKoucher commented 2 weeks ago

When debugging using sky130hd/uW, SoftSA overlap penalty was resulting in -nan. The reason was that the APIs from Cluster were returning 0.0 area for the macro area of clusters with macros. The problem doesn't look like it's mpl2 itself, but rather the fact that LibertyCell::area() is returning 0.0 for the macros in this design during mpl2 flow.

github-actions[bot] commented 2 weeks ago

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

AcKoucher commented 2 weeks ago

running secure-ci

maliberty commented 2 weeks ago

merge pending ci results

AcKoucher commented 2 weeks ago

CI showed a large increasing in drt WL for sky130hd/uW: [ERROR] detailedroute__route__wirelength fail test: 9237052.0 <= 8799572.0

I'll fix the PAR .lib area calls and test it again locally as it might have some meaningful impact on mpl2 clustering.

Edit: @maliberty I didn't affect uW, but including the PAR fix will require a new secure-ci run, right?

github-actions[bot] commented 2 weeks ago

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

maliberty commented 2 weeks ago

They are not fake rams, they are real dfframs that were used in a tapeout. I think it is generally better to rely on the LEF in any case as Liberty doesn't even require the area field to be present.

maliberty commented 2 weeks ago

Yes the par change would need another secure CI

maliberty commented 2 weeks ago

@AcKoucher how does the uW macro placement change with this PR?

AcKoucher commented 2 weeks ago

Left base, right this PR.

I just realized something: even though for this design we turned off the boundary push weight in SoftSA, `HierRTLMP::alignHardMacroGlobal` (which will be substituted by the new post process in #4991) pushes the macros to the boundaries anyway. Not using this function would make the macros be a bit more centralized which for this case could be good. Thinking of a general improvement, perhaps we could check whether or not the macro occupies all the layers and if yes we don't push it to the boundary during neither SA (so then the user wouldn't need to set the boundary weight to 0 manually) nor the post process.