Odd pin access code in detailed router.
https://github.com/The-OpenROAD-Project/OpenROAD/blob/719c6dc617d4b8d6896500bb41cdf057864f9e09/src/drt/src/dr/FlexDR_maze.cpp#L1940-L1968
The pin access code above is phrased a bit weird. It seems to select only the first pin access point in the list rather than iterating through the whole thing. Is this intentional? The rest of the code seems to accommodate selecting multiple pin access points.
I uncommented the line to see if things would break, but it looks like it improves the slack on designs like AES. To
---------------------------------------------------------------------------
7.663 data required time
-9.087 data arrival time
---------------------------------------------------------------------------
-1.424 slack (VIOLATED)
From
---------------------------------------------------------------------------
7.916 data required time
-9.549 data arrival time
---------------------------------------------------------------------------
-1.633 slack (VIOLATED)
@osamahammad21 @maliberty Should we remove the break statement?
The first AP is the preferred AP so I think this is intentional. We would need to run a full suite of designs to consider changing this.
If that's the case why is dividing by toApCnt in the lines following this loop? It seems at some point there was some kind of averaging algorithm that is now is basically dead.
That code snippet also only generates something called the centerPt. The AP selection is actually below this.
@osamahammad21 Can you check this pin access code?
I don't think removing the break would be correct. Moving it to the outer for loop could be correct. Here is why: uConnPins is a vector of all the pins that are to be connected for the current net. We pass this vector to routeNet_setSrc(), where we are calculating the centerPt and the srcPin. It's worth mentioning that the distance to the centerPt is used in the maze search as a second preference for sorting the waveFrontGrid points after the cost (it is the tie breaker of 2 waveFronts with the same cost). Now that this is explained, I think the code is meant to do the following:
- Get the centerPt of the first pin's access points.
- Set the src pin as the furthest away pin from that first pin we chose.
This makes the search start from that src pin and keep on advancing towards the centerPt which is "final destination" for that net. I am considering the centerPt as the theoritical final destination because in mazeSearch we start at the src pin and keep on going to the nearest pin to that src pin. since the centerPt is the furthest away from the initial src pin, it should theoretically be the final pin chosen as destination in the maze search of the net. If we remove the break entirely, then we are setting the centerPt as the center of all access points which directs the maze search to advance more to that center. So if we have a net of 2 pins on the opposing boundaries, the centerPt would be exactly at the center of the worker (either vertically or horizontally). This doesn't seem correct. Also it's worth mentioning that I don't think we rely heavily on the centerPt as the cost takes into consideration the distance to the current destination node. That said, I didn't write the code so I am not sure if that was the initial intention. But it seems to me that this is the most logical explanation.