circt icon indicating copy to clipboard operation
circt copied to clipboard

[FIRRTL] Allow foreign types in module/instance ports

Open fabianschuiki opened this issue 2 years ago • 4 comments

In order to support partial lowerings involving the FIRRTL dialect, and to mix in foreign operations and types, we need a way to pass foreign types through FIRRTL modules, instances, and trivial connects.

This commit extends FModuleLike, InstanceOp, and StrictConnectOp to also accept foreign, non-FIRRTL types for ports and connections to those ports. It also adapts the LowerToHW pass to simply pass through foreign-typed values when they appear in module and instance ports. This requires adding special case handling in a few places since we cannot create an InOutType and therefore WireOp around arbitrary foreign types, so we generally have to immediately materialize foreign values.

This lowering can be quite usefully combined with the builtin unrealized_conversion_cast to wrap and unwrap values of foreign types and establish a path for FIRRTL values to cross dialect boundaries in a controlled fashion.

Allows foreign types in the following ops:

  • FModuleLike
  • InstanceOp
  • StrictConnectOp
  • WireOp

During lowering to HW, values are materialized directly through a backedge (wires and instance ports) since we cannot create temporary SV wires to hold these values.

Todo

  • [x] Land #2693
  • [x] Land #3323
  • [x] Land #3362

fabianschuiki avatar Feb 25 '22 17:02 fabianschuiki

It certainly makes sense that most firrtl ops do not support non-firrtl types. external types look a lot like firrtl.analog types in their restrictions. Even the lowering of the instance port case sounds like analog. Given it's only a couple ops (though key ops), it would be worth updating everyone to make them safe. So suggested rules: no external types in aggregates, No external types in any type-dependent operation (add, etc). StrictConnect only. With these restrictions, the impact shouldn't be too bad. This is also likely what we will need if we do partial or multi-pass lowering to seq and HW.

Just be sure to document which operations support external types and what the rules for dealing with them are in firrtl passes.

As for the instance lowering, it's fine to make a wire temporarily, but it would be good to remove those wires and do pure pass-through by the end of hw-lowering. I don't want to assume that connect and wire semantics are legal for external types (but using a wire just to make the mapping easier is fair, I think there is already a try-to-eliminate-temporary-wire thing).

darthscsi avatar Mar 05 '22 23:03 darthscsi

Sounds great! Marking this PR as a draft until this is reworked :+1:

fabianschuiki avatar Mar 07 '22 10:03 fabianschuiki

Reworked this PR to use 100% fewer OpaqueTypes. It now extends FModuleLike, InstanceOp, and StrictConnectOp to support foreign, non-FIRRTL types and just pass those values through when lowering to HW.

Should now be much simpler than the original PR, and no longer uses the PlaceholderOp helper to construct use-before-def in the resulting HW IR.

fabianschuiki avatar Mar 28 '22 16:03 fabianschuiki

With #3362 landed, this PR should now be a lot simpler and more streamlined. For FIRRTL types this should be an NFC, but for non-FIRRTL types this should enable a simple pass-through behaviour on module/instance ports, wires, and strict connects.

fabianschuiki avatar Jul 21 '22 14:07 fabianschuiki

Going to hit merge on this. We'll do a very blameful revert if this breaks anything :grimacing:

fabianschuiki avatar Sep 29 '22 17:09 fabianschuiki