Incorrect value from getPinMappingsL2P

[litghost]

When Cell.getPinMappingsL2P is invoked on a RAMB36E1 with:

Parameters:  RAM_MODE=SDP WRITE_WIDTH_A=0 WRITE_WIDTH_B=72

getPinMappingsL2P returns an incorrect pin mapping, output:

RAMB36_X0Y29 (RAMB36E1) RAMB36E1 getPinMappingsL2P:
    WEBWE[0] <-> WEBWEU7
    WEBWE[1] <-> WEBWEU1
    WEBWE[2] <-> WEBWEU2
    WEBWE[3] <-> WEBWEU3
    WEBWE[4] <-> WEBWEU4
    WEBWE[5] <-> WEBWEU5
    WEBWE[7] <-> WEBWEU7
    WEBWE[6] <-> WEBWEU6

getPinMappingsP2L returns the correct subset of pins:

RAMB36_X0Y29 (RAMB36E1) RAMB36E1 getPinMappingsP2L:
    WEBWE[0] <-> WEBWEL1
    WEBWE[0] <-> WEBWEL0
    WEBWE[0] <-> WEBWEL7
    WEBWE[0] <-> WEBWEL6
    WEBWE[0] <-> WEBWEL5
    WEBWE[0] <-> WEBWEL4
    WEBWE[0] <-> WEBWEL3
    WEBWE[0] <-> WEBWEL2
    WEBWE[0] <-> WEBWEU7
    WEBWE[0] <-> WEBWEU6
    WEBWE[0] <-> WEBWEU5
    WEBWE[0] <-> WEBWEU4
    WEBWE[0] <-> WEBWEU3
    WEBWE[0] <-> WEBWEU2
    WEBWE[0] <-> WEBWEU1
    WEBWE[0] <-> WEBWEU0

So getPinMappingsL2P is reporting that WEBWE[1] should map to WEBWEU1, but for SDP WRITE_WIDTH_B=72, WEBWE[1] is actual unmapped. 

From Vivado, run:

link_design -part xc7a35tcpg236-1
set c [create_cell -ref RAMB36E1 test2]
place_cell $c RAMB36_X0Y17
set_property WRITE_WIDTH_B 72 $c
set_property WRITE_WIDTH_A 0 $c
set_property RAM_MODE SDP $c
get_bel_pins -of [get_pins $c/WEBWE[0]]
get_bel_pins -of [get_pins test2/WEBWE[1]]
get_bel_pins -of [get_pins test2/WEBWE[2]]
get_bel_pins -of [get_pins test2/WEBWE[3]]

Returns:

RAMB36_X0Y17/RAMB36E1/WEBWEL0 RAMB36_X0Y17/RAMB36E1/WEBWEL1 RAMB36_X0Y17/RAMB36E1/WEBWEL2 RAMB36_X0Y17/RAMB36E1/WEBWEL3 RAMB36_X0Y17/RAMB36E1/WEBWEU0 RAMB36_X0Y17/RAMB36E1/WEBWEU1 RAMB36_X0Y17/RAMB36E1/WEBWEU2 RAMB36_X0Y17/RAMB36E1/WEBWEU3
> WARNING: [Common 17-346] No Bel Pins matched '*'
> WARNING: [Common 17-346] No Bel Pins matched '*'
> WARNING: [Common 17-346] No Bel Pins matched '*'

[clavin-xlnx]

The issue is that in a minority of instances, logical cell pins can have a one-to-many mapping to physical pins. RapidWright uses two maps to provide fast pin mapping access in either direction and so the Logical-to-Physical map loses information when populated.

Perhaps it makes sense to simply remove the Logical-to-Physical map? Creating another list object for every pin in a design will negatively impact memory usage and runtime. A new API could be created that lazily builds the multi-map and returns the list of physical pins the logical pin maps to.

[litghost]

I'd argue what should happen is L2P should return null when the logical pin is missing from the pinout. I'd also argue that a multimap is the right data structure here. Alternatively, the signature should be changed to Map<String, Set<String>>

[clavin-xlnx]

This should be fixed in 2020.1.6. The internal data structure for logical to physical pins has been replaced with a lazily-built hash multimap. The return types of relevant APIs have also been updated accordingly:

• In com.xilinx.rapidwright.design.Cell public Map<String,String> getPinMappingsL2P() --> Changed return type: public Map<String,Set<String>> getPinMappingsL2P()
• In com.xilinx.rapidwright.design.Cell public List<String> getAllPhysicalPinMappings(String logicalPin) --> Changed return type: public Set<String> getAllPhysicalPinMappings(String logicalPin)