Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong LUT equation when used as LUT-thru #76

Closed
acomodi opened this issue Apr 29, 2021 · 31 comments · Fixed by YosysHQ/nextpnr#706
Closed

Wrong LUT equation when used as LUT-thru #76

acomodi opened this issue Apr 29, 2021 · 31 comments · Fixed by YosysHQ/nextpnr#706
Labels
bug Something isn't working

Comments

@acomodi
Copy link
Contributor

acomodi commented Apr 29, 2021

While debugging the Murax design with the interchange format, the following DRC errors were generated:

ERROR: [DRC PDRC-131] SLICE_PairEqSame_A6A5_ERROR: Incompatible programming for site SLICE_X11Y72. A6LUT and A5LUT must have a compatible equation, lower bits must be programmed the same.
ERROR: [DRC PDRC-135] SLICE_PairEqSame_C6C5_ERROR: Incompatible programming for site SLICE_X1Y83. C6LUT and C5LUT must have a compatible equation, lower bits must be programmed the same.

These correspond to LUTs where both the O5 and O6 pins were used to have signals routed through the LUTs (one as a pseudo PIP and the other as a site PIP).

Screenshot from 2021-04-29 18-18-28

The LUT equations in this case are:

O6=(A6+~A6)*((A6))
O5=(A5)

Given that this should be a valid configuration, I wonder whether the issue might be happening when writing the DCP with RapidWright.

In addition, the following warnings appear as well, which I believe should be taken into consideration:

WARNING: [DRC PDRC-132] SLICE_PairEqSame_A6A5_WARN: Luts A6LUT and A5LUT in use in site SLICE_X0Y81 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-132] SLICE_PairEqSame_A6A5_WARN: Luts A6LUT and A5LUT in use in site SLICE_X11Y70 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-132] SLICE_PairEqSame_A6A5_WARN: Luts A6LUT and A5LUT in use in site SLICE_X3Y81 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-132] SLICE_PairEqSame_A6A5_WARN: Luts A6LUT and A5LUT in use in site SLICE_X5Y38 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-132] SLICE_PairEqSame_A6A5_WARN: Luts A6LUT and A5LUT in use in site SLICE_X5Y80 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-132] SLICE_PairEqSame_A6A5_WARN: Luts A6LUT and A5LUT in use in site SLICE_X7Y56 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-134] SLICE_PairEqSame_B6B5_WARN: Luts B6LUT and B5LUT in use in site SLICE_X0Y67 with different equations without A6 pin connected to Global Logic High.
WARNING: [DRC PDRC-134] SLICE_PairEqSame_B6B5_WARN: Luts B6LUT and B5LUT in use in site SLICE_X11Y79 with different equations without A6 pin connected to Global Logic High.
@issuelabeler issuelabeler bot added the bug Something isn't working label Apr 29, 2021
@gatecat
Copy link
Contributor

gatecat commented Apr 29, 2021

I don't think this configuration is valid, so there is a bug in nextpnr-fpga_interchange's LUT and pseudo pip logic.

8Z6QE

I5 (i.e. A6 in this context) is used to select O6 between O5 (the lower half of the LUT function) and the upper half of the LUT function. As O5 is defined to be A5, O6 can't always be A6.

@acomodi
Copy link
Contributor Author

acomodi commented Apr 29, 2021

@gatecat @clavin-xlnx Here is the murax DCP

@gatecat
Copy link
Contributor

gatecat commented Apr 29, 2021

Looking at the DCP and nextpnr side code, I think the problem is that LUT function constraints are only being considered for LUT route-through pseudo-tile-PIPs and not LUT routethrough site-PIPs. This is something I can look into fixing, if you want.

@acomodi
Copy link
Contributor Author

acomodi commented Apr 29, 2021

All right, I think I might look into this so that I start to get more familiarity with it and nextpnr in general.

I don't think this configuration is valid, so there is a bug in nextpnr-fpga_interchange's LUT and pseudo pip logic.

So, If I understand correctly, we cannot have ever two LUT thrus where one of the pins used is A6, as this is actually a selection pin, which eventually can conflict with the lower half which controls the 05 output.
In general though, having two LUT-thrus on pins other than A6 should be fine (provided that A6 is routed to VCC).

@acomodi
Copy link
Contributor Author

acomodi commented Apr 30, 2021

After debugging this problem in more depth in nextpnr, I think what is happening is the following:

  • the site router does his job and binds sitePIPs corresponding to LUT-thrus and everything is fine at this point. In the error case, the A5 to O5 output has been bound to BLUT5.
  • As you mentioned, it seems indeed that the update site routine does not recognize the site-through PIP as being active
  • The main router, which can bind pseudo tile PIP to route through tiles will check the availability of the pseudo PIPs (if chosen as potential candidates) is not aware about the fact that the site with the pseudo site-thru PIP (A5 -> O5 for BLUT5) is active and binds the B6 -> B pseudo PIP, which should really not be allowed.

I think the fix should be added to the update site routine, to be able to identify also the site-thru PIPs and, based on their activation state, remove from the allowed pseudo PIPs all those which can generate conflicts later on. @gatecat does this sound good?

@gatecat
Copy link
Contributor

gatecat commented Apr 30, 2021

One option would be to create wire LUTs to the route-throughs, similar to how pseudo-pips work, which would then mean the usual LUT-blocking code would block the pseudo-pips:

https://github.com/gatecat/nextpnr/blob/3dd89863220d92bddaf21acae78ab16179dae6eb/fpga_interchange/pseudo_pip_model.cc#L369-L397

however, it is worth noting that site updates are already a performance critical path and this will only slow things down more. But long term (i.e. on the ~3-month timeframe) I think a substantial performance-focused rewrite of the site related code will be needed in any case.

GitHub
nextpnr portable FPGA place and route tool. Contribute to gatecat/nextpnr development by creating an account on GitHub.

@acomodi
Copy link
Contributor Author

acomodi commented May 5, 2021

All right, so, the problem I am having at the moment is how to get the information on how the "other" LUT's connectivity. In other words, all tile pseudo-pips, as far as I have seen, only imply A6 -> O6 connections in the LUT6 bel, therefore I am not sure how to get the info about the LUT5 bel that may contain a LUT-thru.

The solution might be that, if a LUT-thru is present in the LUT5 (or in general in one of the "other" LUT), the pseudo-pip is not valid, or actually, as you suggested, to add the LUT-wire equation to the LUT6 and have the lut-blocking code do the rest.

On a side note, I think that this issue might need to be pushed higher in the priorities, so that we could better test these kinds of situations (unsure if we can control general routing during testing, so to use also tile pseudo pips). The problem is that this specific issue is hard to reproduce in small designs, as the router will not use pseudo pips if it does not need to.

@acomodi acomodi added this to In progress in FPGA interchange bootstrapping May 5, 2021
@acomodi
Copy link
Contributor Author

acomodi commented May 5, 2021

While further debugging this issue, I came across a similar one, which do not exactly fall in the exact same category though.

In this failing implementation (includes dcp, phys.yaml and netlist.yaml), the same BLUT (SLICE_X5Y39/B6LUT) uses the same A3 pin, both for an explicit LUT and for a site-thru LUT, which I suppose shouldn't be allowed.

@gatecat
Copy link
Contributor

gatecat commented May 5, 2021

In this failing implementation (includes dcp, phys.yaml and netlist.yaml), the same BLUT (SLICE_X5Y39/B6LUT) uses the same A3 pin, both for an explicit LUT and for a site-thru LUT, which I suppose shouldn't be allowed.

Hmm, as the net appears to be the same for both uses of the pin (LUT and route-thru), and everything else is legal (A6 not used and no apparent output overlap) it should technically be a valid usage - no different to packing two LUT5s sharing some inputs together, which is a valid and intended use of the structure. I'm not immediately sure why Vivado rejects this pattern, there might be some subtlety with route-thrus that means this isn't allowed, or it's a red herring and the problem is elsewhere...

@acomodi
Copy link
Contributor Author

acomodi commented May 5, 2021

@gatecat You are right, I missed that the net corresponding to the A3 pin is actually the same, and indeed this might not be the root cause of this issue.

What I see with the report_route_status is the following:

open_checkpoint: Time (s): cpu = 00:00:22 ; elapsed = 00:00:08 . Memory (MB): peak = 6411.957 ; gain = 498.770 ; free physical = 6141 ; free virtual = 12562
report_route_status
Design Route Status
                                               :      # nets :
   ------------------------------------------- : ----------- :
   # of logical nets.......................... :       13459 :
       # of nets not needing routing.......... :        8841 :
           # of internally routed nets........ :        8728 :
           # of nets with no loads............ :         113 :
       # of routable nets..................... :        4618 :
           # of fully routed nets............. :        4617 :
       # of nets with routing errors.......... :           1 :
           # of nets with antennas/islands.... :           1 :
   ------------------------------------------- : ----------- :


Nets with Routing Errors:
  murax._zz_104
    Antenna Nodes: CLBLL_L_X4Y39/CLBLL_L_B

Where the antenna node corresponds to the wire-through. This is quite interesting and the problem might be that the same net is being output from the same site twice, which might not be accepted? Quite unclear at the moment. @clavin-xlnx Do you have some additional insights on what might be wrong here?

@clavin-xlnx
Copy link

@acomodi I am looking into the issue. I currently can't get python-fpga-interchange to compile, so if you happen to have the binary Interchange files handy, that might be useful.

@acomodi
Copy link
Contributor Author

acomodi commented May 5, 2021

so if you happen to have the binary Interchange files handy, that might be useful.

@clavin-xlnx Sure, I have packed the physical and logical capnp netlists alongside the dcp and the yamls: failing_murax.tar.gz

@clavin-xlnx
Copy link

One thing I am not sure about is that there is a psuedo PIP CLBLL_L_X4Y39/CLBLL_L.CLBLL_L_B3->>CLBLL_L_B that makes the route thru connection intended, but I don't see it in the routing of the net murax._zz_104. I've looked through >100 other designs on Series 7 to try and find a similar example, but none seem to use any psuedo PIPs like one above. What is more surprising is that using Vivado's own find_routing_path:

find_routing_path -from [get_nodes CLBLL_L_X4Y39/CLBLL_L_B3] -to [get_nodes CLBLL_L_X4Y39/CLBLL_L_B]
No path found

It doesn't seem like Vivado generally produces routes that take advantage of that PIP, but it may still be valid. I just have to keep digging a little deeper.

@clavin-xlnx
Copy link

Strangely, if I move backwards by one hop, the method does find a path from B3->B:

find_routing_path -from [get_nodes INT_L_X4Y39/IMUX_L17] -to [get_nodes CLBLL_L_X4Y39/CLBLL_LL_B]
INT_L_X4Y39/IMUX_L17 CLBLL_L_X4Y39/CLBLL_LL_B3 CLBLL_L_X4Y39/CLBLL_LL_B

@acomodi
Copy link
Contributor Author

acomodi commented May 6, 2021

The main problem I see is that this kind of pseudo PIP is used quite often in the linked design, and in all the other examples I found, all the nets were correctly routed. One of the main differences between a correctly routed net using a LUT-thru and the antenna net was that, in the latter, the same input net to B3 is used both as a "normal" LUT5 input and also as a LUT6 route-thru.

@gatecat
Copy link
Contributor

gatecat commented May 6, 2021

fwiw, this TCL - run on an empty xc7a35t design - seems to demonstrate successful use of the pip shared with a LUT (ignoring the unrouted Vcc, but that should be immaterial here):

# Clear out the design
route_design -unroute
remove_cell *
remove_net *

create_cell -reference VCC vcc_src
create_net vcc_net
connect_net -net vcc_net -objects vcc_src


create_cell -reference LUT1 driver_lut_1
place_cell driver_lut_1 SLICE_X4Y39/B6LUT

create_cell -reference LUT4 sink_lut_1
place_cell sink_lut_1 SLICE_X5Y39/B5LUT

create_net output_net
connect_net -net output_net -objects sink_lut_1/O

# Force use of the B3 pin with some dummy pins
connect_net -net vcc_net -objects [list sink_lut_1/I1 sink_lut_1/I2]

# Route a net from the driver LUT to the B3 pin of the sink LUT,
# and also routed through the B3 pin back to the driver LUT
create_net b3_net
connect_net -net b3_net -objects [list driver_lut_1/O sink_lut_1/I0 driver_lut_1/I0]

set_property ROUTE {CLBLL_L_X4Y39/CLBLL_LL_B CLBLL_L_X4Y39/CLBLL_LOGIC_OUTS13 \
	INT_L_X4Y39/NW2BEG1 INT_R_X3Y40/EL1BEG0 INT_L_X4Y40/SL1BEG0 INT_L_X4Y39/IMUX_L16 \
	CLBLL_L_X4Y39/CLBLL_L_B3 \
	CLBLL_L_X4Y39/CLBLL_L_B CLBLL_L_X4Y39/CLBLL_LOGIC_OUTS9 INT_L_X4Y39/BYP_ALT4 \
	INT_L_X4Y39/BYP_BOUNCE4 INT_L_X4Y39/IMUX_L12 CLBLL_L_X4Y39/CLBLL_LL_B6} [get_nets b3_net]

Screenshot from 2021-05-06 11-26-14

@gatecat
Copy link
Contributor

gatecat commented May 6, 2021

This TCL aims to reproduce the "two output sitepins on the same net" hypothesis that was also discussed, and also it seems like the route-thru works fine:

# Clear out the design
route_design -unroute
remove_cell *
remove_net *

# Create a DFF with D driven by Q for testing
create_cell -reference FDRE test_dff
create_net test_net
connect_net -net test_net -objects [list test_dff/D test_dff/Q]

place_cell test_dff SLICE_X5Y39/AFF

# Route from DFF Q to D, but via  LUT route-thru in the same site

# FF output -> B3->B LUT routethrough -> AX
set_property ROUTE {CLBLL_L_X4Y39/CLBLL_L_AQ CLBLL_L_X4Y39/CLBLL_LOGIC_OUTS0 INT_L_X4Y39/IMUX_L16 CLBLL_L_X4Y39/CLBLL_L_B3 \
	CLBLL_L_X4Y39/CLBLL_L_B CLBLL_L_X4Y39/CLBLL_LOGIC_OUTS9 INT_L_X4Y39/FAN_ALT2 INT_L_X4Y39/FAN_BOUNCE2 \
	INT_L_X4Y39/BYP_ALT0 INT_L_X4Y39/BYP_L0 CLBLL_L_X4Y39/CLBLL_L_AX} [get_nets test_net]

Screenshot from 2021-05-06 11-41-18

So it seems like there might be some very complex interaction going on here...

@acomodi
Copy link
Contributor Author

acomodi commented May 6, 2021

Yeah, I have experimented a bit more and it seems that both situations are actually accepted when dealt by Vivado. The following TCL should enclose both of them and mimic the failing antenna net:

 open_checkpoint test.dcp
 
 # Clear out the design
 route_design -unroute
 remove_cell *
 remove_net *
 
 create_cell -reference LUT2 driver_lut_1
 place_cell driver_lut_1 SLICE_X4Y39/B6LUT
 
 create_cell -reference LUT1 sink_lut_1
 
 place_cell sink_lut_1 SLICE_X5Y39/B5LUT
 
 create_cell -reference FDRE fdre_1
 place_cell fdre_1 SLICE_X5Y39/AFF
 create_net test_net
 connect_net -net test_net -objects [list sink_lut_1/O driver_lut_1/I0]
 
 route_design
 
 # Route a net from the driver LUT to the B3 pin of the sink LUT,
 # and also routed through the B3 pin back to the driver LUT
 create_net b3_net
 connect_net -net b3_net -objects [list fdre_1/Q sink_lut_1/I0 driver_lut_1/I1]
 
 reset_property LOCK_PINS [get_cells sink_lut_1]
 set_property LOCK_PINS {I0:A3} [get_cells sink_lut_1]
 route_design -unroute -nets [get_nets b3_net]
 
 set_property ROUTE {CLBLL_L_X4Y39/CLBLL_L_AQ \
     CLBLL_L_X4Y39/CLBLL_LOGIC_OUTS0 \
     INT_L_X4Y39/IMUX_L16 \
     CLBLL_L_X4Y39/CLBLL_L_B3 \
     CLBLL_L_X4Y39/CLBLL_L_B CLBLL_L_X4Y39/CLBLL_LOGIC_OUTS9 INT_L_X4Y39/BYP_ALT4 \
     INT_L_X4Y39/BYP_BOUNCE4 INT_L_X4Y39/IMUX_L12 CLBLL_L_X4Y39/CLBLL_LL_B6} [get_nets b3_net]
 
 write_checkpoint -force test3.dcp

I wonder whether the DCP generated starting from the interchange might be missing some information required by Vivado to handle this case?

@gatecat
Copy link
Contributor

gatecat commented May 6, 2021

It seems like I can 'fix' the broken design by manually patching the ROUTE property of the offending net using

set_property ROUTE "(  { CLBLL_L_X4Y39/CLBLL_L_AQ CLBLL_LOGIC_OUTS0  { IMUX_L16 CLBLL_L_B3 CLBLL_L_B CLBLL_LOGIC_OUTS9 NL1BEG0 IMUX_L15 CLBLL_LL_B1}  IMUX_L8 CLBLL_LL_A5 CLBLL_LL_A CLBLL_LL_AMUX CLBLL_LOGIC_OUTS20  { IMUX_L36 CLBLL_L_D2 }  IMUX_L20 CLBLL_L_C2 }  )  " [get_nets murax._zz_104]

in the broken form it is

(  { CLBLL_L_X4Y39/CLBLL_L_B CLBLL_LOGIC_OUTS9 NL1BEG0 IMUX_L15 CLBLL_LL_B1 }  )   (  { CLBLL_L_X4Y39/CLBLL_L_AQ CLBLL_LOGIC_OUTS0  { IMUX_L16 CLBLL_L_B3 }  IMUX_L8 CLBLL_LL_A5 CLBLL_LL_A CLBLL_LL_AMUX CLBLL_LOGIC_OUTS20  { IMUX_L36 CLBLL_L_D2 }  IMUX_L20 CLBLL_L_C2 }  )

as if the connectivity across the PIP isn't being seen.

Looking at the YAML, it seems like the route-thru is specified as a bel and so the PIP is never actually being passed to Vivado?

                      - routeSegment:
                          pip:
                            tile: INT_L_X4Y39
                            wire0: LOGIC_OUTS_L0
                            wire1: IMUX_L16
                            forward: true
                            isFixed: false
                        branches:
                          - routeSegment:
                              pip:
                                tile: CLBLL_L_X4Y39
                                wire0: CLBLL_IMUX16
                                wire1: CLBLL_L_B3
                                forward: true
                                isFixed: false
                            branches:
                              - routeSegment:
                                  sitePin:
                                    site: SLICE_X5Y39
                                    pin: B3
                                branches:
                                  - routeSegment:
                                      belPin:
                                        site: SLICE_X5Y39
                                        bel: B3
                                        pin: B3
                                    branches:
                                      - routeSegment:
                                          belPin:
                                            site: SLICE_X5Y39
                                            bel: B6LUT
                                            pin: A3
                                        branches:
                                          - routeSegment:
                                              belPin:
                                                site: SLICE_X5Y39
                                                bel: B6LUT
                                                pin: O6
                                            branches:
                                              - routeSegment:
                                                  sitePIP:
                                                    site: SLICE_X5Y39
                                                    bel: BUSED
                                                    pin: 0
                                                    isFixed: false
                                                    isInverting: false
                                                branches:
                                                  - routeSegment:
                                                      belPin:
                                                        site: SLICE_X5Y39
                                                        bel: B
                                                        pin: B
                                                    branches:
                                                      - routeSegment:
                                                          sitePin:
                                                            site: SLICE_X5Y39
                                                            pin: B
                                                        branches:
                                                          - routeSegment:
                                                              pip:
                                                                tile: CLBLL_L_X4Y39
                                                                wire0: CLBLL_L_B
                                                                wire1: CLBLL_LOGIC_OUTS9
                                                                forward: true
                                                                isFixed: false
                                                            branches:
                                                              - routeSegment:
                                                                  pip:
                                                                    tile: INT_L_X4Y39
                                                                    wire0: LOGIC_OUTS_L9
                                                                    wire1: NL1BEG0
                                                                    forward: true
                                                                    isFixed: false
                                                                branches:
                                                                  - routeSegment:
                                                                      pip:
                                                                        tile: INT_L_X4Y39
                                                                        wire0: NL1END_S3_0
                                                                        wire1: IMUX_L15
                                                                        forward: true
                                                                        isFixed: false

It seems like this might be better if it just included the route-thru PIP as a regular PIP and not all the site routing. Some more investigation needed to find out what is happening here compared to what is supposed to happen, after all LUT route-thrus are working fine 99% of the time.

@gatecat
Copy link
Contributor

gatecat commented May 6, 2021

Even stranger there are other cases where - correctly behaving - LUT route-thru PIPs are written as if they were regular PIPs in the YAML:

                                  - routeSegment:
                                      pip:
                                        tile: CLBLM_L_X10Y60
                                        wire0: CLBLM_IMUX6
                                        wire1: CLBLM_L_A1
                                        forward: true
                                        isFixed: false
                                    branches:
                                      - routeSegment:
                                          pip:
                                            tile: CLBLM_L_X10Y60
                                            wire0: CLBLM_L_A1
                                            wire1: CLBLM_L_A
                                            forward: true
                                            isFixed: false
                                        branches:
                                          - routeSegment:
                                              pip:
                                                tile: CLBLM_L_X10Y60
                                                wire0: CLBLM_L_A
                                                wire1: CLBLM_LOGIC_OUTS8
                                                forward: true
                                                isFixed: false

So something must be up with this specific PIP that's causing it to be written as a bel rather than a PIP?

@acomodi
Copy link
Contributor Author

acomodi commented May 6, 2021

So, afaiu these are actually two different route-thrus that happen at two different stages.

  • tile LUT-thru. This ends up being a PIP, and is used by the main router to route through the whole tile. (This is part of the original issue described here, when another kind of LUT-thru is present at the same LUT5-6 pair).
  • site pin LUT-thru. This one is computed during site routing, therefore it is not expressed as a typical routing PIP but rather as a set of BEL pins (indicating that this is indeed a pseudo PIP)

@gatecat
Copy link
Contributor

gatecat commented May 6, 2021

Sorry for somewhat thinking aloud here (edit and I think our messages collided somewhat...)

I think the problem is that the LUT route-thru site pip, rather than tile pip is being used. It is unclear in the current semantics of the interchange format if this is legal in this case or not. If it is then some tweaks in the DCP generation will be needed to get Vivado to recognise the site pip.

Otherwise, if it's not legal, this nextpnr check needs to be fixed:

https://github.com/YosysHQ/nextpnr/blob/c322cda3f875a5e5dd2575d3a390cbe1cee073e0/fpga_interchange/arch.cc#L1769-L1791

to specifically reject the use of a site PIP when the site has been left and then re-entered; rather than with the simpler constraint that the driver of the net is in the same site as the site PIP.

GitHub
nextpnr portable FPGA place and route tool. Contribute to YosysHQ/nextpnr development by creating an account on GitHub.

@acomodi
Copy link
Contributor Author

acomodi commented May 6, 2021

Sorry for somewhat thinking aloud here

No probs, I am all for thinking aloud, so it is easier to get to the bottom of the problem.

Now I see what you mean, and I think I agree, as in, a net going through a site PIP should not be able to "escape" the site and this indeed needs to be marked as illegal. A LUT-thru site PIP should be allowed if the sink is in the same path and in the same site where the LUT-thru is present, otherwise, as you pointed out, the site PIP should really be a tile PIP and handled by the router. And this should extend to all site-thrus of course and not only LUT ones.

@clavin-xlnx
Copy link

Ok, I have confirmed on my side when a route is passing through a site by the means of an input site pin to an output site pin, it must use the Tile PIP (in this case CLBLL_L_X4Y39/CLBLL_L.CLBLL_L_B3->>CLBLL_L_B), instead of site routing (intra-site routing resources can't generally be used for inter-site routing). I tried using @gatecat's Tcl fix, but I was still getting issues when running report_route_status, but I think the spirit of the fix is the same. I was able to confirm by making the switch in RapidWright by reading in the Interchange files, switching from site routing to a Tile PIP and then writing to a DCP. This yields 0 errors when running report_route_status and the net and design appears to be fully routed in Vivado. Here is the RapidWright code that swaps the routing (from intra-site to inter-site):

Design design = PhysNetlistReader.readPhysNetlist("murax_basys3.phys", 
  LogNetlistReader.readLogNetlist("murax_basys3.netlist"));

// Add Tile route thru PIP
design.getNet("murax._zz_104").addPIP(
  new PIP("CLBLL_L_X4Y39/CLBLL_L.CLBLL_L_B3->>CLBLL_L_B", design.getDevice()));

// Remove existing site routing
SiteInst si = design.getSiteInst("SLICE_X5Y39");
si.unrouteIntraSiteNet(si.getBEL("B6LUT").getPin("O6"), si.getBEL("B").getPin("B"));

design.writeCheckpoint("murax_basys3_fixed.dcp");

As discussed, this opens the question of should such a preference be part of the Interchange, or, should we just clarify what should happen in intra-site vs inter site routing. From my perspective, it seems the issue here is that an intra-site routing solution was used to solve an inter-site routing problem. In Vivado, routing inside of a site happens prior to inter-site routing and uses intra-site routing resources (SitePIPs and SiteWire net annotations) to route inside a site. Tile route thru PIPs are present to allow inter-site routing to make use of the site resources. Generally, when performing inter-site routing, all the sources and sinks are routed out to a site pin already, so there should generally be no need (all placements remaining static) to route inside a site.

For the Interchange, nextpnr and other tools, I assumed there is a distinction/ordering between intra-site routing and inter-site routing. However, the nature of how they are being executed is perhaps more broad than that. My personal assumption was that intra-site routing happens prior to inter-site routing just as I assume placement takes place prior to routing. Perhaps @gatecat can add more perspective here.

@acomodi
Copy link
Contributor Author

acomodi commented May 7, 2021

Thanks for double-checking this @clavin-xlnx. At this point, I think we have a pretty good understanding of what is happening, and I believe that a fix should be added to the site router, which is the one that yields intra-site routes. The particular faulty path should be generated at that stage, which is prior to the general router, which is disallowed to perform intra-site routing and can only perform inter-site routing.

As far as I have seen, at the moment, the site router is aware of the immediate adjacent routing resources, which might allow for this situation to happen (e.g. in this case, a net is allowed to be routed through a neighbouring SLICE using site resources, entering and exiting the site without being produced or consumed by any source/sink). Therefore, another possible location for a fix is in the site router indeed.

@acomodi
Copy link
Contributor Author

acomodi commented May 13, 2021

With YosysHQ/nextpnr#700 and YosysHQ/nextpnr#695, the illegal site-thru should now be solved.

What remains is the original issue for which a tile pseudo PIP is used illegally, based on the LUT5 status. That is, if the tile pseudo PIP passes through the A6 -> O6 pins of the LUT6, the LUT5 cannot have a site-thru as well.

This is a case that cannot be generalized IMO, unless we assume that all LUTs work in the same way (e.g. the uppermost input is used as a mux selector). Probably the right way to handle this is to use the constraint mechanism (which I think still needs to be implemented in nextpnr), so that some PIPs get excluded for specific situations.

As a very naive workaround, if there is no other immediate solution, I think that we can temporarily disable pseudo PIPs passing through a LUT6. @gatecat thoughts on that?

@gatecat
Copy link
Contributor

gatecat commented May 13, 2021

Tile pseudo PIPs should be checking the LUT equation already:

https://github.com/YosysHQ/nextpnr/blob/21d594a150ba2bc6e595c62d107cfd84e734fa5a/fpga_interchange/pseudo_pip_model.cc#L357-L397

I think the problem is that if a site LUT route-through is used (for example to access a FF, which is still legal post recent patches), a wire LUT isn't created so the pseudo PIP validity checker doesn't see that A6 is unavailable.

GitHub
nextpnr portable FPGA place and route tool. Contribute to YosysHQ/nextpnr development by creating an account on GitHub.

@acomodi
Copy link
Contributor Author

acomodi commented May 13, 2021

Ok, I think I get it and have a possible solution for this. As you said, the LUT route thru at site level are not marked as such.

I believe that yet another piece of data needs to be recorded for the site status, which stores all LUT BELs used as site pseudo PIP, as well as the input/output pins, such that the wire lut can be added as a cell in the lut mapper, which will then have a complete information on the lut element, discarding invalid tile psuedo PIPs.

@gatecat
Copy link
Contributor

gatecat commented May 13, 2021

My understanding is that the current site router implies this when the bel associated with the site PIP is a LUT bel. But adding explicit tags for this wouldn't be a bad idea, if you do look into this also consider chipsalliance/fpga-interchange-schema#47 at the same time.

@acomodi
Copy link
Contributor Author

acomodi commented May 13, 2021

My bad, the solution I was thinking of does not imply modifying the schema, but nextpnr, so that when the site router has finished, we have a lookup to check which LUT wires (if any) were used in each site.

@gatecat
Copy link
Contributor

gatecat commented May 13, 2021

Yes, that sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants