YosysHQ / arachne-pnr

Place and route tool for FPGAs
MIT License
414 stars 72 forks source link

arachne-pnr: src/route.cc:653: void Router::route(): Assertion `cnet_net[cn] == nullptr || cnet_net[cn] == n' failed. #65

Closed Bill-3d closed 6 years ago

Bill-3d commented 7 years ago

I'm developing on a BlackIce board, basically an ICE40 FPGA.

My code is a very simple counter on 4 LEDS, almost identical with a Blackice Tutorial example, but with a software reset facility, which I need for further development. The tutorial example compiles and runs without problems, but with the reset counter added in, I get the assertion failure.

I had a lot of other code in there, but after cutting it back to the minimum that still fails it is:

module test(input CLK100,
        output [3:0] LEDS
        );

   reg [26:0]      rst_ct;
   reg [26:0]      bigcount;
   wire        reset;

   assign reset   = !rst_ct[26];
   assign LEDS = bigcount[26:23];

   always @ (posedge CLK100) 
     begin
    if (rst_ct[26]) begin
    end
    else
      rst_ct <= rst_ct + 27'd1;
     end

   always @ (posedge CLK100)
     if (reset) 
       begin
      bigcount <= 27'd0;
       end
     else
       begin
      bigcount <= bigcount + 27'd1;
       end // else: !if(reset)
endmodule // test

My .pcf file is:

# .pcf file for BlackIce
#
# based on Richard Miller's version - thanks, Richard!
#
#LEDs 
set_io --warn-no-port LEDS[0] 70 # LD1
set_io --warn-no-port LEDS[1] 68 # LD2
set_io --warn-no-port LEDS[2] 67 # LD3
set_io --warn-no-port LEDS[3] 71 # LD4

#PMOD13: 17A 17B 23A 23B
# set_io --warn-no-port PMOD13A 23
# set_io --warn-no-port PMOD13B 24
# set_io --warn-no-port RX 28
# set_io --warn-no-port TX 29

# Onboard 100Mhz oscillator
set_io CLK100 129

My version of IceStorm:

Yosys 0.7+74 (git sha1 45e10c1, clang  -fPIC -Os)    and
arachne-pnr 0.1+187+0 (git sha1 e97e35c, g++ 4.8.4-2ubuntu1~14.04.3 -O2)

I'm happy to try work-arounds if you can suggest them.

Thanks, Bill

ghost commented 7 years ago

Hello there,

Reading your code, I am not sure what logic are you trying to infer, specially in the reset logic. This is causing your problem.

Also, did you simulated first your design before going all the way to Place and Route?, as I said, your reset logic does not seems correct (also we can see it in GTKwave with a proper testbench that is not working):

image

You can use the following code as an example for your implementation, but remember to simulate first before going to configure the FPGA, it is very important: https://pastebin.com/eQSVxSYQ

Note that, for the simulation, I have changed the LEDS vector from 26:23 to 3:0 to have the result faster in the waveform. image

I am able to generate the .asc (or .txt file, whatever suits better for you), with the new synthesized code. Please, try again and do let me know your results.

ghost commented 7 years ago

Bill-3d did you had a chance to try my suggestion?

Bill-3d commented 7 years ago

Hi DH73 (sorry don't know your real name). I lifted the reset code from the Lattice tutorials for the IceStick. The problem with simulation is that the initial conditions don't match the behaviour of the FPGA. The Lattice chips all start up with zeros, whereas the simulator sets them to 'x's. I was trying to test out RS232 comms via some of the BlackIce I/O pins, and found that didn't work without a reset, added the reset code and hit the bug. I then removed almost all of the actual program, just leaving the reset code and monitor LEDs and the problem remained. I have found an alternative bit of code that does seem to work with the basic test. I coalesced the bigcount and resetcount into a single register, assigned reset to !reset_N (which would start at zero) and set it to one when the top bit of the counter turns on. That now compiles OK and seems to work. I now have to add back the rest of the code. I will test it out by simulation, but the project has been sidelined for a week or two. Hope to get back to it soon! My gut feeling was that something was getting confused by the existence of the two counters, but as I haven't a clue how Yosys/Archne handle things I'm probably way off beam.

Thanks

Bill

mtve commented 7 years ago

sorry for intervening, but this assertion really feels like a bug, maybe in yosys.

i've managed to reduce example above to following code:

// change "3" to "2" (two times) to avoid assertion
module test(input clk, output led);
reg[3:0] a;
assign led = a[0];
always @(posedge clk)
    if (a[3]) a <= a + 1;
endmodule

hope this helps.

Bill-3d commented 7 years ago

Hi DH73 (sorry don't know your real name). I lifted the reset code from the Lattice tutorials for the IceStick. The problem with simulation is that the initial conditions don't match the behaviour of the FPGA. The Lattice chips all start up with zeros, whereas the simulator sets them to 'x's. I was trying to test out RS232 comms via some of the BlackIce I/O pins, and found that didn't work without a reset, added the reset code and hit the bug. I then removed almost all of the actual program, just leaving the reset code and monitor LEDs and the problem remained. I have found an alternative bit of code that does seem to work with the basic test. I coalesced the bigcount and resetcount into a single register, assigned reset to !reset_N (which would start at zero) and set it to one when the top bit of the counter turns on. That now compiles OK and seems to work. I now have to add back the rest of the code. I will test it out by simulation, but the project has been sidelined for a week or two. Hope to get back to it soon! My gut feeling was that something was getting confused by the existence of the two counters, but as I haven't a clue how Yosys/Archne handle things I'm probably way off beam.

Thanks

Bill

Bill-3d commented 7 years ago

Having found a work-around, I have now moved on to try to run my real program, which is quite substantial. Sadly, it also fails with this same error. I tried some minor changes, similar to that suggested by mtve above, but it still fails with this same error. Bill

mtve commented 7 years ago

Hi Bill-3d!

I've found that correct verilog code should work, so in my understanding this assertion happens only on broken code. Try to test your design with iverilog and gtkwave to be sure there is no unexpected X'es like dh73 suggested above.

Bill-3d commented 7 years ago

OK, I've struggled through iverilog and gtkwave and made a few minor tweaks, but nothing that I would consider to be broken. It seem to me that if the assertion in Arachne fails, then it's getting bad code from Yosys. This may well originate in something bad about my code, but Yosys should pick that up. It should never generate invalid output. I can only assume its a bug in Yosys.

ZipCPU commented 7 years ago

Hello, all. The following describes a very unsatisfactory work-around, but workaround it is.

The current code in the repo, somewhere around route.cc line 643 or so, prints out the name of the net that is causing the error. The .blif file is a text file. Thus, if you don't recognize the name, you can search the .blif file for the LUTs that use this net, and so find the "fault" in your code.

For me, the code that caused this problem was: always @(posedge i_clk) if ((a)||(b)) counter <= 0; else { stb, counter } <= counter + 1'b1;

The problem in my code was that the "stb" didn't have a defined value to be given it upon reset. To make matters worse, the "reset" signal ((a)||(b)) was given a nearly incomprehensible name ($abc$14968$n158), and the .blif file helped me associated the name with the logic around it.

Dan

cliffordwolf commented 7 years ago

Here is the smallest example I was able to create for this issue:

.model test
.inputs clk a b
.outputs y
.names $false
.gate SB_CARRY CI=b CO=n2 I0=$false I1=a
.gate SB_LUT4 I0=$false I1=$false I2=n5 I3=n2 O=n1
.param LUT_INIT 0110100110010110
.gate SB_CARRY CI=n2 CO=n3 I0=$false I1=n5
.gate SB_LUT4 I0=$false I1=$false I2=y I3=n3 O=n4
.param LUT_INIT 0110100110010110
.gate SB_DFFE C=clk D=n1 E=y Q=n5
.gate SB_DFF C=clk D=n4 Q=y
.end

Schematic generated by yosys -p 'read_verilog -lib +/ice40/cells_sim.v;; show' test.blif:

image

The problem is the following: There are two FFs in this design (the last two .gate statements). Because of the CARRY logic we are strongly suggesting to arachne-pnr to place them both in the same logic tile.

But an iCE40 logic tile (8 logic cells) has only one clock enable (CEN) pin, so all FFs mapped to the logic tile must share the same CEN signal. The SB_DFF is constantly enabled (CEN=1) and the SB_DFFE cell is not (CEN=y). So they can not be placed in the same logic tile.

When the router tries to route the signal driving the CEN pin this conflict is detected by the assert and the program terminates.

Clearly this is a bug in arachne-pnr: It should not have placed both FFs in the same tile. There are two possible ways to fix this: (1) don't accept something like that as a legal placement in the first place, or (2) legalize the placement by detecting such cases and unpacking the offending FF from it's logic cell an placing it in a nearby free logic cell (using a route-through LUT) in a tile that is compatible with the CEN requirements of the FF.