Hard to obtain needed context to build IR for referencing results of FirrtlMemory

[SpriteOvO]

I'm trying to add support for #3955 in panama converter, and it's working in progress under binder-fir-mem-binding branch (code is a bit mess at the moment). We create IR firrtl.memory for FirrtlMemory, the IR returns multiple MlirValue results based on the number of ports.

So far, I seem to be successfully associating the MlirValue with the target port via SramTarget. For referencing normal bundle fields, we compute the field index (because firrtl.subfield needs index) via chisel.data.binding (code). But for referencing fields under ports of FirrtlMemory in subsequent operations, something like Slot(Slot(Node(chisel3.SramTarget),RW0),wmask) is given in the Chisel IR - only the field name is provided in the Slot.

Currently panama converter does not store types itself, but tries its best to use the context given by Chisel internal. Am I missing a clean way to compute their indexes? Reflection might work, but it's not really clean to me. Maybe we should make some changes / improvements to FirrtlMemory?

[jackkoenig]

This is a tricky one. The type of wmask is essentially a shadow type of the data type where every leaf element is a Bool instead of whatever the original data type has. This is pretty difficult to represent in Chisel, so we took a short cut. We left wmask off of the Chisel types representing the memory ports (

chisel/src/main/scala/chisel3/util/SRAM.scala

Line 641 in 03ef61f

private[chisel3] final class FirrtlMemoryWriter[T <: Data](writePort: MemoryWritePort[T]) extends Bundle {

) and instead just manually construct the Chisel IR for accessing the wmask:

chisel/src/main/scala/chisel3/util/SRAM.scala

Line 564 in 03ef61f

private def assignMask(

.

It's not super clean, but for computing the field index, wmask should have the same field indices as the regular data type would, but again with the tweak that the actual type of the leaf element field is UInt<1>. I would like to represent the wmask in pure Chisel, and I have a prototype of how to do it, but the code is nasty and I don't have time to clean it up and upstream it right now.

[sequencer]

I’m wondering how about going to another path to intmodule version of SRAM? w/o using firrtl.mem as intermediate representation? We also have some problem on the memory for the mbist which cannot be represented by firrtl ir.

[SpriteOvO]

@jackkoenig I apologize for the late update.

Let's say a val mem = SRAM(1024, UInt(8.W), 1, 1, 1), it will auto generated a

mem mem_sram :
  data-type => UInt<8>
  depth => 1024
  read-latency => 1
  write-latency => 1
  reader => R0
  writer => W0
  readwriter => RW0
  read-under-write => undefined

and auto connect them to a mem wire

connect mem_sram.R0.addr, mem.readPorts[0].address
connect mem_sram.R0.clk, clock
connect mem.readPorts[0].data, mem_sram.R0.data
connect mem_sram.R0.en, mem.readPorts[0].enable
connect mem_sram.W0.addr, mem.writePorts[0].address
connect mem_sram.W0.clk, clock
connect mem_sram.W0.data, mem.writePorts[0].data
connect mem_sram.W0.en, mem.writePorts[0].enable
connect mem_sram.W0.mask, UInt<1>(0h1)
connect mem_sram.RW0.addr, mem.readwritePorts[0].address
connect mem_sram.RW0.clk, clock
connect mem_sram.RW0.en, mem.readwritePorts[0].enable
connect mem.readwritePorts[0].readData, mem_sram.RW0.rdata
connect mem_sram.RW0.wdata, mem.readwritePorts[0].writeData
connect mem_sram.RW0.wmode, mem.readwritePorts[0].isWrite
connect mem_sram.RW0.wmask, UInt<1>(0h1)

The concrete value of loc: Arg for these connect IRs looks like Slot(Slot(Node(chisel3.SramTarget),RW0),wmask), so the only known information here is a SramTarget, a mem port name string ("RW0"), and a port field name string ("wmask").

FirrtlMemory contains 3 names, respectively for R, W, and RW

chisel/core/src/main/scala/chisel3/internal/firrtl/IR.scala

Lines 291 to 299 in d4dd11c

	case class FirrtlMemory(
	sourceInfo: SourceInfo,
	id: HasId,
	t: Data,
	size: BigInt,
	readPortNames: Seq[String],
	writePortNames: Seq[String],
	readwritePortNames: Seq[String])
	extends Definition

firrtl.mem returns multiple MlirValue in order of the parameter portNames we passed. portNames is an array which should not care about the order, as long as the order of the given return types matches the names.

So the first thing is that we need a way to determine which referenced mem port (e.g. R0, R1, W0, or RW0) corresponds to which returned MlirValue.

Then, we need to know which bundle the field (e.g addr, wmask, or mask) under the mem port belongs to, so we can calculate the field index. Like the way we calculate for a normal field

chisel/panamaconverter/src/PanamaCIRCTConverter.scala

Lines 378 to 381 in d4dd11c

	case record: Record =>
	val index = record.elements.size - record.elements.values.iterator.indexOf(data) - 1
	assert(index >= 0, s"can't find field '$data'")
	Reference.SubField(index, tpe)

---

These are the difficulties I can imagine at the moment and I'm trying to explain it clearly, but if you have any further questions please feel free to ask me.