OpenROAD icon indicating copy to clipboard operation
OpenROAD copied to clipboard

Odd pin access code in detailed router.

Open QuantamHD opened this issue 4 years ago • 4 comments

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.

QuantamHD avatar Dec 28 '21 04:12 QuantamHD

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)

QuantamHD avatar Dec 28 '21 06:12 QuantamHD

@osamahammad21 @maliberty Should we remove the break statement?

QuantamHD avatar Dec 29 '21 02:12 QuantamHD

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.

maliberty avatar Dec 29 '21 04:12 maliberty

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.

QuantamHD avatar Dec 29 '21 06:12 QuantamHD

@osamahammad21 Can you check this pin access code?

vijayank88 avatar Jun 14 '23 12:06 vijayank88

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.

osamahammad21 avatar Jun 15 '23 16:06 osamahammad21