Duplicate EDIFPortInsts - Expected behaviour?

[jakobwenzel]

Hi,

after hours of debugging, I found out that 131331d75ae66678901e5eb614fd5145428eb0eb subtly changed behaviour when there are duplicate EDIFPortInsts.

Let's use this minimal example:

@Test
    public void test() {

        String designName = "design";
        final EDIFNetlist netlist = EDIFTools.createNewNetlist(designName);

        EDIFCell ec = new EDIFCell(netlist.getWorkLibrary(), "foo");
        EDIFPort port = ec.createPort("in", EDIFDirection.INPUT, 1);

        EDIFCell top = netlist.getTopCell();
        EDIFCellInst cell = ec.createCellInst("a", top);

        EDIFNet netA = top.createNet("netA");
        EDIFNet netB = top.createNet("netB");

        netA.createPortInst(port, cell);
        netA.removePortInst(netA.getPortInsts().iterator().next());

        netB.createPortInst(port, cell);
        EDIFPortInst epi = cell.getPortInsts().iterator().next();

        Assertions.assertEquals(netB, epi.getNet());
    }

We create a cell inst and connect it to one net. Then we change our mind, disconnect and connect to a different net. When querying the cell inst for which net its port inst connects to, we should get netB, right? Well, before 131331d75ae66678901e5eb614fd5145428eb0eb we did. Now, we get null.

The reason for this change is subtle: Previously, EDIFCellInst.portInsts was a Map. When adding a duplicate entry in EDIFCellInst.addPortInst, the previous one was discarded and the new one was saved in the map. In that change EDIFPortInstList was introduced, which is now used in EDIFCellInst. In case of duplicates, EDIFPortInstList keeps the old value and discards the new one.

This change was unexpected for me and very hard to find. I believe that mirroring the previous behaviour more closely is more intuitive.

[jakobwenzel]

Mirroring the previous behaviour would probably be to overwrite previously stored values with new duplicates here: https://github.com/Xilinx/RapidWright/blob/a959449d1073747359e955a36e625656af956542/src/com/xilinx/rapidwright/edif/EDIFPortInstList.java#L42-L45

[eddieh-xlnx]

Fixed by #571.