search

chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
3.9k stars 586 forks source link

ROM construct #266

Open shunshou opened 8 years ago

shunshou commented 8 years ago

In case anyone has extra time on their hand, there should be a version of ROM similar to smem that a FIRRTL pass can be run on to possibly generate a black box -- some vendors have ROM compilers that would be better to use than nests of combinational logic.

aswaterman commented 8 years ago

Yeah. We'll be building some of these soon (using blackboxes in the interim) so we should have a better idea how best to generalize them.

shunshou commented 8 years ago

Haha yeah. Just wanted to raise the issue publicly :P ...

aswaterman commented 8 years ago

Indeed. I just wanted to raise our rationale publicly :)

shunshou commented 7 years ago

@ducky64 @jackkoenig @azidar Any chance a FIRRTL ROM thing might be on the horizon sometime soon? aka before March? I have a tapeout in April that I'd like this for. If not, I'll just blackbox my ROMs in the interim.

ducky64 commented 7 years ago

Resolution from today's meeting: Vec of Literals will be synthesized as a ROM (FIRRTL will emit stylized Verilog). ROM inference in FIRRTL!

I'll leave it to @azidar to provide an estimated timeframe.

shunshou commented 7 years ago

2 things:

In some cases, we would want the same case based ROM we had in Chisel2 (with a default). In other cases, we'd want to black box stuff to use with a ROM compiler. Both should be supported.

On Thu, Jan 26, 2017 at 1:41 PM, Richard Lin notifications@github.com wrote:

Resolution from today's meeting: Vec of Literals will be synthesized as a ROM (FIRRTL will emit stylized Verilog). ROM inference in FIRRTL!

I'll leave it to @azidar https://github.com/azidar to provide an estimated timeframe.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/chisel3/issues/266#issuecomment-275522865, or mute the thread https://github.com/notifications/unsubscribe-auth/AGTTFvjsPeN3czBsYjQJNtkKL669NpsEks5rWRMRgaJpZM4Jrh33 .

aswaterman commented 6 years ago

I've stumbled across this issue again. Yesterday I tried implementing a function on a 16-bit input as a 2^16-entry ROM, which took about 10 minutes to go through Firrtl (largely expanding whens and removing accessors, IIRC). After that, DC segfaulted on the Verilog. I ended up writing a Verilog case statement, which sailed through synthesis in a few minutes.

So, clearly the Verilog emission needs to change, but now I'm also wondering whether inferring ROMs from Vecs is the way to go. Unless ROM inference can happen early in in compilation, we'll have to vastly improve the performance of several Firrtl passes to cope with large Vecs.

shunshou commented 6 years ago

Why not have a separate ROM macro? You might want Chisel to emit either a case statement version, or, if your application benefits from it, you might want to feed your constants into an actual ROM compiler (which I know foundries supply)...

grebe commented 6 years ago

I agree on the ROM macro idea- I think you might actually want a lot of the same things that exist for mems. My black-box based ROM generator based on @shunshou's is a little different b/c Xilinx FPGAs will infer BRAMs if you register the output.

ekiwi commented 4 years ago

Just stumbled over this old issue. If we add support for an initial value to firrtl memories, we could model ROMs as read only memories. @chick Initial values for memories are currently supported through an annotation, correct? What do you think, could this be used to model ROMs? Should they become part of the firrtl ir?

chick commented 4 years ago

@ekiwi I don't understand the use cases enough to give a good response. I think you are referring to the load memory annotation which specifies a file to read and put into memory. I don't think this works for most of the cases being referred to above.

grebe commented 4 years ago

It seems that readmemx is synthesizable for FPGA platforms, oddly. Interestingly, firrtl seems to happily compile memories without read ports (although you still need to specify write-latency). Maybe this is suitable for FPGAs.

I think in ASIC-land you have a separate out-of-band process with a ROM compiler that would work via something like repl-seq-mem, and I imagine that could use the readmemx annotation as well.

ekiwi commented 4 years ago

I think in ASIC-land you have a separate out-of-band process with a ROM compiler that would work via something like repl-seq-mem, and I imagine that could use the readmemx annotation as well.

That was sort of what I was thinking about when suggesting to model ROMs with read only memories. Since we already have a memory compiler, it would just also need to be able to handle ROMs.

However, it seems like ROMs are implemented as Vec: https://github.com/freechipsproject/chisel3/wiki/Memories#rom

Does this mean this issue is actually fixed?

albert-magyar commented 4 years ago

However, it seems like ROMs are implemented as Vec: https://github.com/freechipsproject/chisel3/wiki/Memories#rom

Does this mean this issue is actually fixed?

I believe the original complaint (and @aswaterman's followup) was that large Vecs that implement ROMs are handled poorly, in that they are lowered in a way that leads to long FIRRTL runtimes and simulation-inefficient Verilog.

The overarching goal of "make case statements get emitted in cases where they help" that has been tossed around in several related contexts would be one solution, as would adding a dedicated ROM construct.

shunshou commented 4 years ago

At the minimum, throwing some error/warning when you use Vec for large ROMs is a good idea. I haven't tested this theory, but I feel like Stevo's twiddle factor "red ring of death" (= really congested, rats-nesty, hard-to-close-timing, etc. etc. P&R result) could've been avoided with some more sane generated Verilog (even if not fed directly into some ROM compiler, I expect some more compiler optimization would be possible).

On Fri, Oct 18, 2019 at 10:48 AM Albert Magyar notifications@github.com wrote:

However, it seems like ROMs are implemented as Vec: https://github.com/freechipsproject/chisel3/wiki/Memories#rom

Does this mean this issue is actually fixed?

I believe the original complaint (and @aswaterman https://github.com/aswaterman's followup) was that large Vecs that implement ROMs are handled poorly, in that they are lowered in a way that leads to long FIRRTL runtimes and simulation-inefficient Verilog.

The overarching goal of "make case statements get emitted in cases where they help" that has been tossed around in several related contexts would be one solution, as would adding a dedicated ROM construct.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/266?email_source=notifications&email_token=ABSNGFRJ3M2O3M2BB76KQVDQPHZHFA5CNFSM4CNODX32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBVJ7OA#issuecomment-543858616, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSNGFQ7N45ZC5X2RW77ZHDQPHZHFANCNFSM4CNODX3Q .

kammoh commented 4 years ago

It would be really nice to have a built-in FIRRTL pass that extracts ROMs from designer annotated Vecs, or Vecs larger than a specified depth/width threshold (again set through a chisel leve annotation).