SRAM.masked broken when the mask is a constant of all 1s

[elipsitz]

Type of issue: Bug Report

Please provide the steps to reproduce the problem:

I'm instantiating an SRAM with 2 read-write ports. I need byte-masking on one of the ports, but not the other:

val mem = SRAM.masked(16 * 1024, Vec(4, UInt(8.W)), numReadPorts = 0, numWritePorts = 0, numReadwritePorts = 2)
val memHostPort = mem.readwritePorts(0)
val memDevicePort = mem.readwritePorts(1)

memDevicePort is linked up with another submodule that uses the byte-mask. memHostPort connects to the Module's IO ports.

Then, I have this, to set the writeMask to 1111 for this port:

val writeMask = Wire(Vec(4, Bool()))
writeMask := VecInit(true.B, true.B, true.B, true.B)
memHostPort.mask.get := writeMask

What is the current behavior?

When producing SystemVerilog output, Chisel/CIRCT ends up producing a module mem_mem_sram_16384x32 for the SRAM, and in my parent Module, passes it (SystemVerilog)

mem_mem_sram_16384x32 mem_mem_sram_ext (
  .RW0_wmask ({_GEN_3, _GEN_4, _GEN_5, _GEN_6}),
  ...
  .RW1_wmask (4'h1)
);

which is, of course, the wrong mask for port 1. The mask for port 0 is correct. This is generated with:

ChiselStage.emitSystemVerilogFile(
    new MyModule(),
    argRest.toArray,
    firtoolOpts = Array(
      "--preserve-aggregate=1d-vec",
    )
)

without --preserve-aggregate=1d-vec, the output is even worse. In the mem_mem_sram_16384x32 module definition, RW0_wmask and RW1_wmask both disappear! So now, there's no masking at all.

What is the expected behavior?

.RW1_wmask (4'hF), to pass all 1s.

Please tell us about your environment:

Version: 7.9.0
OS: macOS Sequoia 15.7.1 (also verified on an Ubuntu 20.04 system).

Other Information
I was previously using Version 6.6.0, which did not have this problem.

I have found a workaround. Wrap the writeMask with dontTouch:

val writeMask = dontTouch(Wire(Vec(4, Bool())))
writeMask := VecInit(true.B, true.B, true.B, true.B)

This produces:

wire [3:0]      mem_writeMask = '{1'h1, 1'h1, 1'h1, 1'h1};

mem_mem_sram_16384x32 mem_mem_sram_ext (
  .RW0_wmask ({_GEN_3, _GEN_4, _GEN_5, _GEN_6}),
  ...
  .RW1_wmask (mem_writeMask)
);

and without --preserve-aggregate=1d-vec, the masks are still there and correct.

    .RW0_wmask
      ({mem_mem_sram_RW1_wmask_3,
        mem_mem_sram_RW1_wmask_2,
        mem_mem_sram_RW1_wmask_1,
        mem_mem_sram_RW1_wmask_0}),
    ...
    .RW1_wmask ({mem_writeMask_3, mem_writeMask_2, mem_writeMask_1, mem_writeMask_0})

---

I'll also note that, weirdly enough, other constant values (other than b1111) work. e.g.:

    val writeMask = Wire(Vec(4, Bool()))
    writeMask := VecInit(true.B, true.B, true.B, false.B)

generates:

.RW1_wmask (4'h7)

and every other combination I've tried also works, except for 1111.

[elipsitz]

Here's a self-contained scala-cli script to reproduce the issue:

//> using repository https://central.sonatype.com/repository/maven-snapshots
//> using scala 2.13.18
//> using dep org.chipsalliance::chisel:7.9.0
//> using plugin org.chipsalliance:::chisel-plugin:7.9.0
//> using options -unchecked -deprecation -language:reflectiveCalls -feature -Xcheckinit
//> using options -Xfatal-warnings -Ywarn-dead-code -Ywarn-unused -Ymacro-annotations

import chisel3._
import chisel3.util._
import _root_.circt.stage.ChiselStage

class Foo extends Module {
  val io = IO(new Bundle {
    val enable = Input(Bool())
    val isWrite = Input(Bool())
    val address = Input(UInt(10.W))
    val dataIn = Input(UInt(32.W))
    val dataOut = Output(UInt(32.W))
  })

  val mem = SRAM.masked(1024, Vec(4, UInt(8.W)), numReadPorts = 0, numWritePorts = 0, numReadwritePorts = 2)
  val memHostPort = mem.readwritePorts(0)
  val memDevicePort = mem.readwritePorts(1)

  memHostPort.enable := io.enable
  memHostPort.isWrite := io.isWrite
  memHostPort.address := io.address
  // memHostPort.mask.get := dontTouch(VecInit("b1111".U.asBools))
  memHostPort.mask.get := VecInit("b1111".U.asBools)
  memHostPort.writeData := io.dataIn.asTypeOf(memHostPort.writeData)
  io.dataOut := memHostPort.readData.asUInt

  // Do some writes.
  val counter = RegInit(0.U(14.W))
  counter := counter + 1.U
  memDevicePort.enable := true.B
  memDevicePort.isWrite := true.B
  memDevicePort.address := counter(13, 4)
  memDevicePort.mask.get := VecInit(counter(3, 0).asBools)
  memDevicePort.writeData := VecInit(0xAA.U, 0xBB.U, 0xCC.U, 0xDD.U)
}

object Main extends App {
  println(
    ChiselStage.emitSystemVerilog(
      new Foo,
      firtoolOpts = Array(
        "--preserve-aggregate=1d-vec",
      )
    )
  )
}

which produces:

  mem_sram_1024x32 mem_sram_ext (   
    .RW0_addr  (io_address),
    .RW0_en    (io_enable),
    .RW0_clk   (clock),
    .RW0_wmode (io_isWrite),
    .RW0_wdata (_GEN),  
    .RW0_rdata (io_dataOut),
    .RW0_wmask (4'h1),  
    .W0_addr   (counter[13:4]), 
    .W0_en     (1'h1),  
    .W0_clk    (clock),
    .W0_data   (32'hDDCCBBAA),  
    .W0_mask   (mem_sram_RW1_wmask) 
  );    

Change the memHostPort.mask.get := ... line to see the workaround or not.

Comment out the --preserve-aggregate=1d-vec to observe both incorrect mask behaviors.

[elipsitz]

I think this is a Chisel issue, not a CIRCT issue. With ChiselStage.emitFIRRTLDialect I get:

%c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> {name = "mem_sram_RW1_en"}
...
%5 = firrtl.subfield %mem_sram_RW0[wmask] : !firrtl.bundle<addr: uint<10>, en: uint<1>, clk: clock, rdata flip: uint<32>, wmode: uint<1>, wdata: uint<32>, wmask: uint<1>>
      firrtl.matchingconnect %5, %c1_ui1 : !firrtl.uint<1>

So %c1_ui1 looks like it's explicitly wrong, passing the 1-bit wide constant with value 1.

If I change VecInit("b1111".U.asBools) to VecInit("b1110".U.asBools), for example, and recompile, I get:

%c14_ui4 = firrtl.constant 14 : !firrtl.uint<4>
...
%12 = firrtl.subfield %mem_sram_RW0[wmask] : !firrtl.bundle<addr: uint<10>, en: uint<1>, clk: clock, rdata flip: uint<32>, wmode: uint<1>, wdata: uint<32>, wmask: uint<4>>
      firrtl.matchingconnect %12, %c14_ui4 : !firrtl.uint<4>