DesignTools.copyImplementation sometimes results in unplaced non-VCC/GND cells

[korbin]

I'm working on a highly occupied (99%+) UltraScale+ design and am using DesignTools.copyImplementation to piece together separately-implemented (but identical) pblocks from multiple SLRs.

RapidWright/src/com/xilinx/rapidwright/design/DesignTools.java

Lines 2840 to 2841 in 4e804cf

	SiteInst dstSiteInst = dest.getSiteInstFromSite(cell.getSite());
	SiteInst srcSiteInst = cell.getSiteInst();

For some reason, dest.getSiteInstFromSite() here is returning:

SiteInst(name="STATIC_SOURCE_SLICE_X117Y252", type="SLICEL", site="SLICE_X117Y252")

for a source SiteInst of:

SiteInst(name="SLICE_X117Y252", type="SLICEL", site="SLICE_X117Y252")

This is causing a small number of sites to be unplaced in the merged Design. Most Cells within the same bundle get copied exactly as expected, copying the routing works, but a handful of sites break here.

I don't believe the source to dest.getSiteInstFromSite is published or I would debug further. The part in question is axcvu35p-fsvh2104

[korbin]

Update: there's not a lot of documentation on what STATIC_SOURCE slices specifically are, but, copying this logic:

RapidWright/src/com/xilinx/rapidwright/design/tools/RelocationTools.java

Lines 225 to 227 in e8126ba

	if (destSiteInst.getName().startsWith("STATIC_SOURCE")) {
	destSiteInst.unPlace();
	} else {

into DesignTools.copyImplementation (near the existing null check) has resulted in the issue going away.

[clavin-xlnx]

@korbin - Impressive that you are able to achieve 99% utilization! Thanks for bringing up this issue. The STATIC_SOURCE SiteInst objects are present because they are used as a placeholder SiteInst when there are no cells placed in the Site, but, there is at least one or more LUTs in the Site that are being used as a GND source. It looks like what is happening is that when the circuit is being copied to the destination, it finds one of these STATIC_SOURCE site instances in the place where it is supposed to go and given that these do potentially occupy the same Site in a sense, the copying fails. You've found that by unplacing them ahead of time, that the solution can succeed. This is probably not far off from what should happen.

These STATIC_SOURCE site instances are technically not part of the design and only an artifact of the implementation. They could be moved (source GND from a different LUT somewhere else). In highly congested scenarios (like your design), there is a small chance that finding an accessible GND source might become harder, but you could leave routing the GND net to Vivado.

I'm glad you've found a work-around for the issue. Please reach out with other concerns issues. I think we'll need to think a bit more on the best way to handle these scenarios.

[korbin]

Thanks for the additional info.

This was initially confusing because these STATIC_SOURCE sites were located within floorplanned EXCLUDE_PLACEMENT pblocks.

Upon further digging, I figured out what was happening. In this specific case, the culprit was my "greybox" creator function (could be useful for #356):

def makeGreybox(dcp: Design, cellName: String): Unit = {
    val netlist = dcp.getNetlist()
    val top = netlist.getTopCell()
    val futureGreybox = netlist.getHierCellInstFromName(cellName).getInst()

    val oldCell = futureGreybox.getCellType()
    val greybox = new EDIFCell(futureGreybox.getCellType().getLibrary(), oldCell.getName() + "_gb")

    for (port <- oldCell.getPorts().asScala) {
      val netsPre = port.getInternalNets().asScala
      greybox.addPort(port)

      val newNets = for ((net, i) <- netsPre.zipWithIndex) yield {
        val newNet = greybox.createNet(net.getName())

        val newInst = if (port.isBus()) {
          newNet.createPortInst(port, i)
        } else {
          newNet.createPortInst(port)
        }

        val lutInst = greybox.createChildCellInst(newNet.getName(), Design.getUnisimCell(Unisim.LUT1))
        lutInst.addProperty("INIT", "2'h2")
        lutInst.addProperty("DONT_TOUCH", "TRUE")

        if (port.isInput()) {
          newNet.createPortInst("I0", lutInst)
        }

        if (port.isOutput()) {
          val gnd = EDIFTools.getStaticNet(NetType.GND, greybox, netlist);
          gnd.createPortInst("I0", lutInst);
          newNet.createPortInst("O", lutInst)
        }
      }
    }

    futureGreybox.setCellType(greybox)
    netlist.removeUnusedCellsFromAllWorkLibraries()
    greybox.rename(oldCell.getName())
  }

In createGreybox, I recreate update_design -buffer_ports by creating LUT1 placeholder buffers to GND for each of a hierarchical cell's ports. Upon placing and routing this design, Vivado was adding GND source LUTs to drive these greybox placeholders LUTs' I0 pins within the EXCLUDE_PLACEMENT pblock.

As previously discovered, these STATIC_SOURCE SiteInsts unfortunately aren't removed when Design.copyImplementation copies the cell, despite no real conflict existing in the final design (the logic these static LUTs were driving was completely removed and replaced.)

In some cases dstSiteInst.unplaceSite() worked - in others it did not - I'm unsure which resources were being left behind.

Completely removing the destination SiteInst does seem to make everything work correctly. I'm sure more logic is needed to make this generally safe in more cases, but I don't know what the intended behavior of copyImplementation is supposed to be for sites with conflicts:

  if (dstSiteInst != null && dstSiteInst.getName().contains("STATIC_SOURCE")) {
    dest.removeSiteInst(dstSiteInst);
    dstSiteInst = null;
  }

One question regarding the overall model: does Vivado not place VCC/GND source LUTs in otherwise partially-used sites? Is STATIC_SOURCE granular enough on SiteInst - or is this property possibly more precise on a BEL?

[eddieh-xlnx]

Allow me to fill in for Chris while he is out for the next week and a bit; please forgive me if I'm a little out of the loop.

It sounds like you've got some code which adds LUT1 placeholder buffers (with undriven inputs?) and then Vivado requires that those LUT1s' inputs must be driven by something which it decides has to be GND.

Later, you call DesignTools.copyImplementation() (with copyStaticNets argument to be true?) and find that it is copying those GND nets/sources too.

As previously discovered, these STATIC_SOURCE SiteInsts unfortunately aren't removed when Design.copyImplementation copies the cell

My take on your initial question was that for a particular site in your source design, the equivalent site in the destination design was found to be a static source, rather than be completely empty as you expect. This is legal, in the sense that VCC/GND nets don't really follow pblock rules since they can be sourced from any free LUT. In this case, I agree that RapidWright could do better by attempting to merge the two two SiteInst-s, and where that's not possible then give the user's SiteInst (whether it is on the source or the destination side) priority over any STATIC_SOURCE and ideally rip up its associated subtree so that someone else can fix it.

I think your workaround achieves the latter -- throw away the STATIC_SOURCE and assume someone else (e.g. Vivado) will notice any conflicts and fix it. On the surface, that sounds perfectly acceptable. STATIC_SOURCEs should only contain static sources -- and never any user logic -- and Vivado should pick up when it is no longer able to source the GND from the same place it did before and fix itself.

Note that DesignTools.copyImplementation()'s job is to copy everything under the sun (including static sources and static routing) such that the result remains fully placed-and-routed under ideal conditions. What you've run into is a non-ideal condition where we may have to admit that's not possible.

One question regarding the overall model: does Vivado not place VCC/GND source LUTs in otherwise partially-used sites? Is STATIC_SOURCE granular enough on SiteInst - or is this property possibly more precise on a BEL?

Yes. Vivado can source a GND from any unused LUT (VCC sources can typically be found as a PIP on most site inputs) -- regardless of whether that SLICE is occupied by user logic. STATIC_SOURCE is simply a RapidWright placeholder to say "something in this site is only being used as a static source" -- it may be possible to figure out exactly which BEL is being used by examining the site wires, but those sitewire assignments are not mandatory.

[korbin]

Vivado requires that those LUT1s' inputs must be driven by something which it decides has to be GND.

I decided they are GND (see code above), but they could just as easily be VCC and still cause this issue.

• Destination DCP has greybox buffer LUT1s in it for pblocked modules that were removed.
  • Greybox LUT1s are given a STATIC_SOURCE GND driver by Vivado upon implementation
• Source DCP has actual logic in it
• copyImplementation(source, destination, "my_cell") replaces the LUT1 buffers with the actual logic - but the STATIC_SOURCE SiteInsts that are only valid for the LUT1 buffer cells (now removed) are orphaned.

throw away the STATIC_SOURCE and assume someone else (e.g. Vivado) will notice any conflicts and fix it

The issue here is that the greybox logic that the STATIC_SOURCE LUT is driving is completely removed and replaced by this very same call to copyImplementation - the STATIC_SOURCE LUT/route isn't going to be readded by Vivado, removing it in this case is the proper behavior.

IMO: the copyImplementation logic should be changed to walk through the destination DCP and prune all soon-to-be-orphaned STATIC_SOURCE LUTs and associated routes that drive loads contained entirely within the destination Cell - prior to copying any logic.

[eddieh-xlnx]

I decided they are GND (see code above), but they could just as easily be VCC and still cause this issue.

VCC driving LUT inputs do not need to be sourced from another LUT. They can be sourced from a VCC_WIRE node in the INT tile and should not cause any STATIC_SOURCE -related problems.

Besides that, I agree that copyImplementation() should be more robust in its handling of STATIC_SOURCEs. It'd be most efficient if you can provide a simplified testcase I can see exactly what you mean, or better yet, submit a PR with the fix (and a test).

[korbin]

Sorry this is confusing to describe, I am confident this is an issue though (though I don't have all of the lingo dialed-in yet):

VCC driving LUT inputs do not need to be sourced from another LUT.

It's possible these VCC_WIRE PIPs wouldn't get pruned either - this will be interesting to test.

I will have to try and put a stripped-down/contrived test case together that doesn't involve a million LUT DCP, maybe it can be done with RapidWright (I am just not familiar with how these test case DCPs are created, etc.):

---

• Destination Design: Create a hierarchical cell foo with an output pin/port bar containing a LUT with its I0 pin driven by GND and O pin connected to bar, Psuedocode:

module foo (output bar);
    LUT1 #(
        .INIT(2'b01)
    ) testlut (
        .I0(1'b0),
        .O(bar)
    );
endmodule

When you place and route this with Vivado, a STATIC_SOURCE LUT and route will be created connecting GND to testlut/I0.

• Source Design: Create a hierarchical cell foo with an output pin/port bar containing an FDRE with its Q connected to bar, Pseudocode:

module foo (input clk, output bar);
    FDRE testfdre (
        .Q(bar),
        .C(clk), 
        .D(),         // Shouldnt matter
    );
endmodule

• When you DesignTools.copyImplementation(src, dest, 'foo'), the STATIC_SOURCE SiteInst and associated route that were previously driving foo/testlut/I0 are not removed by RapidWright - despite no longer driving any other loads
  • This then causes another issue to potentially manifest: if the new foo source design happens to contain a different LUT placed in the same site as this no-longer-loaded STATIC_SOURCE SiteInst, placement and routing will not be copied.

A potential fix would be:

• In Design.copyImplementation, for each hierarchical destination cell about to be replaced:
  • Scan for and remove all STATIC_SOURCE SiteInsts and their accompanying routes that are exclusively connected to loads within the destination cell

[clavin-xlnx]

@korbin - I've tried to reproduce the issue by generating some designs and constructing a scenario where it should generate a STATIC_SOURCE site.

I've created two DCPs of cell foo one with the LUT1 driven by an off-site GND STATIC_SOURCE site (in foo_gnd.dcp) and another that has a flip flop inside (foo.dcp) -- see zip file attached below:

foo_gnd.zip

foo_gnd.dcp:

foo.dcp:

I can then load the two designs that each instantiate the cell foo and then use Design.copyImplementation() to copy the flop-based version over the LUT1-based version, but this does not seem to replicate the issue:

        Design fooGnd = Design.readCheckpoint("foo_gnd.dcp");
        Design foo = Design.readCheckpoint("foo.dcp");

        System.out.println(fooGnd.getGndNet().getSource().getSiteInstName());

        DesignTools.copyImplementation(foo, fooGnd, "");

        System.out.println(fooGnd.getGndNet().getSource());

        fooGnd.writeCheckpoint("foo_inst.dcp");

The output is:

...
STATIC_SOURCE_SLICE_X86Y149
null
...

After DesignTools.copyImplementation(), the STATIC_SOURCE site instance is gone. Are you able to modify this example to trigger the issue you are seeing?