ucb-bar / chisel2-deprecated

chisel.eecs.berkeley.edu
388 stars 90 forks source link

Verilog doesn't use`else` so memory not inferred as RAM #709

Open sclukey opened 8 years ago

sclukey commented 8 years ago

Chisel seems to not like using else and else if in Verilog code, and this has caused a problem with my RAM block not being inferred by the Vivado synthesizer because it thinks there are more write ports than there actually are. As a minimal example, this Chisel code

class VectorWriter() extends Module {
    val io = new Bundle {
        val mode = Bool(INPUT)
        val rw_addr = UInt(INPUT, width=8)
        val w_addr = UInt(INPUT, width=8)
        val writeData = UInt(INPUT, width=16)
        val readData = UInt(OUTPUT, width=16)
    }

    val memory = Mem(UInt(width=16), 256)

    when(io.mode) {
        memory(io.rw_addr) := io.writeData
    } .otherwise {
        memory(io.w_addr) := io.writeData
    }

    io.readData := memory(io.rw_addr)
}

results in this Verilog

module VectorWriter(input clk,
    input  io_mode,
    input [7:0] io_rw_addr,
    input [7:0] io_w_addr,
    input [15:0] io_writeData,
    output[15:0] io_readData
);

  wire[15:0] T0;
  reg [15:0] memory [255:0];
  wire[15:0] T1;
  wire T2;
  wire[15:0] T3;

  assign io_readData = T0;
  assign T0 = memory[io_rw_addr];
  assign T2 = io_mode ^ 1'h1;

  always @(posedge clk) begin
    if (T2)
      memory[io_w_addr] <= io_writeData;
    if (io_mode)
      memory[io_rw_addr] <= io_writeData;
  end
endmodule

The memory registers should be implemented as a RAM with one read port and one write port. However, when synthesizing this code the Vivado synthesizer gives this warning saying that this is not possible:

WARNING: [Synth 8-4767] Trying to implement RAM 'memory_reg' in registers. Block RAM or DRAM implementation is not possible; see log for reasons.
Reason is one or more of the following :
    1: RAM has multiple writes via different ports in same process. If RAM inferencing intended, write to one port per process. 
    2: Unable to determine number of words or word size in RAM. 
    3: No valid read/write found for RAM. 
RAM "memory_reg" dissolved into registers

In this case the reason is number 1. Because the writes to the RAM are in independent if statements, the synthesizer thinks that both writes might happen at the same time, so it cannot be a single port RAM.

Now, obviously the if statements are never both going to be true, and if I were hand-writing the Verilog I would write this (just like the Chisel)

    if (io_mode)
      memory[io_rw_addr] <= io_writeData;
    else
      memory[io_w_addr] <= io_writeData;

which is inferred correctly as a RAM block.

The Verilog output seems counterintuitive and in this case problematic, so I think this is an issue.

sbeamer commented 8 years ago

For your example, is the intent to have a RW port and a W port on the same RAM, with the RW port having write precedence over the W port? If so, could you mux the address and data such that they share the same write port?

More generally, it appears the issue is with the current Chisel2, you cannot express precedence for these ports. For Chisel3, there is effort being applied to fix RW ports (https://github.com/ucb-bar/firrtl/issues/147) and multi-write ports (https://github.com/ucb-bar/firrtl/issues/125).

sclukey commented 8 years ago

Well, in my actual usage the intent is to have one read port and one write port. In Xilinx terminology I think what I want is just a Simple Dual-Port RAM. The example is a bit contrived, and I probably should have put a dedicated read address input, but specific to the problem I am having the read port is irrelevant.

The problem is that Vivado thinks there are "multiple writes via different ports in same process", which, in the Chisel code, there are explicitly not. In this simple case yes I could just use a mux on the write address and that works just fine. My actual use case is more complex and a mux would make awkward code. Regardless, either way should work.

My guess is that Chisel seems to think when (mode) { A } .otherwise { B } is equivalent to if (mode) A and if (mode ^ 1) B in Verilog. The Vivado synthesizer seems to disagree, at least in the case of implying RAMs.