[EDIF] Fixes rare bus renaming collision (#1065)

* [EDIF] Fixes rare bus renaming collision Signed-off-by: Chris Lavin <[email protected]> * Work-around for both issues, adds test for rare case Signed-off-by: Chris Lavin <[email protected]> * Minimizes bus suffix usage and resolves incorrect connectivity in MultGenerator Signed-off-by: Chris Lavin <[email protected]> * Fix comment Signed-off-by: Chris Lavin <[email protected]> * Update test/src/com/xilinx/rapidwright/edif/TestEDIFNetlist.java Co-authored-by: eddieh-xlnx <[email protected]> Signed-off-by: Chris Lavin <[email protected]> * Fix typo Signed-off-by: Chris Lavin <[email protected]> --------- Signed-off-by: Chris Lavin <[email protected]> Co-authored-by: eddieh-xlnx <[email protected]>
Xilinx · Sep 20, 2024 · a200396 · a200396
1 parent 0b9683a
commit a200396
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 3 deletions.
diff --git a/src/com/xilinx/rapidwright/edif/EDIFPort.java b/src/com/xilinx/rapidwright/edif/EDIFPort.java
@@ -291,9 +291,23 @@ public void exportEDIF(OutputStream os, EDIFWriteLegalNameCache<?> cache, boolea
      * @throws IOException
      */
     public void exportEDIFBusName(OutputStream os, EDIFWriteLegalNameCache<?> cache) throws IOException {
-        exportSomeEDIFName(os, getName(), cache.getEDIFRename(busName));
+        exportSomeEDIFName(os, getName(), getBusEDIFRename(cache));
     }
 
+    /**
+     * Handles bus name collisions with single bit ports (same root name) to avoid
+     * EDIF export name legalization collisions.
+     * 
+     * @param cache The current EDIF name legalization cache
+     * @return The legalize EDIF bus name for this port
+     */
+    protected byte[] getBusEDIFRename(EDIFWriteLegalNameCache<?> cache) {
+        String busName = getBusName(false);
+        boolean collision = parentCell.getPortMap().containsKey(busName);
+        byte[] rename = collision ? cache.getBusCollisionEDIFRename(busName) : cache.getEDIFRename(busName);
+        return rename == null ? busName.getBytes(StandardCharsets.UTF_8) : rename;
+    }
+
     /**
      * @return the parentCell
      */

diff --git a/src/com/xilinx/rapidwright/edif/EDIFPortInst.java b/src/com/xilinx/rapidwright/edif/EDIFPortInst.java
@@ -314,7 +314,7 @@ public void writeEDIFExport(OutputStream os, byte[] indent, EDIFWriteLegalNameCa
         }
         else {
             os.write(EXPORT_CONST_MEMBER);
-            os.write(cache.getLegalEDIFName(getPort().getBusName(true)));
+            os.write(getPort().getBusEDIFRename(cache));
             os.write(' ');
             os.write(Integer.toString(index).getBytes(StandardCharsets.UTF_8));
             os.write(')');

diff --git a/src/com/xilinx/rapidwright/edif/EDIFWriteLegalNameCache.java b/src/com/xilinx/rapidwright/edif/EDIFWriteLegalNameCache.java
@@ -52,12 +52,15 @@ public abstract class EDIFWriteLegalNameCache<T> {
      */
     private final Map<String, byte[]>[] renames;
 
+    private final Map<String, byte[]> busCollisionRenames;
+
     private EDIFWriteLegalNameCache(Map<String, T> usedRenames, Supplier<Map<String, byte[]>> renameSupplier) {
         this.usedRenames = usedRenames;
         this.renames = new Map[256];
         for (int i = 0; i < renames.length; i++) {
             renames[i] = renameSupplier.get();
         }
+        this.busCollisionRenames = new HashMap<>();
     }
 
     protected abstract int getAndIncrement(String rename);
@@ -86,6 +89,11 @@ public byte[] getEDIFRename(String name) {
         return rename;
     }
 
+    public byte[] getBusCollisionEDIFRename(String name) {
+        return busCollisionRenames.computeIfAbsent(name,
+                n -> (EDIFTools.makeNameEDIFCompatible(n) + "_BUS_").getBytes(StandardCharsets.UTF_8));
+    }
+
     public static EDIFWriteLegalNameCache<?> singleThreaded() {
         return new EDIFWriteLegalNameCache<Integer>(new HashMap<>(), HashMap::new) {
 

diff --git a/src/com/xilinx/rapidwright/examples/MultGenerator.java b/src/com/xilinx/rapidwright/examples/MultGenerator.java
@@ -263,7 +263,12 @@ public static PBlock createMult(Design d, Site origin, int width, String designN
             for (int i=0; i < port.getWidth(); i++) {
                 EDIFNet net = top.createNet(port.getBusName() + (port.getWidth() > 1 ? "[" + i + "]" : ""));
                 int ii = port.getWidth() - 1 - i;
-                net.createPortInst(port.getBusName(), ii, inst);
+                if (port.isBus()) {
+                    net.createPortInst(port.getBusName(), ii, inst);
+                } else {
+                    net.createPortInst(port, inst);
+                }
+
                 Net physNet = d.createNet(inst + "/" + net.getName());
 
                 // Correct differences in physical pin names

diff --git a/test/src/com/xilinx/rapidwright/edif/TestEDIFNetlist.java b/test/src/com/xilinx/rapidwright/edif/TestEDIFNetlist.java
@@ -450,6 +450,44 @@ public void testBussedPortNamingCollision(@TempDir Path path) {
         Assertions.assertEquals(port1.getWidth(), testPort1.getWidth());
     }
 
+    @Test
+    public void testBussedPortReNamingCollision(@TempDir Path path) {
+        final EDIFNetlist origNetlist = EDIFTools.createNewNetlist("test");
+
+        EDIFCell top = origNetlist.getTopCell();
+
+        EDIFCellInst ff = top.createChildCellInst("ff", Design.getPrimitivesLibrary().getCell("FDRE"));
+        origNetlist.getHDIPrimitivesLibrary().addCell(ff.getCellType());
+
+        String portName = "unfortunate_name";
+
+        // Create two bussed ports with a common root name, one having a trailing '_' underscore
+        EDIFPort port0 = top.createPort(portName + "[2:0]", EDIFDirection.INOUT, 3);
+        EDIFPort port1 = top.createPort(portName + "_[1:0]", EDIFDirection.INOUT, 2);
+        // And for good measure, throw in a single-bit port too with the same root
+        EDIFPort port2 = top.createPort(portName, EDIFDirection.INOUT, 1);
+
+        EDIFNet net0 = top.createNet("net0");
+        net0.createPortInst(port0, 0);
+        net0.createPortInst("D", ff);
+
+        EDIFNet net1 = top.createNet("net1");
+        net1.createPortInst(port1, 1);
+        net1.createPortInst("R", ff);
+
+        EDIFNet net2 = top.createNet("net2");
+        net2.createPortInst(port2);
+        net2.createPortInst("Q", ff);
+
+        Path tempFile = path.resolve("test.edf");
+        origNetlist.exportEDIF(tempFile);
+
+        // Check using EDIFNetlistComparator
+        EDIFNetlist testNetlist = EDIFTools.readEdifFile(tempFile);
+        EDIFNetlistComparator comparer = new EDIFNetlistComparator();
+        Assertions.assertEquals(0, comparer.compareNetlists(origNetlist, testNetlist));
+    }
+
     @Test
     public void testGetIOStandard() {
         final EDIFNetlist netlist = EDIFTools.createNewNetlist("test");