tried build with latest yosys(0.26)/nextpnr(0.5)- results in a design with 25x the LUTs of an ice40?

[dirkenstein]

Ouput was this? Any idea what's going on here?

Info: Device utilisation: Info: ICESTORM_LC: 195891/ 7680 2550% Info: ICESTORM_RAM: 8/ 32 25% Info: SB_IO: 51/ 256 19% Info: SB_GB: 8/ 8 100% Info: ICESTORM_PLL: 2/ 2 100% Info: SB_WARMBOOT: 0/ 1 0%

Info: Placed 57 cells based on constraints. ERROR: Unable to place cell 'vexrv_inst.ram[988]_SB_DFFE_Q_7_E_SB_DFFE_E_2_DFFLC', no BELs remaining to implement cell type 'ICESTORM_LC' 0 warnings, 1 error

[dirkenstein]

It's a problem with yosys not recognizing the ram in vexriscv_wrapper.v as BRAM and trying ro implement it a DFFs. Still breaks in latest yosys but there is a bug reported in relation to it.

[zeldin]

Yeah, when the number of ICESTORM_LC increases explosively for no apparent reason, that is usually what has happened. :smile:

This is what I get with yosys 0.17 and nextpnr 0.3:

Info: Device utilisation:
Info:            ICESTORM_LC:  4546/ 7680    59%
Info:           ICESTORM_RAM:    32/   32   100%
Info:                  SB_IO:    51/  256    19%
Info:                  SB_GB:     8/    8   100%
Info:           ICESTORM_PLL:     2/    2   100%
Info:            SB_WARMBOOT:     0/    1     0%

Info: Placed 57 cells based on constraints.

Note that the ICESTORM_RAM utilisation is expected to be 100%. Could you please point me towards the yosys bug report you mentioned? Thanks.

[dirkenstein]

I think it's this:

https://github.com/YosysHQ/yosys/issues/3627

But the fix (-no-rw-check on synth_ice40) doesn't resolve your issue. I've tried it on the HEAD release, too.

[dirkenstein]

Or possibly this:

https://github.com/YosysHQ/yosys/issues/3370

[dirkenstein]

Is it actually something todo with this? Yosys being formal about BRAM having a clocked output?

https://www.reddit.com/r/yosys/comments/adlsv4/inferring_a_single_cycle_2w1r_register_file_in/ http://zipcpu.com/formal/2018/07/21/zipcpu-icoboard.html

[zeldin]

Well, the ram array does have a clocked output -- the read data is always assigned to ram_read on a clock edge. So it's already specifically written to support the BRAMs of ICE40.

[dirkenstein]

It's definitely written to only use registered output, however,here is the wacky thing:

The following code sythesizes correctly:(though i'm not sure it's correct. Verilog hurts my head.)

   wire [$clog2(mem_size*256):0] ram_read_addr;

   always @(posedge clk) begin
     if (prog_mode) begin
        ram_read_addr <= prog_addr >> 1;
     end else if (!rst_out) begin
        if (dBus_cmd_valid && !dBus_cmd_payload_wr && dbus_ram_access)
          ram_read_addr <= dBus_cmd_payload_address[15:2];
        else if (ibus_stalled)
          ram_read_addr <= ibus_stall_pc;
        else if (iBus_cmd_valid)
          ram_read_addr <= iBus_cmd_payload_pc[15:2];
     end
     ram_read <= ram[ram_read_addr];
   end

This doesn't synthesize bram either:

  always @(posedge clk)
     if (prog_mode) begin
        ram_read <= ram[prog_addr >> 1];
     end else if (!rst_out) begin
        if (dBus_cmd_valid && !dBus_cmd_payload_wr && dbus_ram_access)
          ram_read <= ram[dBus_cmd_payload_address[15:2]];
        else if (ibus_stalled)
          ram_read <= ram[ibus_stall_pc];
        else if (iBus_cmd_valid)
          ram_read <= ram[iBus_cmd_payload_pc[15:2]];
        else
          ram_read <= ram[0];
     end else
       ram_read <= ram[0];

looks like it doesn't like conditional output register assignments even if every possble condition is covered.

[zeldin]

That can probably be considered a bug, since it does work in 0.17. I'll see if I can construct a minimal reproduction recipe.

[zeldin]

(BTW, your alternate implementation will not work since it introduces an extra cycle of latency for reads. But it could probably be made to work if the assignments of ram_read_addr are moved to an always @(*) block instead.)

[dirkenstein]

Hmm - with always @(*) it fails with:

ERROR: timing analysis failed due to presence of combinatorial loops, incomplete specification of timing ports, etc.

[zeldin]

You need to put only the assignments of ram_read_addr in the always @(*), not anything else (like the assignment of ram_read). Anyway, I have a stand-alone reproduction recipe now, so I'll create a yosys ticket once I minimized it a little more.

[zeldin]

@dirkenstein I took the liberty of tacking on my reproduction recipe to YosysHQ/yosys#3679. Also, I pushed a workaround so it should be possible to build iceGDROM with yosys 0.26 now.