RFE: Make switch statement optimized for one-hot encoded arguments

[tomverbeure]

Current code:

        object PcState extends SpinalEnum(binaryOneHot) {
            val Idle           = newElement()
            val WaitReqReady   = newElement()
            val WaitRsp        = newElement()
            val WaitJumpDone   = newElement()
            val WaitStallDone  = newElement()
        }
...
    val instr_final = (pc.cur_state === pc.PcState.WaitStallDone) ? instr_r     | instr
...
        switch(cur_state){
            is(PcState.Idle){
...

This generates the following verilog:

`define PcState_defaultEncoding_type [4:0]
`define PcState_defaultEncoding_Idle 5'b00001
`define PcState_defaultEncoding_WaitReqReady 5'b00010
`define PcState_defaultEncoding_WaitRsp 5'b00100
`define PcState_defaultEncoding_WaitJumpDone 5'b01000
`define PcState_defaultEncoding_WaitStallDone 5'b10000
...
// GOOD:
  assign instr_final = (((pc_cur_state & `PcState_defaultEncoding_WaitStallDone) != 5'b00000) ? instr_r : instr);
// BAD
    case(pc_cur_state)
      `PcState_defaultEncoding_Idle : begin

The regular compare operation is optimized for one-hot encoding, but the switch statement compares the full vector.
Ideally, it should generate something like this:

(* parallel case *) 
case(1) // synopsys parallel_case
    ((pc_state & `PcState_defaultEncoding_Idle) != 5'b00000): begin
...

The (* parallel case *) is supported by some, but not all, synthesis tools. The same is true for // synopsys parallel_case. So you need both. It's seriously annoying.

parallel_case usage is usually highly discouraged, but one-hot encoded case statements are an exception. From the paper:

Guideline: only use full_case parallel_case to optimize onehot FSM designs.

[typingArtist]

I’d suggest to enter the code like the following. It should be functionally equivalent but points out better the intent.

(* parallel case *)
case(1) // synopsys parallel_case
    ((pc_state & `PcState_defautEncoding_Idle) == `PcState_defaultEncoding_Idle): begin

Also this would allow the code to be used for other *hot encodings as well, e.g. two-hot.

However, the parallel_case tags should only be applied if really all cases have been decoded. Is SpinalHDL already capable of detecting this property in a water-proof way?

[Dolu1990]

@typingArtist
So you have access to the whole AST durring the Verilog generation.
There is where the switch cases statements is emited :

SpinalHDL/core/src/main/scala/spinal/core/internals/ComponentEmitterVerilog.scala

Line 540 in f238a44

case switchStatement : SwitchStatement =>

The is isPure is calculated to be true if only literals are used in the SpinalHDL 'is' statements, as Verilog doesn't support case statements with non literal cases.

So the generation of switch statement with only literal stuff is there

SpinalHDL/core/src/main/scala/spinal/core/internals/ComponentEmitterVerilog.scala

Line 565 in f238a44

b ++= s"${tab}case(${emitExpression(switchStatement.value)})\n"

@typingArtist and @tomverbeure
What happend if you throw some

(* parallel case *)
case(1) // synopsys parallel_case
    ((pc_state & `PcState_defautEncoding_Idle) == `PcState_defaultEncoding_Idle): begin

code into non synposys / parrallel cases capable tools ? is that still legal verilog ?

[tomverbeure]

However, the parallel_case tags should only be applied if really all cases have been decoded.

I believe that's only true for full_case, but not for parallel_case. My rule of thumb is to never ever use full_case. It's too dangerous. For parallel_case, the simply rule of them is the one listed above: if you know for sure that the input is one-hot, then you're good, even if you don't cover all the options.

code into non synposys / parrallel cases capable tools ? is that still legal verilog ?

Tools that don't implement the parallel case will implement a priority encoder instead of a flat encoder. But it will functionally still be totally fine. It's legal Verilog.

Tom

[tomverbeure]

@typingArtist
Your patch is great! As expected, the clock speed of my design went up, and number of gates went down.

I suggest 2 fixed:

• // synopsys parallel_case -> // synthesis parallel_case

  I'm old school, but even if some tools still don't support (* parallel_case *), they all support synthesis instead of synopsys.

• Either keep the (* parallel_case *) attribute, or provide an option to use one or the other or both.

Tom