[HW] Pass to flatten !hw.struct types

[jopperm]

trafficstars

As discussed in the ODM on 5/18/22. The idea is to transform

hw.module @foo(%in: !hw.struct<a: i8, b: i16>) -> (out: !hw.struct<c: i16, d: i8>) {
  %0 = hw.struct_extract %in["a"] : !hw.struct<a: i8, b: i16>
  %1 = hw.struct_extract %in["b"] : !hw.struct<a: i8, b: i16>
  %2 = hw.struct_create (%1, %0) : !hw.struct<c: i16, d: i8>
  hw.output %2 : !hw.struct<c: i16, d: i8>
}

to

hw.module @foo_flat(%in_a: i8, %in_b: i16) -> (out_c: i16, out_d: i8) {
  hw.output %in_b, %in_a : i16, i8
}

[jopperm]

I have done this in a limited fashion in an internal tool. I'll put up the code in a draft PR for further discussion.

[jopperm]

CC: @rsetaluri @leonardt

[jopperm]

Turned out that #3170 wasn't the right way to this. The FIRRTL dialect already provides some form of structure flattening, but unfortunately I don't have the time right now to dig into that. If anyone is interested in taking over, let me know. I'm happy to help and still interested in using such a pass for the core dialects.

[rsetaluri]

Looks like #4007 works very well to flatten structs appearing in IO -- awesome!

Wondering if there's away to also get rid of internal uses of structs; i.e. basically purge all uses of hw.struct in a piece of IR. The motivation (as most folks probably know) is that some downstream tools don't like seeing struct in the Verilog.

We could of course do this at a higher-level -- and we do currently -- but it might be useful for there to be a pass that essentially is a (soft) contract that there is no struct in the output Verilog.

I imagine this may not be trivial given issues with in-outs, bi-directional ports, etc. Happy to help lead or contribute to this effort.

cc @jopperm @mortbopet @mikeurbach

[mikeurbach]

There is a pass in the FIRRTL dialect that does exactly this: https://github.com/llvm/circt/blob/main/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp. It handles flattening structs in the ports and then chases them through the bodies of modules to flatten them as well. I haven't thought about what that would look like in the HW dialect, but I don't think there is anything that fundamentally prevents it, so it seems like a reasonably addition on top of #4007.

[rsetaluri]

There is a pass in the FIRRTL dialect that does exactly this: https://github.com/llvm/circt/blob/main/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp. It handles flattening structs in the ports and then chases them through the bodies of modules to flatten them as well. I haven't thought about what that would look like in the HW dialect, but I don't think there is anything that fundamentally prevents it, so it seems like a reasonably addition on top of #4007.

Thanks for the context @mikeurbach. As a starting point, it looks like the pass doesn't deal with hierarchy (e.g. instances of modified modules are not updated).

@mortbopet perhaps you can point me to places in #4007 I can start to modify things? Not sure if structural changes to the pass are needed to make this work.

[mortbopet]

@rsetaluri I'd suggest adding a complimentary pass (e.g. flatten-structs) which will flatten all structs internally in a module, but not touch the I/O - both passes can then be used in conjunction to remove all structs.

wrt. implementation, i'd expect a lot of the code can be pulled out of --hw-flatten-io and reused. This new pass will have to do a bit more thinking wrt. rewriting all of the struct operations. However, this should be a rather straight-forward application of MLIRs pattern rewrite infrastructure - a lot of the complexity of hw-flatten-io is concerning the I/O modifications (which is another argument for keeping the two passes separate).

[mikeurbach]

FWIW, FIRRTL's lower types does deal with module internals, including hierarchy: https://github.com/llvm/circt/blob/6224bb39460344f5aa4dab749f5236e5a42dae58/test/Dialect/FIRRTL/lower-types.mlir#L45-L49

If it provides some inspiration, instance ops are handled here: https://github.com/llvm/circt/blob/e76fec3adba65f293904c98e2c4b08c2509e3b23/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp#L1262