chipsalliance / chipsalliance/chisel
Remove Type Restriction on Mem Write
- Dominant language
- Scala
- Stars
- 4.8k
- Forks
- 658
- Avg merge
- 18h 59m
- Merged PRs (30d)
- 14
Description
Memories of bundles don't work for writes even though the FIRRTL compilation of them is fine.
We should just remove the additional type constraint that when using the write operations that the type is a sub-type of a vec.
#### Example
Consider the following:
```scala
//> using scala "2.13.10"
//> using repository "https://s01.oss.sonatype.org/content/repositories/snapshots"
//> using lib "org.chipsalliance::chisel::6.0.0-M2"
//> using plugin "org.chipsalliance:::chisel-plugin::6.0.0-M2"
//> using options "-Ymacro-annotations"
import chisel3._
import circt.stage.ChiselStage
class MyMemoryType extends Bundle {
val data = UInt(64.W)
val pNext = UInt(6.W)
val gNext = UInt(6.W)
val gPrev = UInt(6.W)
}
class MaskedReadWriteSmem extends Module {
val width: Int = 8
val io = IO(new Bundle {
val enable = Input(Bool())
val write = Input(Bool())
val addr = Input(UInt(10.W))
val mask = Input(Vec(4, Bool()))
val dataIn = Input(new MyMemoryType)
val dataOut = Output(new MyMemoryType)
})
// Create a 32-bit wide memory that is byte-masked
val mem = SyncReadMem(1024, new MyMemoryType)
// Write with mask
mem.write(io.addr, io.dataIn, io.mask)
io.dataOut := mem.read(io.addr, io.enable)
}
object Main extends App {
println(
ChiselStage.emitSystemVerilog(
new MaskedWriteSmem,
firtoolOpts = Array("-strip-debug-info", "-disable-all-randomization")
)
)
}
```
If you run this through Chisel you get (`scala-cli Foo.scala`):
```
Compiling project (Scala 2.13.10, JVM)
[error] ./Foo.scala:31:3
[error] Cannot prove that MyMemoryType <:< chisel3.Vec[_].
[error] mem.write(io.addr, io.dataIn, io.mask)
[error] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error compiling project (Scala 2.13.10, JVM)
Compilation failed
```
Chisel is checking that the memory type is a sub-type of a vec.
However, if I manually construct this in FIRRTL with the following:
```
FIRRTL version 3.1.0
circuit Foo :
module Foo :
input r : { addr : UInt<3>,
en : UInt<1>,
clk : Clock,
flip data
: { a : UInt<32>, b : UInt<6>, c : UInt<6>, d : UInt<6> } }
input w : { addr : UInt<3>,
en : UInt<1>,
clk : Clock,
data : { a : UInt<32>, b : UInt<6>, c : UInt<6>, d : UInt<6> },
mask : { a : UInt<1>, b : UInt<1>, c : UInt<1>, d : UInt<1> } }
mem memory :
data-type => { a : UInt<32>, b : UInt<6>, c : UInt<6>, d : UInt<6> }
depth => 8
read-latency => 1
write-latency => 1
reader => r
writer => w
read-under-write => undefined
connect memory.r.addr, r.addr
connect memory.r.en, r.en
connect memory.r.clk, r.clk
connect r.data, memory.r.data
connect memory.w, w
```
I can get the following reasonable Verilog for it:
```verilog
// Generated by CIRCT unknown git version
// VCS coverage exclude_file
module memory_8x50(
input [2:0] R0_addr,
input R0_en,
R0_clk,
input [2:0] W0_addr,
input W0_en,
W0_clk,
input [49:0] W0_data,
input [24:0] W0_mask,
output [49:0] R0_data
);
reg [49:0] Memory[0:7];
reg _GEN;
reg [2:0] _GEN_0;
always @(posedge R0_clk) begin
_GEN <= R0_en;
_GEN_0 <= R0_addr;
end // always @(posedge)
always @(posedge W0_clk) begin
if (W0_en & W0_mask[0])
Memory[W0_addr][32'h0 +: 2] <= W0_data[1:0];
if (W0_en & W0_mask[1])
Memory[W0_addr][32'h2 +: 2] <= W0_data[3:2];
if (W0_en & W0_mask[2])
Memory[W0_addr][32'h4 +: 2] <= W0_data[5:4];
if (W0_en & W0_mask[3])
Memory[W0_addr][32'h6 +: 2] <= W0_data[7:6];
if (W0_en & W0_mask[4])
Memory[W0_addr][32'h8 +: 2] <= W0_data[9:8];
if (W0_en & W0_mask[5])
Memory[W0_addr][32'hA +: 2] <= W0_data[11:10];
if (W0_en & W0_mask[6])
Memory[W0_addr][32'hC +: 2] <= W0_data[13:12];
if (W0_en & W0_mask[7])
Memory[W0_addr][32'hE +: 2] <= W0_data[15:14];
if (W0_en & W0_mask[8])
Memory[W0_addr][32'h10 +: 2] <= W0_data[17:16];
if (W0_en & W0_mask[9])
Memory[W0_addr][32'h12 +: 2] <= W0_data[19:18];
if (W0_en & W0_mask[10])
Memory[W0_addr][32'h14 +: 2] <= W0_data[21:20];
if (W0_en & W0_mask[11])
Memory[W0_addr][32'h16 +: 2] <= W0_data[23:22];
if (W0_en & W0_mask[12])
Memory[W0_addr][32'h18 +: 2] <= W0_data[25:24];
if (W0_en & W0_mask[13])
Memory[W0_addr][32'h1A +: 2] <= W0_data[27:26];
if (W0_en & W0_mask[14])
Memory[W0_addr][32'h1C +: 2] <= W0_data[29:28];
if (W0_en & W0_mask[15])
Memory[W0_addr][32'h1E +: 2] <= W0_data[31:30];
if (W0_en & W0_mask[16])
Memory[W0_addr][32'h20 +: 2] <= W0_data[33:32];
if (W0_en & W0_mask[17])
Memory[W0_addr][32'h22 +: 2] <= W0_data[35:34];
if (W0_en & W0_mask[18])
Memory[W0_addr][32'h24 +: 2] <= W0_data[37:36];
if (W0_en & W0_mask[19])
Memory[W0_addr][32'h26 +: 2] <= W0_data[39:38];
if (W0_en & W0_mask[20])
Memory[W0_addr][32'h28 +: 2] <= W0_data[41:40];
if (W0_en & W0_mask[21])
Memory[W0_addr][32'h2A +: 2] <= W0_data[43:42];
if (W0_en & W0_mask[22])
Memory[W0_addr][32'h2C +: 2] <= W0_data[45:44];
if (W0_en & W0_mask[23])
Memory[W0_addr][32'h2E +: 2] <= W0_data[47:46];
if (W0_en & W0_mask[24])
Memory[W0_addr][32'h30 +: 2] <= W0_data[49:48];
end // always @(posedge)
assign R0_data = _GEN ? Memory[_GEN_0] : 50'bx;
endmodule
module Foo(
input [2:0] r_addr,
input r_en,
r_clk,
input [2:0] w_addr,
input w_en,
w_clk,
input [31:0] w_data_a,
input [5:0] w_data_b,
w_data_c,
w_data_d,
input w_mask_a,
w_mask_b,
w_mask_c,
w_mask_d,
output [31:0] r_data_a,
output [5:0] r_data_b,
r_data_c,
r_data_d
);
wire [49:0] _memory_ext_R0_data;
memory_8x50 memory_ext (
.R0_addr (r_addr),
.R0_en (r_en),
.R0_clk (r_clk),
.W0_addr (w_addr),
.W0_en (w_en),
.W0_clk (w_clk),
.W0_data ({w_data_d, w_data_c, w_data_b, w_data_a}),
.W0_mask ({{3{w_mask_d}}, {3{w_mask_c}}, {3{w_mask_b}}, {16{w_mask_a}}}),
.R0_data (_memory_ext_R0_data)
);
assign r_data_a = _memory_ext_R0_data[31:0];
assign r_data_b = _memory_ext_R0_data[37:32];
assign r_data_c = _memory_ext_R0_data[43:38];
assign r_data_d = _memory_ext_R0_data[49:44];
endmodule
```
Contributor guide
Research direction
Start by locating the SyncReadMem and mem.write implementation where the write-type constraint is enforced. Reproduce the provided Bundle memory example, then verify that masked writes with a Bundle compile successfully while existing vector-write behavior remains valid.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100