Fixes issue when connecting passthru module ports #477
Conversation
…isting placement Signed-off-by: Chris Lavin <clavin@xilinx.com>
Unit Test Results 33 files 33 suites 11m 19s ⏱️ Results for commit a9b2a40. ♻️ This comment has been updated with latest results. |
Signed-off-by: Chris Lavin <clavin@xilinx.com>
| * @return True if placement was successful, false otherwise. | ||
| */ | ||
| public boolean place(Site newAnchorSite, boolean skipIncompatible){ | ||
| public boolean place(Site newAnchorSite, boolean skipIncompatible, boolean allowOverlap){ |
There was a problem hiding this comment.
Would it be more descriptive to call the argument allowClobbering or allowOverwrite or simply force since it doesn't just overlap such that the module inst can still use the unused BELs -- the module inst's SiteInst appears to replace the existing SiteInst?
It's also not clear if that previous SiteInst will get unplaced correctly?
There was a problem hiding this comment.
Things are technically not lost and RapidWright still has tracking of each SiteInst location. This is how the block placer operates and can recover from placing a module instance on top of another, and then subsequently moving it elsewhere.
There was a problem hiding this comment.
Hmm. Let me wrap my head around that. Where is that overlapping placement tracked? If I asked the Design what SiteInst is located at a particular site, what answer would I get back just the last-placed SiteInst? If so, how would one get the ones "underneath" that one?
There was a problem hiding this comment.
There are two stored locations. One global in Design, this would be the one "on top" and the other locally in the SiteInst itself. Clearly this is not a perfect system. When the block placer legalizes, that is when things are resolved. So, there is some intermediate situations where as you noted, if checking for placement on a Site that previously had two, but now has one SiteInst matching placement, it wouldn't be found globally, only locally. This issue doesn't manifest itself as when writing out a DCP, we iterate over SiteInst objects rather than the Design map.
There was a problem hiding this comment.
Would you happen to know what holds a reference to the SiteInst that gets overwritten? I think it would be a good idea to document this behaviour. Historians from the future will thank you.
There was a problem hiding this comment.
There are two maps in the Design class that maintain the two different locations for placement. The user would see the discrepancy through the APIs Design.getSiteInstFromSite() vs Design.getSiteInsts(). Since each SiteInst keeps track of its own placement, getting a reference to the SiteInst and calling SiteInst.getSite() would be consistent in both the overlap/non-overlap case. However, the Design.getSiteInstFromSite() would not be consistent as this is a map from last placement on a particular site to its SiteInst and removing an overlap doesn't restore that map to the existing/prior placement.
Co-authored-by: eddieh-xlnx <90657806+eddieh-xlnx@users.noreply.github.com>
Also checks existing
SiteInstplacement to make sure not to overlap module instance placements.