-
Notifications
You must be signed in to change notification settings - Fork 582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Real Verilog tables from Chisel Vecs #983
Comments
An update on the issue: with Chisel 3.4 the generated code is a bit better than 3.2, but still a priority MUX. This led to the following regression from updating the Patmos processor from Chisel 2 to 3, when synthesizing to an (old) FPGA Chisel 2: fmax 80 MHz, fetch stage 740 LCs Chisel 2 generated Verilog can be put in on-chip memory, not the Chisel 3 code. Chisel 2 Verilog is:
Chisel 3 Verilog is:
Cheers, |
With Chisel The one bug that still needs to be fixed is that the In general I would prefer to switch to the read-only Mem implementation for ROMs since that one is super easy to recognize in the backend and we could easily adjust the code emission if we find a way that is superior to doing a Verilog array with an |
Thanks for your proposal. But it looks a bit like a workaround to a Verilog emission issue. Using a plain Vec for a logic table is more elegant than the preloading of a ROM. And the same issue pops up when using a |
I think using a Memory to model a ReadOnlyMemory makes a lot of sense. In general Chisel could have gone the route of trying to infer memories from Vec instead of adding an explicit Mem construct but didn't. I do agree that your ALU should not blow up. We could probably write a compiler pass that detects switch statements and tries to emit them as such. |
On 15 Jan, 2021, at 0:07, Kevin Laeufer ***@***.***> wrote:
Using a plain Vec for a logic table is more elegant than the preloading of a ROM.
I think using a Memory to model a ReadOnlyMemory makes a lot of sense. In general Chisel could have gone the route of trying to infer memories from Vec instead of adding an explicit Mem construct.
I do agree that your ALU should not blow up. We could probably write a compiler pass that detects switch statements and tries to emit them as such.
That would be very good.
Yes, I remember an older proposal to use an array for a Vec of regs to automatically be able to infer a memory. However, this might be similar to inferring memory from stylish written VHDL and Verilog. A bit brittle, at least.
I agree on having a dedicated ROM component is good. However, still the generated code for a Vec should be non-priority MUXes. Vec and (combinational) switch are not so different.
Chers,
Martin
|
Tables in Chisel are Vecs, but the Verilog output generates a priority MUX. This leads to the fact that Chisel generated tables result in LUT logic in an FPGA although they can be implemented in on-chip memory. This needs more resources and has a lower maximum frequency.
In my tutorials on Chisel I usually mention it as a big advantage that we can use Scala to generate logic tables, e.g., for microcode for a state machine, code for a processor, or a lookup table for functions. All those large tables should end up in a on-chip RAM in an FPGA.
Therefore, I request that tables (Vecs) generate a case statement that is understood by FPGA synthesize tools to generate on-chip ROM.
I've put up an example of a 1 KB table to illustrate the case:
git clone https://github.com/schoeberl/chisel-playground.git
cd chisel-playground
make table-verilog
and run the synthesize with Intel Quartus (from quartus/issue-table.qpf) for a real Verilog table.
Results:
10 logic elements and 8192 memory bits, fmax 250 MHz (restricted to this)
Generate the Chisel based table with:
make table-chisel
and synthesize:
870 logic elements, no memory bits, fmax 191 MHz
As long as (Quartus) synthesize tools cannot extract a simple lookup table from priority MUX code, I strongly propose that Chisel/FIRRTL shall spill out a case based table for a Vec.
Cheers,
Martin
The text was updated successfully, but these errors were encountered: