rcx: createContextNets use direction #9114
Conversation
Signed-off-by: LucasYuki <[email protected]>
|
Still running secure-CI |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses two issues. First, it correctly fixes a potential infinite recursion in dbCreateNetUtil::createNetSingleWire, which is a critical bug fix. Second, it refactors extMeasure::createContextNets to simplify how context nets are created. However, the refactoring seems to have introduced a logic error in determining the orientation of the context nets. My review includes a suggestion to correct this logic.
| net = _create_net_util.createNetSingleWire(netName, | ||
| ll[0], | ||
| ll[1], | ||
| ur[0], | ||
| ur[1], | ||
| met, | ||
| mlayer->getDirection(), | ||
| false); | ||
|
|
There was a problem hiding this comment.
The logic for determining the direction of the context nets appears to be incorrect. The context nets are created perpendicular to the main wire. _dir is the direction of the main wire, and the loop iterates along !_dir to create parallel context nets. This means if the main wire is HORIZONTAL, the context nets are VERTICAL, and vice-versa.
The current code passes mlayer->getDirection(), which is the direction of the main layer, not the context nets. This will cause createNetSingleWire to misinterpret the width of the wire from the bounding box coordinates. The direction should be perpendicular to mlayer->getDirection().
odb::dbTechLayerDir context_dir =
(mlayer->getDirection() == odb::dbTechLayerDir::HORIZONTAL)
? odb::dbTechLayerDir::VERTICAL
: odb::dbTechLayerDir::HORIZONTAL;
net = _create_net_util.createNetSingleWire(netName,
ll[0],
ll[1],
ur[0],
ur[1],
met,
context_dir,
false);There was a problem hiding this comment.
I don't think it should always be perpendicular to mlayer->getDirection().
Perhaps it should be the direction of the met layer (layer->getDirection()), since in the createNetSingleWire method, the layer used to determine the minimum width is the same as layer, not mlayer.
There was a problem hiding this comment.
This code is never exercised, it is for some variation support we've left unused. I think mlayer is the layer for the net of interest and layer is for the context nets (based on met as an argument). It appears you think otherwise so please explain further.
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
| if (dir == odb::dbTechLayerDir::NONE) { | ||
| return createNetSingleWire( | ||
| netName, x1, y1, x2, y2, routingLayer, dir, skipBterms); | ||
| dir = odb::dbTechLayerDir::HORIZONTAL; |
There was a problem hiding this comment.
Let's just make this an error since I don't expect us to extract on a non-routing layer and clearly it wouldn't work if you do currently.
2 participants
Closes #7908
Changes: