chipsalliance / chipsalliance/chisel

Remove Type Restriction on Mem Write

Open
#3,444 0 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.