[DesignTools] Conform to Vivado *RST* pin inversion site routing configuration #1053
Conversation
Signed-off-by: Chris Lavin <[email protected]>
Signed-off-by: Chris Lavin <[email protected]>
eddieh-xlnx
changed the title
Signed-off-by: Chris Lavin <[email protected]>
Signed-off-by: Chris Lavin <[email protected]>
clavin-xlnx
changed the title
| if (gndInvertibleToVcc != null) { | ||
| // For the RST inversion to be interpreted properly by Vivado, there must be no | ||
| // site routing on the path around the inverter BEL | ||
| BELPin belPin = sitePin.getBELPin(); | ||
| si.unrouteIntraSiteNet(belPin, belPin); | ||
| } |
There was a problem hiding this comment.
Would this be safer inside the (sitePin == null) condition so that we're only modifying the site routing if we were the one to insert it?
There was a problem hiding this comment.
The intent of this method performs two tasks:
- Ensure that the site pin exists (create it if it doesn't)
- If architecturally relevant, modify the site routing (completely remove it) to ensure that the inversion is communicated.
If we guard that code, it seems like #2 would not be ensured.
There was a problem hiding this comment.
Since the parent method is DesignTools.createCeSrRstPinsToVCC() I would argue that the main purpose is 1, and only when 1 is undertaken (which will paint the associated sitewire), to ensure 2. Right now, if the pin already exists, the sitewire is always unpainted which might be necessary if we're assuming that the design was faulty to begin with.
There was a problem hiding this comment.
My concern is that RapidWright creates all site pins with the site wire attached painted (for better or worse). If we encounter a site pin that was created in another method, the site wire would never be reset (the requirement for inversion). I suppose we could change the site pin creation behavior, but I think that would touch and break a lot of things.
There was a problem hiding this comment.
That's a very valid concern, in which case I would prefer that we fix them at/close to where those site pins were created (or come up with a more holistic solution). Note that DesignTools.createCeSrRstPinsToVCC() is only called by DesignTools.createPossiblePinsToStaticNets() which is always called for RWRoute.preprocess() but not always called by PartialRouter.preprocess() such that we shouldn't rely on createCeSrRstPinsToVCC() to fix it up for all scenarios.
There was a problem hiding this comment.
Another thought is -- do we need to set that sitewire at all for VCC/GND nets? If so, perhaps not ever setting it for these two special nets could be a better way?
There was a problem hiding this comment.
I think the cleanest solution is to simply stop setting the sitewire on site pin addition or creation. Unfortunately, this will probably break a lot of downstream use cases.
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void maybeCreateVccPinAndPossibleInversion(SiteInst si, String sitePinName, Net vcc, Net gndInvertibleToVcc) { |
There was a problem hiding this comment.
Is this still the most descriptive method name?
There was a problem hiding this comment.
My slight irk was that this method doesn't do any inversion by itself anymore, since it doesn't set any site PIPs nor do any site routing; rather, it erases some site routing to allow inversion to occur. Let's leave it then.
as part of the
DesignTools.createCeSrRstPinsToVCC()method which gets called byRWRoute.preprocess()