chisel icon indicating copy to clipboard operation
chisel copied to clipboard

loadMemoryFromFile() should place readmemh() inline for better compatibility

Open antonblanchard opened this issue 5 years ago • 18 comments

loadMemoryFromFile() creates a separate file and uses a SV bind statement. A simple example:

import chisel3._
import chisel3.util.experimental.loadMemoryFromFile

class Foo(val bits: Int, val size: Int, filename: String) extends Module {
  val io = IO(new Bundle {
    val nia = Input(UInt(bits.W))
    val insn = Output(UInt(32.W))
  })

  val memory = Mem(size, UInt(32.W))
  io.insn := memory(io.nia >> 2);
  loadMemoryFromFile(memory, filename)
}

object FooObj extends App {
  chisel3.Driver.execute(Array[String](), () => new Foo(32, 1024, "insns.hex"))
}
# cat Foo.Foo.memory.v
module BindsTo_0_Foo(
  input         clock,
  input         reset,
  input  [31:0] io_nia,
  output [31:0] io_insn
);

initial begin
  $readmemh("insns.hex", Foo.memory);
end
                      endmodule

bind Foo BindsTo_0_Foo BindsTo_0_Foo_Inst(.*);

Yosys doesn't like this, and likely there are other tools that don't either. Is there any reason we don't just place it inline?

antonblanchard avatar Jan 03 '20 23:01 antonblanchard

Same here while trying to make a RAM module:

class SinglePortRAM(sizeKB: Int = 1, width: Int = 32, memoryFile: String = "") extends Module {
  val addrWidth = chiselTypeOf((sizeKB * 1024).U)
  val io = IO(new Bundle {
    val addr1 = Input(addrWidth)
    val dataIn1 = Input(UInt(width.W))
    val en1 = Input(Bool())
    val we1 = Input(Bool())
    val dataOut1 = Output(UInt(width.W))
  })
  println(s"Single-port Memory Parameters:")
  println(s"  Size: $sizeKB KB")
  println(s"  Width: $width bit")
  println(s"  Addr Width: " + io.addr1.getWidth + " bit")

  val mem = SyncReadMem(sizeKB * 1024, UInt(width.W))
  if (memoryFile.trim().nonEmpty) {
      loadMemoryFromFile(mem, memoryFile)
  }

  when (io.we1) {
    mem.write(io.addr1, io.dataIn1)
  }
  io.dataOut1 := mem.read(io.addr1, io.en1)
}

If memfile is provided, it gets written to a separate file that is not synthesizable.

carlosedp avatar Feb 18 '21 23:02 carlosedp

We have a MemoryInitAnnotation now which will inline the contents into the generated Verilog. This should be compatible with the memory inference pass in yosys.

For examples on how to use it, see:

  • with an array of BigInt: https://github.com/ekiwi/serv-chisel/blob/main/src/servant/WishBoneRam.scala#L46
  • with a single BigInt: https://github.com/ekiwi/serv-chisel/blob/main/src/serv/Ram.scala#L85

Would that work for you?

ekiwi avatar Feb 18 '21 23:02 ekiwi

Hey Kevin, thanks for your response.

I understand I cannot pass the .hex file I have to be loaded at synthesis by using the annotation, right since it only supports BigInts?

What's the method do do that?

carlosedp avatar Feb 19 '21 00:02 carlosedp

I understand I cannot pass the .hex file I have to be loaded at synthesis by using the annotation, right since it only supports BigInts?

Yeah, so the method that I suggested assumes that you have the data in Scala. You could of course try and write a function that opens a .hex file from Scala and returns the content as a list of BigInt.

What's the method do do that?

There is no separate file involved. You just provide the memory init content in your Chisel generator and the Verilog we produce will contain an initial block that normally works well with yosys. I prefer this way to having a separate file, but I can understand, that it might be a little annoying if you want to change your memory image without re-elaborating your Chisel.

ekiwi avatar Feb 19 '21 00:02 ekiwi

As someone new to HDLs, I found these methods way more complex than just $readmemh(mem, "file") in Verilog... I believe that it should have a somewhat similar syntax...

carlosedp avatar Feb 19 '21 12:02 carlosedp

@carlosedp, could you try to see if the following would address your problem?

import chisel3.experimental.{ChiselAnnotation, annotate}
import firrtl.annotations.{MemoryArrayInitAnnotation, MemoryLoadFileType}

import scala.io.Source

object loadMemoryFromFileInline {
  def apply(mem: MemBase[_], filename: String, hexOrBinary: MemoryLoadFileType.FileType = MemoryLoadFileType.Hex): Unit = {
    val radix = hexOrBinary match {
      case MemoryLoadFileType.Hex => 16
      case MemoryLoadFileType.Binary => 2
    }
    val src = Source.fromFile(filename)
    val lines = src.getLines().toList
    src.close()
    val data = lines.map(_.trim).filter(_.nonEmpty).map(BigInt(_, radix))
    val missingWords = mem.length - data.length
    require(missingWords >= 0, s"$filename contained ${data.length} items, but memory only fits ${mem.length}")
    val padding = Seq.tabulate(missingWords.toInt)(_ => BigInt(0))
    annotate(new ChiselAnnotation {
      override def toFirrtl = MemoryArrayInitAnnotation(mem.toTarget, data ++ padding)
    })
  }
}

You should be able to just replace loadMemoryFromFile with loadMemoryFromFileInline in the example that you posted above.

I know that this isn't 100% what you want, but it would be fairly straight forward to add this helper function from your code base and might be a good stop-gap measure until we come up with a more permanent solution.

ekiwi avatar Feb 19 '21 20:02 ekiwi

Thanks @ekiwi ... just a heads-up is that since the memory gets initialized by directly setting the memory array in Verilog so a 1k Scala module with an 11k .hex became a 282k verilog...

-rw-r--r-- 1 cdepaula staff  11K Feb 19 18:41 hello_world.hex
-rw-r--r-- 1 cdepaula staff 282K Feb 19 18:55 SinglePortRAM.v
-rw-r--r-- 1 cdepaula staff 1.1K Feb 19 18:55 SinglePortRAM.fir
-rw-r--r-- 1 cdepaula staff 147K Feb 19 18:55 SinglePortRAM.anno.json
❯ ll src/main/scala/utils
total 12K
-rw-r--r-- 1 cdepaula staff 1.1K Feb 19 18:53 LoadMemory.scala
-rw-r--r-- 1 cdepaula staff  999 Feb 19 18:55 SingleportRAM.scala

carlosedp avatar Feb 19 '21 21:02 carlosedp

just a heads-up is that since the memory gets initialized by directly setting the memory array in Verilog so a 1k Scala module with an 11k .hex became a 282k verilog...

Thanks for trying it out. I definitely didn't consider that this might blow up significantly in size (20x .. wow).

ekiwi avatar Feb 19 '21 22:02 ekiwi

It's instantiated into a 16KB ram that would be pretty much empty.. but still quite a grow. I still need to test it thru all synthesis... i'll let u know the results.

carlosedp avatar Feb 19 '21 22:02 carlosedp

@antonblanchard check out https://github.com/chipsalliance/firrtl/pull/2107. I've added a new annotation allowing inline readmem in Verilog emiter.

Your sample code would become:

import chisel3._
import chisel3.experimental.{ChiselAnnotation, annotate}
import firrtl.annotations.{MemoryFileInlineAnnotation}


class Foo(val bits: Int, val size: Int, filename: String) extends Module {
  val io = IO(new Bundle {
    val nia = Input(UInt(bits.W))
    val insn = Output(UInt(32.W))
  })

  val memory = Mem(size, UInt(32.W))
  io.insn := memory(io.nia >> 2);
  annotate(new ChiselAnnotation {
    override def toFirrtl = MemoryFileInlineAnnotation(memory.toTarget, filename)
  })
}

object FooObj extends App {
  chisel3.Driver.execute(Array[String](), () => new Foo(32, 1024, "insns.hex"))
}

that would generate the appropriate:

initial begin
  $readmemh("insns.hex", memory);
end

It would allow also configuration for binary files instead of hex, like:

import firrtl.annotations.{MemoryFileInlineAnnotation, MemoryLoadFileType}
...
  annotate(new ChiselAnnotation {
    override def toFirrtl = MemoryFileInlineAnnotation(memory.toTarget, "filename.bin", MemoryLoadFileType.Binary)
  })
...

and output

initial begin
  $readmemb("filename.bin", memory);
end

carlosedp avatar Mar 09 '21 01:03 carlosedp

@carlosedp That looks great! Nice work

antonblanchard avatar Mar 09 '21 10:03 antonblanchard

Now there is also a method in chisel for this:

import chisel3.util.experimental.loadMemoryFromFileInline
...
  annotate(new ChiselAnnotation {
    override def toFirrtl =
      MemorySynthInit
  })

  val mem = SyncReadMem(1024, UInt(width.W))
  loadMemoryFromFileInline(mem, memoryFile)

I believe this could be closed now. Any objections?

carlosedp avatar Apr 05 '21 12:04 carlosedp

I believe this could be closed now. Any objections?

There is still the question of whether we should move this API out of experimental and maybe make the inline version the default.

ekiwi avatar Apr 05 '21 16:04 ekiwi

Ah yes, that would be great!

carlosedp avatar Apr 05 '21 17:04 carlosedp

Any news on this? Should we move on and close or promote to non-experimental API?

carlosedp avatar Nov 23 '21 20:11 carlosedp

Any news on this? Should we move on and close or promote to non-experimental API?

Would you mind making a PR that makes loadMemoryFromFile available in the standard chisel3._ package and changes the behavior to that of loadMemoryFromFileInline?

Then we can discuss the issue on the PR.

ekiwi avatar Nov 23 '21 22:11 ekiwi

Sure, do I keep the previous behavior of loadMemoryFromFile (using external SV file) to another function?

Also should it be in chisel3 or chisel3.util?

carlosedp avatar Nov 24 '21 13:11 carlosedp

Opened #2260 to address this.

carlosedp avatar Nov 24 '21 19:11 carlosedp