SiteInst.routeSite() produces NullPointerExceptions in flat designs where nets contain slashes

[GTRI-Dallon]

Although the names of nets, cells, etc. should ideally not contain any slashes, I sometimes have to work with a design that does. Vivado also introduces slashes in net names and cells if you use "-flatten_design -full" when synthesizing. Anyway, in working with some flattened designs, I've encountered some NullPointerExceptions when trying to use design.routeSites() or siteInst.routeSite().

To reproduce the issue, you can just edit the names of some nets in the Lesson1 example to have slashes. Then, when routeSites() is called, the following occurs:

Exception in thread "main" java.lang.NullPointerException
    at com.xilinx.rapidwright.edif.EDIFNetlist.getHierNetFromName(EDIFNetlist.java:566)
    at com.xilinx.rapidwright.edif.EDIFNetlist.getNetFromHierName(EDIFNetlist.java:500)
    at com.xilinx.rapidwright.edif.EDIFNetlist.getPhysicalNetFromPin(EDIFNetlist.java:615)
    at com.xilinx.rapidwright.design.SiteInst.routeSite(Unknown Source)
    at com.xilinx.rapidwright.design.Design.routeSites(Unknown Source)

I think the problem in this case is probably that the parentHierInstName argument of getPhysicalNetFromPin is being passed in a non-empty name since the net has a slash in it. I can't verify this since I can't see how it's being called though.

I'm pretty sure I've also seen a NullPointer exception occur in EDIFNetlist.getNetAliases() after routeSite was called. I don't have a good example of this right now, but it happened on line 811, instName = p.getHierarchicalInstName().substring(0, lastIndex);, where lastIndex was -1. Again, I believe this occurred because some slashes were present in the names of some nets in a flat design.

[clavin-xlnx]

The legality of using the hierarchy separator as also part of the name is not ideal, but I believe this is legal in Verilog so its just something we have to live with.

There is some code that attempts to handle situations like this (see EDIFNetlist.java:574 and EDIFNetlist.java:477), but the solution is obviously not robust in all cases.

I tried to reproduce the error using the Lesson1 example, and I can only get it to happen when I put the / into a net name that coincidentally breaks the name into a previously existing cell name. For example, there already exists a cell called button0, I can only reproduce the same error above when I rename net0 from button0_IBUF to button0/_IBUF. If I put the slash anywhere else in the middle of the name, it fails on creating the net (which I believe is ok, I don't necessarily want people doing this on purpose).

I believe creating a net with the name button0/_IBUF using RapidWright APIs when a cell called button0 already exists, is actually creating a net called _IBUF inside button0 which also should be invalid (sorry, no exception thrown here) because button0 is a primitive.

Long story short, I think we just need a different test case to get at the heart of the issue. Is there a small DCP generated by Vivado that reproduces the issue directly that you don't mind sharing?

[GTRI-Dallon]

Sorry, I made a mistake in my original post. In order to get the error trace I shared, I actually just added a / to the name of a cell in the Lesson1 example (not a net). I tried this again by renaming the and2 cell to fakeHierarchy/and2.

Note that I'm not trying to create a design from scratches with names like these. If it would still help to have a DCP I can create one that reproduces the issue too.

[clavin-xlnx]

Sorry, it would probably be best with a DCP example. It is one thing to handle designs coming in with this issue, but I don't think we want to enable RapidWright creation APIs to support naming that allows / characters to be part of the names. This would be a fair amount of work to support a questionable feature.

[GTRI-Dallon]

I created a simpler example to test this problem out with and was unable to reproduce the same issue. I will reopen if I come across it again and can provide an example.