[HW][FlattenIO] Flatten io arrays

[hovind]

This is an attempt at following @uenoku's suggestions in https://github.com/llvm/circt/issues/6557#issuecomment-1884185136, by extending the FlattenIO pass flatten hw::ArrayType in addition to hw::StructType.

@mortbopet how does this agree with your usage of FlattenIO with ESI? According to 062eab31a67c8a8418b2ba9fb4f1814cdca08783:

My specific usecase for this is to be able to correctly lower ESI ports that are nested within a struct.

Is this specific usecase covered by a test?

diff --git a/test/Dialect/ESI/lowering.mlir b/test/Dialect/ESI/lowering.mlir
index b28197bcc..06d248133 100644
--- a/test/Dialect/ESI/lowering.mlir
+++ b/test/Dialect/ESI/lowering.mlir
@@ -1,6 +1,6 @@
 // RUN: circt-opt %s --lower-esi-to-physical -verify-diagnostics | circt-opt -verify-diagnostics | FileCheck %s
 // RUN: circt-opt %s --lower-esi-ports -verify-diagnostics | circt-opt -verify-diagnostics | FileCheck --check-prefix=IFACE %s
-// RUN: circt-opt %s --lower-esi-to-physical --lower-esi-ports --hw-flatten-io --lower-esi-to-hw -verify-diagnostics | circt-opt -verify-diagnostics | FileCheck --check-prefix=HW %s
+// RUN: circt-opt %s --lower-esi-to-physical --lower-esi-ports --lower-esi-to-hw -verify-diagnostics | circt-opt -verify-diagnostics | FileCheck --check-prefix=HW %s
 
 hw.module.extern @Sender(in %clk: !seq.clock, out x: !esi.channel<i4>, out y: i8) attributes {esi.bundle}
 hw.module.extern @ArrSender(out x: !esi.channel<!hw.array<4xi64>>) attributes {esi.bundle}
@@ -67,7 +67,7 @@ hw.module @add11(in %clk: !seq.clock, in %ints: !esi.channel<i32>, out mutatedIn
 }
 // HW-LABEL: hw.module @add11(in %clk : !seq.clock, in %ints : i32, in %ints_valid : i1, in %mutatedInts_ready : i1, out ints_ready : i1, out mutatedInts : i32, out mutatedInts_valid : i1, out c4 : i4) {
 // HW:   %{{.+}} = hw.constant 11 : i32
-// HW:   [[RES0:%.+]] = comb.add %ints, %{{.+}} : i32
+// HW:   [[RES0:%.+]] = comb.add %{{.+}}, %ints : i32
 // HW:   %{{.+}} = hw.constant 0 : i4
 // HW:   hw.output %mutatedInts_ready, [[RES0]], %ints_valid, %{{.+}} : i1, i32, i1, i4

The above changes seem to make the tests pass, but I imagine that I might be breaking something that I don't quite understand.

Would it be preferable to template the FlattenIO pass over an aggregate type (hw::StructType or hw::ArrayType in practice)? Or to provide parameters to the pass describing what aggregate types to flatten? Any advice would be appreciated.

[mortbopet]

Good catch; there's actually no test for lowering ESI channels to HW inside test/Dialect/ESI/lowering.mlir, which should definitely be added (CC @teqdruid). I don't think ArrayType in I/O has been an issue for us yet.

Would it be preferable to template the FlattenIO pass over an aggregate type (hw::StructType or hw::ArrayType in practice)? Or to provide parameters to the pass describing what aggregate types to flatten? Any advice would be appreciated.

Personally, i think opting in to enabling the specific types to be flattened is preferred.

[uenoku]

Thank you for doing this! Please feel free to merge HWLegalizeModule change.

[teqdruid]

I don't think ArrayType in I/O has been an issue for us yet.

Lowering an Array of channels is actually something I was working on recently. In that case, it's not so much flattening as lowering to Arrays of data/valid/ready signals.

Good catch; there's actually no test for lowering ESI channels to HW inside test/Dialect/ESI/lowering.mlir, which should definitely be added (CC @teqdruid).

There is:

• https://github.com/llvm/circt/blob/4a240b9f502ff648df705d54ddaf2ac83daf45b4/test/Dialect/ESI/lowering.mlir#L3C46-L3C63
• https://github.com/llvm/circt/blob/4a240b9f502ff648df705d54ddaf2ac83daf45b4/test/Dialect/ESI/lowering.mlir#L68