Investigating timing degradations on multiple designs

[eder-matheus]

Subject

[Stage]: Other. Please describe below.

Describe the bug

On PR https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/2489, multiple timing metrics are updated, and most of them show a huge degradation. This happened after OpenROAD PR https://github.com/The-OpenROAD-Project/OpenROAD/pull/5992 was merged, and it wasn't expected.

Expected Behavior

Some level of degradation could be expected, but not in so many designs and such large values.

Environment

Public and private CI

To Reproduce

Check latest CI run.

Relevant log output

No response

Screenshots

No response

Additional Context

No response

[maliberty]

@andyfox-rushc can you help look into these?

[andyfox-rushc]

please back out commit 5922 to make code stable and I will isolate why in non-hierarchical mode some cells apparently have a dbModule (ie parent not topInstance).

To debug I think it is a matter of printing out all the cells in non-hierarchical mode which do not have a parent as topInstance and then isolate where they are being created. No big deal, might as well back out 5922 as we know that will cause the crash Peter saw.

[maliberty]

It is not unexpected for some cells to have a dbModule in non-hierarchical mode. When we preserve hierarchy in yosys that is reflected in OR and used by mpl2. The data needs to be in odb but sta should not be looking at it unless hierarchy is on.

[maliberty]

Recall that the module instance tree predates all your recent work to support the full connectivity model.

[andyfox-rushc]

ok so the code in 5922 should be backed out:

if (!hasHierarchy()) { <-- this is obviously wrong if we know that there are cases when there is hierarchy without -hier. return topinstance; }

Also, I think you are assuming the problem is occuring on hierarchical designs. I suspect that somewhere lower down in the flow dbCells are being added with a weird parent (this is what I saw in Peter's case) -- looked like filler cells of some kind.

Also I recall we saw a delta in the odb in floorplan_3_4.odb (I think) which was after resizer on floorplan. So maybe your hypothesis is correct ! So the blanket test if (!hierarchy()) return topinstance for parent which is in PR5922 does not seem right to me..

[maliberty]

There is hierarchy but I don't expect sta to see it. The way it was written sta would write out incorrect SDC by including both the hierarchy from the dbModInst and the flat name from the dbInst (eg a/b/a/b instead of a/b).