[Support/ESI] Add generic port conversion utility #5533
Conversation
|
Thanks for doing this. Do you think it'll work for your other PR? |
oh, definitely! As i wrote, #5390 fits perfectly into this paradigme. |
mortbopet
force-pushed
the
dev/mpetersen/input_rewriter
branch
from
471b8d0
to
715adb0
Compare
mortbopet
changed the base branch from
main
to
dev/mpetersen/flatten_io_fix_instance
mortbopet
force-pushed
the
dev/mpetersen/input_rewriter
branch
from
ecb5a56
to
8cebf71
Compare
There was a problem hiding this comment.
Schweeeet! Thanks again for doing this.
You should probably change all verbage around "signaling standard" to something more generic. (Given that this is intended to be more generic than different signaling protocols. Yes, I do get that pretty much every wire is a ad-hoc signaling protocol if you squint hard, but most people wouldn't call it that.)
The problem with replacing 'esi.portFlattenStructswithhw-flatten-io` is that the latter flattens every module in the design. The former attribute allows you to select each module individually. Which is kinda important when you're trying to glue together a bunch of external modules with the 'flatten' signaling standard.
lib/Dialect/HW/CMakeLists.txt
Outdated
| @@ -32,7 +33,7 @@ add_circt_dialect_library(CIRCTHW | |||
| LINK_COMPONENTS | |||
| Support | |||
|
|
|||
| LINK_LIBS PUBLIC | |||
There was a problem hiding this comment.
uber-nit: unnecessary whitespace creates inconsistency within this file.
| Value SignalingStandard::mapOutputDataPorts(OpBuilder &b, | ||
| ArrayRef<Backedge> newResults) { | ||
| auto chanTy = origPort.type.dyn_cast<ChannelType>(); | ||
| Type dataPortType = chanTy ? chanTy.getInner() : origPort.type; |
There was a problem hiding this comment.
I don't mind if we get rid of this functionality short term, but I suspect we're going to need it.
I thought about this but didn't come up with a good name. Should we just do
Can just add that as an argument to the pass or tag on attributes to the modules to be flattened. |
|
Yeah, port portconversion works. I'd prefer a tag on each module rather than a list of module symbols to the pass. Does flatten-io flatten embedded structs as well? We don't want that behavior. |
Nope, strictly structs in module in/outputs. |
This commit takes the ESI `ChannelRewriter` and generalizes it to provide a generic utility for performing port conversions on a `hw::HWMutableModuleLike` op. It is intended to be a generic utility that can facilitate replacement of a given module in- or output to an arbitrary set of new inputs and outputs (i.e. 1 port -> N in, M out ports). Typical usecase is where an input (/output) of a module represents has some higher-level type that will be implemented by a set of lower-level typed in- and outputs ports + supporting operations within a module. It also attempts to do so in an optimal way, by e.g. being able to collect multiple port modifications of a module, and perform them all at once. A change wrt. the `ChannelRewriter` is that struct flattening has been removed. This is both to make the lowering more generic (hard to support the notion of "if this ESI channel type contains a struct, and we need to flatten the struct, ...", and is redundant, due to the `hw-flatten-io` pass.
mortbopet
force-pushed
the
dev/mpetersen/input_rewriter
branch
from
5e32408
to
51c958b
Compare
mortbopet
requested review from
dtzSiFive,
darthscsi,
seldridge,
fabianschuiki and
maerhart
as code owners
mortbopet
changed the base branch from
dev/mpetersen/flatten_io_fix_instance
to
main
This commit takes the ESI `ChannelRewriter` and generalizes it to provide a generic utility for performing port conversions on a `hw::HWMutableModuleLike` op. It is intended to be a generic utility that can facilitate replacement of a given module in- or output to an arbitrary set of new inputs and outputs (i.e. 1 port -> N in, M out ports). Typical usecase is where an input (/output) of a module represents has some higher-level type that will be implemented by a set of lower-level typed in- and outputs ports + supporting operations within a module. It also attempts to do so in an optimal way, by e.g. being able to collect multiple port modifications of a module, and perform them all at once. A change wrt. the `ChannelRewriter` is that struct flattening has been removed. This is both to make the lowering more generic (hard to support the notion of "if this ESI channel type contains a struct, and we need to flatten the struct, ...", and is redundant, due to the `hw-flatten-io` pass.
This commit takes the ESI
ChannelRewriterand generalizes it to provide a generic utility for performing port conversions on ahw::HWMutableModuleLikeop.It is intended to be a generic utility that can facilitate replacement of a given module in- or output to an arbitrary set of new inputs and outputs (i.e. 1 port -> N in, M out ports). The typical usecase would be where an input (/output) of a module represent some higher-level type that will be implemented by a set of lower-level typed in- and outputs ports alongside supporting operations in the module-under-modifications and at instantiation sites. This capability can be leveraged by a bunch of other future tools, such as instance-hierarchy-boring capabilities.
It also attempts to do so in an optimal way by being able to collect multiple port modifications of a module and perform them all at once.
A change wrt. the
ChannelRewriteris that struct flattening has been removed. This is both to make the lowering more generic/less monolothic (hard to support the notion of "if this ESI channel type contains a struct, and we need to flatten the struct, ...", and is redundant, due to thehw-flatten-iopass.This first PR is used to rewrite the ESI
ChannelRewriter. The first immediate use-case afterwards is to reimplement #5390 which fits perfectly under this paradigm.Note: still a draft due to a bug in
hw-flatten-iowhich is needed for the tests.