schoeberl / chisel-book

Digital Design with Chisel
773 stars 144 forks source link

SyncReadMem example with read enable #25

Closed danielkasza closed 4 years ago

danielkasza commented 4 years ago

Unlike Mem, SyncReadMem has a read method with two arguments:

def read(x: UInt, en: Bool)

I could not find any examples of this method being used in the book. I think "1 KiB of synchronous memory" example should have a read enable input.

class Memory() extends Module {
  val io = IO(new Bundle {
    val rdEna = Input(Bool())
    val rdAddr = Input(UInt(10.W))
    val rdData = Output(UInt(8.W))
    val wrEna = Input(Bool())
    val wrData = Input(UInt(8.W))
    val wrAddr = Input(UInt(10.W))
  })

  val mem = SyncReadMem(1024, UInt(8.W))

  io.rdData := mem.read(io.rdAddr, io.rdEna)

  when(io.wrEna) {
    mem.write(io.wrAddr, io.wrData)
  }
}
schoeberl commented 4 years ago

I am not a big fan of a read enable. What would it be used for? When you apply an address to a (synchronous) SRAM (e.g., and FPGA on-chip memory), it will just read from that address.

danielkasza commented 4 years ago

I am going to close this issue. I opened it because of a misunderstanding of what the enable of SyncReadMem does. I will describe the misunderstanding below just in case someone else will find this issue in the future.


I have a use case where I have to look at the same word potentially for multiple cycles while working on a request. I also want this to synthesize to block RAM on an FPGA. So, naturally, I would want to change the read address only when accepting a new request. This can be expressed using Mem this way:

class Foo extends Module {
  val io = IO(new Bundle {
    val en = Input(Bool())
    val addr = Input(UInt(8.W))
    val out = Output(UInt(32.W))
  })

  val mem = Mem(256, UInt(32.W))
  val addrReg = Reg(UInt(8.W))

  when (io.en) {
    addrReg := io.addr
  }

  io.out := mem(addrReg)
}

Here is the generated Verilog (slightly cleaned up):

module Foo(
  input         clock,
  input         reset,
  input         io_en,
  input  [7:0]  io_addr,
  output [31:0] io_out
);
  reg [31:0] mem [0:255]; // @[main.scala 13:16]
  reg [31:0] _RAND_0;
  wire [31:0] mem__T_data; // @[main.scala 13:16]
  wire [7:0] mem__T_addr; // @[main.scala 13:16]
  reg [7:0] addrReg; // @[main.scala 14:20]
  reg [31:0] _RAND_1;
  assign mem__T_addr = addrReg;
  assign mem__T_data = mem[mem__T_addr]; // @[main.scala 13:16]
  assign io_out = mem__T_data; // @[main.scala 20:10]

  always @(posedge clock) begin
    if (io_en) begin
      addrReg <= io_addr;
    end
  end
endmodule

My expectation was that SyncReadMem's read enable also worked this way, so I could write code more like:

class Foo extends Module {
  val io = IO(new Bundle {
    val en = Input(Bool())
    val addr = Input(UInt(8.W))
    val out = Output(UInt(32.W))
  })

  val mem = SyncReadMem(256, UInt(32.W))

  io.out := mem.read(io.addr, io.en)
}

...and when I looked at the generated Verilog, it seemed to confirm my expectations:

module Foo(
  input         clock,
  input         reset,
  input         io_en,
  input  [7:0]  io_addr,
  output [31:0] io_out
);
  reg [31:0] mem [0:255]; // @[main.scala 13:24]
  reg [31:0] _RAND_0;
  wire [31:0] mem__T_3_data; // @[main.scala 13:24]
  wire [7:0] mem__T_3_addr; // @[main.scala 13:24]
  reg  mem__T_3_en_pipe_0;
  reg [31:0] _RAND_1;
  reg [7:0] mem__T_3_addr_pipe_0;
  reg [31:0] _RAND_2;
  assign mem__T_3_addr = mem__T_3_addr_pipe_0;
  assign mem__T_3_data = mem[mem__T_3_addr]; // @[main.scala 13:24]
  assign io_out = mem__T_3_data; // @[main.scala 15:10]

  always @(posedge clock) begin
    mem__T_3_en_pipe_0 <= io_en;
    if (io_en) begin
      mem__T_3_addr_pipe_0 <= io_addr;
    end
  end
endmodule

Unfortunately, this is not the guaranteed behavior. Kevin Laeufer pointed this out on https://gitter.im/freechipsproject/chisel3:

I just looked at the example codes that you posted. Unfortunately they do different things: The 1. one only updates the address when (io.en) is asserted, but always returns a valid entry of the memory. The 2. one only returns a valid memory entry iff in the previous cycle io.en was asserted. When the enable signal of a read port on a SyncReadMem was false in the previous cycle, that means that the value on it's data lines is undefined. This models the behavior of some SRAMs that can go into a power saving mode.

This is also confirmed by the generated FIRRTL code:

circuit Foo :
  module Foo :
    input clock : Clock
    input reset : UInt<1>
    output io : { flip en : UInt<1>, flip addr : UInt<8>, out : UInt<32>}

    mem mem : @[main.scala 13:24]
      data-type => UInt<32>
      depth => 256
      read-latency => 1
      write-latency => 1
      reader => _T_3
      read-under-write => undefined
    mem._T_3.addr is invalid @[main.scala 13:24]
    mem._T_3.clk is invalid @[main.scala 13:24]
    mem._T_3.en <= UInt<1>("h0") @[main.scala 13:24]
    wire _T : UInt<8> @[main.scala 15:21]
    _T is invalid @[main.scala 15:21]
    when io.en : @[main.scala 15:21]
      _T <= io.addr @[main.scala 15:21]
      node _T_1 = or(_T, UInt<8>("h0")) @[main.scala 15:21]
      node _T_2 = bits(_T_1, 7, 0) @[main.scala 15:21]
      mem._T_3.en <= UInt<1>("h1") @[main.scala 15:21]
      mem._T_3.addr <= _T_2 @[main.scala 15:21]
      mem._T_3.clk <= clock @[main.scala 15:21]
    io.out <= mem._T_3.data @[main.scala 15:10]

From the FIRRTL spec:

If the en field is high, then the element value associated with the address in the addr field can be retrieved by reading from the data field after the appropriate read latency. If the en field is low, then the value in the data field, after the appropriate read latency, is undefined. The port is driven by the clock signal in the clk field.

So, while the Verilog code generated today happens to match my expectations, this behavior is not guaranteed. The read enable may be useful for power savings in the future, but there is not much use for it outside that.