chipsalliance / chipsalliance/chisel

Memory with write masks result in confusing layout and readmemh() behaviour

Open
#1,289 2 comments 1 reaction 0 assignees View on GitHub
bug
Dominant language
Scala
Stars
4.8k
Forks
658
Avg merge
18h 59m
Merged PRs (30d)
14

Description

The Chisel documentation says to use vectors for sub word writes. When I add this, my memory goes from an array of 64 bits:

```verilog
reg [63:0] mem [0:1023]; // @[Memory.scala 14:24]
```

To 8 separate arrays of 8 bits:

```verilog
reg [7:0] mem_0 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_1 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_2 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_3 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_4 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_5 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_6 [0:1023]; // @[Memory.scala 33:24]
reg [7:0] mem_7 [0:1023]; // @[Memory.scala 33:24]
```

There are a few issues with this:

- I can't see how this would synthesize correctly on an FPGA. A quick test on yosys shows it using 4x the number of RAM elements.
- This layout is confusing if you are trying to look at the memory directly, eg in simulation
- readmemh() goes from requiring a single file to 8 separate files each containing one byte of each 64 bit word. Very confusing.

```scala
import chisel3._
import chisel3.util.experimental.loadMemoryFromFile

class Memory(val words: Int, val bits: Int, val addrBits: Int, val filename: String) extends Module {
val io = IO(new Bundle {
val dataOut = Output(UInt(bits.W))
val dataIn = Input(UInt(bits.W))
val readAddr = Input(UInt(addrBits.W))
val readEnable = Input(Bool())
val writeAddr = Input(UInt(addrBits.W))
val writeEnable = Input(Bool())
})

val mem = SyncReadMem(words, UInt(bits.W))
loadMemoryFromFile(mem, filename)

when (io.writeEnable) {
mem.write(io.writeAddr, io.dataIn)
}
io.dataOut := mem.read(io.readAddr, io.readEnable)
}

class ByteEnableMemory(val words: Int, val bits: Int, val addrBits: Int, val filename: String) extends Module {
val io = IO(new Bundle {
val dataOut = Output(Vec(bits/8, UInt(8.W)))
val dataIn = Input(Vec(bits/8, UInt(8.W)))
val readAddr = Input(UInt(addrBits.W))
val readEnable = Input(Bool())
val writeAddr = Input(UInt(addrBits.W))
val writeMask = Input(Vec(bits/8, Bool()))
})

val mem = SyncReadMem(words, Vec(bits/8, UInt(8.W)))
loadMemoryFromFile(mem, filename)

mem.write(io.writeAddr, io.dataIn, io.writeMask)
io.dataOut := mem.read(io.readAddr, io.readEnable)
}

object MemoryObj extends App {
chisel3.Driver.execute(Array[String](), () => new Memory(1024, 64, 32, "test.mem"))
chisel3.Driver.execute(Array[String](), () => new ByteEnableMemory(1024, 64, 32, "test.mem"))
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.