llvm / circt

Circuit IR Compilers and Tools
https://circt.org
Other
1.62k stars 281 forks source link

firtool misses `regreset` when generating .mlir from .fir with --preserve-aggregate=vec and array registers #7423

Open Anillc opened 1 month ago

Anillc commented 1 month ago

command:

firtool -format=fir -ir-fir --preserve-aggregate=vec foo.fir

foo.fir:

FIRRTL version 4.0.0
circuit Top :%[[
  {
    "class":"firrtl.transforms.DontTouchAnnotation",
    "target":"~Top|Top>x[0][0]"
  }
]]

  public module Top :
    input clock : Clock
    input reset : UInt<1>

    wire foo : UInt<1>[1]
    connect foo[0], UInt<1>(0h1)
    wire bar : UInt<1>[1][1]
    connect bar[0], foo
    regreset x : UInt<1>[1][1], clock, reset, bar

result:

module {
  firrtl.circuit "Top" {
    firrtl.module @Top(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) attributes {convention = #firrtl<convention scalarized>} {
      %0 = firrtl.aggregateconstant [1 : ui1] : !firrtl.vector<uint<1>, 1>
      %foo = firrtl.wire : !firrtl.vector<uint<1>, 1>
      %x = firrtl.reg %clock {annotations = [{circt.fieldID = 2 : i32, class = "firrtl.transforms.DontTouchAnnotation"}], firrtl.random_init_start = 0 : ui64} : !firrtl.clock, !firrtl.vector<vector<uint<1>, 1>, 1>
      firrtl.matchingconnect %x, %x : !firrtl.vector<vector<uint<1>, 1>, 1>
      firrtl.matchingconnect %foo, %0 : !firrtl.vector<uint<1>, 1>
      %1 = firrtl.vectorcreate %foo : (!firrtl.vector<uint<1>, 1>) -> !firrtl.vector<vector<uint<1>, 1>, 1>
    }
  }
}

without --preserve-aggregate=vec:

module {
  firrtl.circuit "Top" {
    firrtl.module @Top(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) attributes {convention = #firrtl<convention scalarized>} {
      %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> {name = "foo_0"}
      %x_0_0 = firrtl.regreset %clock, %reset, %c1_ui1 {annotations = [{class = "firrtl.transforms.DontTouchAnnotation"}], firrtl.random_init_start = 0 : ui64} : !firrtl.clock, !firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>
      firrtl.matchingconnect %x_0_0, %x_0_0 : !firrtl.uint<1>
    }
  }
}

tested on circt 1.58.0 and 1.76.0

Emin017 commented 1 month ago

Same output as above, also found on firtool 1.77.0:

~/.cache/llvm-firtool/1.77.0/bin/firtool --version                       
LLVM (http://llvm.org/):
  LLVM version 19.0.0git
  Optimized build.
CIRCT firtool-1.77.0
owlxiao commented 1 month ago

Through the --mlir-print-ir-after-all command, it can be seen that (firrtl-sfc-compat) has mishandled this.

before

firrtl.module @Top(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) attributes {convention = #firrtl<convention scalarized>} {
  %foo = firrtl.wire : !firrtl.vector<uint<1>, 1>
  %0 = firrtl.subindex %foo[0] : !firrtl.vector<uint<1>, 1>
  %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1>
  firrtl.strictconnect %0, %c1_ui1 : !firrtl.uint<1>
  %bar = firrtl.wire : !firrtl.vector<vector<uint<1>, 1>, 1>
  %1 = firrtl.subindex %bar[0] : !firrtl.vector<vector<uint<1>, 1>, 1>
  %2 = firrtl.subindex %foo[0] : !firrtl.vector<uint<1>, 1>
  %3 = firrtl.subindex %1[0] : !firrtl.vector<uint<1>, 1>
  firrtl.strictconnect %3, %2 : !firrtl.uint<1>
  %x = firrtl.regreset %clock, %reset, %bar {annotations = [{circt.fieldID = 2 : i32, class = "firrtl.transforms.DontTouchAnnotation"}]} : !firrtl.clock, !firrtl.uint<1>, !firrtl.vector<vector<uint<1>, 1>, 1>, !firrtl.vector<vector<uint<1>, 1>, 1>
  %4 = firrtl.subindex %x[0] : !firrtl.vector<vector<uint<1>, 1>, 1>
  %5 = firrtl.subindex %4[0] : !firrtl.vector<uint<1>, 1>
  firrtl.connect %5, %5 : !firrtl.uint<1>, !firrtl.uint<1>
}

IR Dump After SFCCompat (firrtl-sfc-compat)

firrtl.module @Top(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) attributes {convention = #firrtl<convention scalarized>} {
  %foo = firrtl.wire : !firrtl.vector<uint<1>, 1>
  %0 = firrtl.subindex %foo[0] : !firrtl.vector<uint<1>, 1>
  %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1>
  firrtl.strictconnect %0, %c1_ui1 : !firrtl.uint<1>
  %bar = firrtl.wire : !firrtl.vector<vector<uint<1>, 1>, 1>
  %1 = firrtl.subindex %bar[0] : !firrtl.vector<vector<uint<1>, 1>, 1>
  %2 = firrtl.subindex %foo[0] : !firrtl.vector<uint<1>, 1>
  %3 = firrtl.subindex %1[0] : !firrtl.vector<uint<1>, 1>
  firrtl.strictconnect %3, %2 : !firrtl.uint<1>
  %x = firrtl.reg %clock {annotations = [{circt.fieldID = 2 : i32, class = "firrtl.transforms.DontTouchAnnotation"}]} : !firrtl.clock, !firrtl.vector<vector<uint<1>, 1>, 1>
  %4 = firrtl.subindex %x[0] : !firrtl.vector<vector<uint<1>, 1>, 1>
  %5 = firrtl.subindex %4[0] : !firrtl.vector<uint<1>, 1>
  firrtl.connect %5, %5 : !firrtl.uint<1>, !firrtl.uint<1>
}
Emin017 commented 1 month ago

The problem seems to be with the following statement (suppose the width of the other dimension of Vec is 2):

regreset x : UInt<1>[2][1], clock, reset, bar

circt can handle with:

regreset x : UInt<2>[1], clock, reset, bar

Both generated verilog statements are:

reg [0:0][1:0] x;

But the verilog generated by the UInt<1>[2][1] statement does not contain the reset value