Xilinx / RapidWright

Build Customized FPGA Implementations for Vivado
http://www.rapidwright.io
Other
294 stars 108 forks source link

Routethru LUT and FF cells compare equal #611

Closed jgoeders closed 1 year ago

jgoeders commented 1 year ago

As noted in https://github.com/Xilinx/RapidWright/issues/36, Cell.getSiteInst().getCells() will return route thru LUT cells that would otherwise not be returned by Design.getCells().

However, I've noticed that the LUT routethru cell and the FF cell compare as equal.

matching_cells = [c for c in cell.getSiteInst().getCells() if c.getName() == cell.getName()]
for c in matching_cells:
  print(c)
print(matching_cells[0] == matching_cells[1])

This code prints:

btnu_d_reg(BEL: CFF) CFF
btnu_d_reg(BEL: C5LUT) C5LUT
True

In my opinion this behavior is unexpected; however, if it is designed to work this way there should perhaps be a warning in the documentation.

For example, my goal was to find the associated FF cell for a given route thru cell, but my search returned an empty list:

matching_cells = [ c for c in cell.getSiteInst().getCells() if c.getName() == cell.getName() and c != cell]

I suppose it's possible that this is a Jpype artifact as I haven't tested in Java.

clavin-xlnx commented 1 year ago

Thanks, this issue is interesting. In Java, there are two main ways to check for equality:

  1. Reference equality, ==
  2. Object equality, equals()

In this case, there is some interesting behavior in that the CFF and C5LUT cells are both equal from an object perspective (they have the same name, which is one of the equality checks in Cell.equals()). They are not, however, reference equal as they are different instances of the Cell class and reside in different locations in memory.

The issue is that Jpype had to choose one of the two Java equality methods for Python's == and != and they chose equals() for reasons cited here (see the section on Equality). Clearly, this is a pitfall for Python RapidWright users and I will update the documentation accordingly.

To work around the issue in Python and achieve your expected behavior, you can use the following:

matching_cells = [ c for c in cell.getSiteInst().getCells() if c.getName() == cell.getName() and c.isRoutethru() != cell.isRoutethru()]

This does bring up the question of if Cell.equals() should also check for the routethru flag, so we'll probably think about that and perhaps make a change there if it is not too disruptive.

jgoeders commented 1 year ago

Thanks! This is very helpful. I'll leave the issue open in case you want to amend equals(), but feel free to close it as your workaround should work fine. I mainly wanted to document the behavior somewhere.

jgoeders commented 1 year ago

@clavin-xlnx I noticed a few other issues when dealing with the "routethru cell". I'm not sure if it is meant to behave this way or whether they are issues and I should open a new issue. ...figured I would ask here first.

  1. getPinMappingsL2P() returns a somewhat strange value. For a LUT routethru I would expect something like {I0=[A4]}, but instead I get {D=[A4]}, where D is of course a logical pin on the FF. It's not a big deal and I can work around it, it's just a bit unexpected.

  2. LUTTools.getLUTEquation(lut_passthru_cell) produces a java.lang.RuntimeException: ERROR: Unsupported LUT Size/INIT value 1'b0. Again it looks like it's using the INIT value from the FF cell which throws an exception. Once again, not too surprising since there isn't actually a LUT cell in the netlist, but I didn't see this documented anywhere.

clavin-xlnx commented 1 year ago

Regarding question 1, this is expected as it is faithfully reproducing what can be seen in Vivado. For example, our test suite has a routethru example that routes through a LUT to a FF: image image The above image shows a snapshot of the ff6 FF cell placed on the AFF BEL cell pin properties. There is a mapping from the D input of the FF from the A1 input from the routed-thru LUT. Corresponding mappings will be found on the relevant cells in RapidWright for the same design.

Regarding question 2, the ERROR is unfortunate as getLUTEquation() doesn't check if the provided cell is indeed a LUT. In this case, the code is trying to get the EDIFCellInst (logical netlist cell) of the provided physical cell. It does this by looking up the name and since there is no logical cell equivalent in the netlist, it finds the identically named flip-flop instance. The LUT INIT values are stored on the logical cell instances and must be translated to get the "physical" equation. Since LUT routethrus don't have a logical equivalent, getLUTEquation() won't be able to help much here. A new method that examines the cell pin mappings of a LUT routethru would be needed to generate the physical LUT equation.

jgoeders commented 1 year ago

Thanks! Good to know for Q1.

Yes, for Q2 it's easy to generate a simple equation using the pin map. The explanation of the error makes perfect sense. Thanks.

clavin-xlnx commented 1 year ago

The 2022.2.2 release updates Cell.equals() and Cell.hashCode() to take into account the flag indicating whether a cell is a routethru or not.