Feedback from China Users in CCC

[sequencer]

These days, I had a long discussion with XiangShan team(they are doing great jobs!!!) and Chinese Chisel community team:
Here are something I noticed:

1. Users complain about the Multiplexer chaining will introduce bad performance, they are asking for parallel selection circuits. Here are my answers:

• A FIRRTL linting transform should be provided, to detect the depth of Mux, and warn user they are making crazy circuit.
• Switch to decoder for PLA, using which to generate parallel Multiplexer circuit. BTW, since we have merged decoder, I think we should consider switching MuxLookup-like API to decoder implementation, which can provide better PPA to users. And we may need to deprecate switch-is API to stop misleading users.
• If some synthesis tools are synthesis switch-case to transmission gate, Chisel should consider implementing a switch-case IR, and use pragma to ask Verilog synthesis to that, since Chisel is not able to represent this circuit purely. I think we need have some benchmark to synthesis tools, including Yosys, Genus, Design Compiler, test a stdcell with a transmission gate to check the behavior of generated Verilog.
  Responds from @jackkoenig on dev-meeting:

Infer and emit from mux structure (easy, but perhaps brittle)

• [schuyler] Schuyler got like 80% of the way to implementing this via branch: https://github.com/chipsalliance/firrtl/tree/case
• [schuyler]Add FIRRTL IR nodes for this and Chisel emit them
• [schuyler] FIRRTL IR is what it is, using CIRCT we could emit a different IR directly
• [jiuyang] switch-case is basically a decoder
  [jack] this is stealing optimizations opportunities from synthesis tools.
• [jack] In my opinion, we should do (a), max benefit with minimal work

2. Naming is much better than 3.3! KUDO @jackkoenig. But there are still some anonymous nodes. And they showed me some Verilog generated by BlueSpec Compiler, I guess those BSC are using a circuit graph to name anonymous nodes, I think it might be useful to implement an optional InferNameTransform to get anonymous node name from nearest named node.

3. Some HuaWei employees complains about chisel3.utils has a poor PPA, I ask them which one, but they refuse to tell us, and said they had forked Chisel internally, so I directly ignore them. But I think we should consider reviewing circuit generated by chisel3.util, and find will there be a better circuit that chisel can generate? If they are really doing something bad, I think we should try to improve it, rather than let user complaining them.

4. Debug is a painful issue, and Chinese users are doing crazy dirty jobs to resolve it. I think we need to consider speeding up sorting the Verilator API from VerilatorEmitter firrtl#2249 to make chisel be able to have a general API for annotating circuit to mark_debug and providing a general DPI/VPI interfaces.

[schoeberl]

+1 on switch IR for Chisel switch and Vec. -1 on deprecating Chisel switch. There is an open issue on this: #983

[sequencer]

when discussing switch IR, I have a question, what circuit do we need?
basically it should be a big TruthTable to lookup, which means decode should handle it.
If propagate switch IR down to Verilog, means we need synthesis tool helping us decode this, should this be a desired behavior? different synthesis tool may generate different circuit. If using decoder, it will be a determined circuit: pla.
However, if synthesis tools are really crazy to synthesis Mux to transmission gate and or them on a wire. OK, switch-case should definitely being added since that's a circuit which chisel is not able to handle.

[schoeberl]

I would like that a read-only Vec and a Chisel switch`` statements translate to a Verilog case``` statement. In that case, FPGA synthesize tools can translate it to tables placed into on-chip memory. Furthermore, even when it is not using an on-chip memory, but LUTs, it consumes still less logic than the current version of the chain of priority MUXs.

So yes, both constructs may be translated to a truth table. When a Chisel switch statement contains logic, it may still be possible to emit a Verilog case statement.

[seldridge]

For tracking, the open CIRCT issue about case statement emission is: llvm/circt#675. tl;dr: raise regular mux structure to casez and leave any further optimization for synthesis.

The SystemVerilog dialect already has support for casez operations, so it should be pretty straightforward to write a "good enough" raising pass that will catch the extremely regular mux structure that chisel3.utils mux stuff generates. We should also be able to catch regular uses of when and switch.

[sequencer]

However there is another question: BitPat is often being used in switch-case statements for better optimization. If there are ? in the selection?

[seldridge]

Lowering from BitPat wildcard ? to casez's wildcard would be awesome. I'm not sure exactly how to communicate that from Chisel.

It's probably fine to hill climb to better and better solutions. We can start with just heuristic raising to case statements. Then heuristic raising to wildcards. Finally seeing about integration with BitPat via annotations, IR additions, or directly writing MLIR.

[sequencer]

Lowering from BitPat wildcard ? to casez's wildcard would be awesome. I'm not sure exactly how to communicate that from Chisel.

That's what I thought about! And that's the reason I think it will be tricky for SFC to detect BitPat pattern with https://github.com/chipsalliance/firrtl/tree/case
However I don't know whether Verilog support wire === 1?0? or not? BitPat related operators should also have its own pass?

And another thing is I believe we need a benchmark to compare Verilog switch-case vs chisel decoder(espresso/qmc) in some different synthesis tools.