mbff: Move IsValidTray back to mbff.cpp.#10682
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the isValidTray function by moving it from dbNetwork to the MBFF class as IsValidTray, updating its usages and associated tests. The review feedback suggests adding a null check for the tray pointer in MBFF::IsValidTray to prevent potential null pointer dereferences.
| bool MBFF::IsValidTray(dbInst* tray) | ||
| { | ||
| const sta::Cell* cell = network_->dbToSta(tray->getMaster()); |
There was a problem hiding this comment.
Defensive programming: tray is a pointer passed to IsValidTray and could potentially be nullptr if the instance creation fails or if an invalid pointer is passed. We should add a null check before dereferencing it to prevent potential segmentation faults.
bool MBFF::IsValidTray(dbInst* tray)
{
if (tray == nullptr) {
return false;
}
const sta::Cell* cell = network_->dbToSta(tray->getMaster());|
@precisionmoon I feel that using LibertyCell::setDontUse from Resizer::setDontUse, and removing Resizer::dont_use_ is a better solution. It avoids having the "same" information in two places which inevitably leads to problems like this. Agree? |
|
We'd prefer having just one dont-use as well. Does this leave an easy way to do reset_dont_use? |
|
I suppose it depends on the semantics of reset_dont_use. Does it reset only user defined such or does it include those that come from Liberty itself? If the former rsz would have to keep track of which cells were set but the actual value could stay in sta. |
|
It does the former. It shouldn't be too hard to make it work, though. |
Sorry for the delayed response. We need to keep Resizer::setDontUse as separate API because 1) STA API exists to specify library dont-use (static) 2) Resizer API is needed to modify dont-use on the fly (dynamic). |
It was not respecting the resizer's dont_use, instead deferring to the LibertyCell's dont_use. Signed-off-by: Sean Luchen <seanluchen@google.com>
Signed-off-by: Sean Luchen <seanluchen@google.com>
|
Rebased to HEAD, since there were other MBFF changes. |
|
@precisionmoon @dsengupta0628 PTAL when you get a chance. It seems we still need this fix to resolve MBFF issues on our test case, otherwise please confirm that there's a replacement effort in place. |
It was not respecting the resizer's dont_use, instead deferring to the LibertyCell's dont_use.
This issue was introduced in 89f47ff, and this PR reverts only the relevant portion of that commit.
This PR will only build once #10665 is submitted -- it depends on testCell being public.