[FIRRTL] Improve else-if Verilog Quality

[seldridge]

Consider the following FIRRTL text. This is a sketch of what a designer would write using when/elsewhen/else` Chisel constructs. (This is derived from a classic Chisel example, https://github.com/chipsalliance/chisel3/issues/1198):

circuit Foo :
  module Foo :
    input clock : Clock
    input reset : UInt<1>
    input in : UInt<2>
    output outWhen : UInt<2>

    reg outWhen_tmp : UInt<2>, clock with :
      reset => (reset, UInt<2>("h0"))
    node _outWhen_T = eq(in, UInt<1>("h0"))
    when _outWhen_T :
      outWhen_tmp <= UInt<1>("h0")
    else :
      node _outWhen_T_1 = eq(in, UInt<1>("h1"))
      when _outWhen_T_1 :
        outWhen_tmp <= UInt<1>("h1")
      else :
        node _outWhen_T_2 = eq(in, UInt<2>("h2"))
        when _outWhen_T_2 :
          outWhen_tmp <= UInt<2>("h2")
        else :
          outWhen_tmp <= UInt<2>("h3")
    outWhen <= outWhen_tmp

The Chisel input for this is (run-able Scastie: https://scastie.scala-lang.org/4BcVTPJCRsaM6nm1wbnyAQ):

class Foo extends Module {
  val in = IO(Input(UInt(2.W)))
  val outWhen = IO(Output(UInt(2.W)))

  outWhen := {
    val tmp = RegInit(0.U(2.W))
    when(in === 0.U)        { tmp := 0.U }
      .elsewhen(in === 1.U) { tmp := 1.U }
      .elsewhen(in === 2.U) { tmp := 2.U }
      .otherwise            { tmp := 3.U  }
    tmp
  }
}

Currently, MFC will generate (using either -O=debug or -O=release):

module Foo(
  input        clock,
               reset,
  input  [1:0] in,
  output [1:0] outWhen);

  reg [1:0] outWhen_tmp;
  always @(posedge clock) begin
    if (reset)
      outWhen_tmp <= 2'h0;
    else
      outWhen_tmp <= in == 2'h0 ? 2'h0 : in == 2'h1 ? 2'h1 : {1'h1, in != 2'h2};
  end // always @(posedge)
  assign outWhen = outWhen_tmp;
endmodule

This is nice and terse, but will confuse the hell out of designers or verification engineers that are looking at the output Verilog.

The SFC, conversely, will generate:

module Foo(
  input        clock,
  input        reset,
  input  [1:0] in,
  output [1:0] outWhen
);
  reg [1:0] outWhen_tmp;
  assign outWhen = outWhen_tmp;
  always @(posedge clock) begin
    if (reset) begin
      outWhen_tmp <= 2'h0;
    end else if (in == 2'h0) begin
      outWhen_tmp <= 2'h0;
    end else if (in == 2'h1) begin
      outWhen_tmp <= 2'h1;
    end else if (in == 2'h2) begin
      outWhen_tmp <= 2'h2;
    end else begin
      outWhen_tmp <= 2'h3;
    end
  end
endmodule

This is far, far less optimized, but a designer or verifier can handle this. Note: I don't think that the SFC output is the end goal. We can do better here, likely via case statements. This issue serves to track improvements to the above and also to solicit ideas on what would be good output.

The SFC algorithm here is extremely naive. It collapses all muxes looking backwards through wires and registers into a single large mux tree. It then poops this out as if/else trying to use else if when possible.

[uenoku]

FYI, #2553 and #2554 were my previous trials to convert these muxes into case statements in HWCleanUp. At that time I gave up because the transformation does violates xprop semantics. If we want to do this, we might have to perform the transformation between LowerSeqToSV and canonicalizers.

(EDIT: I mistakenly closed the issue :)

[seldridge]

In retrospect, #2553 and #2554 may have failed because they were applied to all muxes and not just ones driving registers. I realize that this is a seemingly arbitrary distinction. However, this is the exact distinction that SFC makes. Therefore, it is possible that the reason we ran into x-prop issues for existing Chisel code was due to applying the pattern differently, i.e., applying this pattern to combinational logic and there may have been undiscovered x-prop issues in our combinational logic.

Doing this in LowerSeqToSV (or before), as you astutely mention, would be a reasonable point for us to pull out the SFC pattern.

[seldridge]

Current output for this looks pretty good! Calling this closed.

// Generated by CIRCT unknown git version
module Foo(
  input        clock,
               reset,
  input  [1:0] in,
  output [1:0] outWhen
);

  reg [1:0] outWhen_tmp;
  always @(posedge clock) begin
    if (reset)
      outWhen_tmp <= 2'h0;
    else if (in == 2'h0)
      outWhen_tmp <= 2'h0;
    else if (in == 2'h1)
      outWhen_tmp <= 2'h1;
    else
      outWhen_tmp <= {1'h1, in != 2'h2};
  end // always @(posedge)
  assign outWhen = outWhen_tmp;
endmodule