Update coreir backend to support arrays of mixed direction #274
Conversation
Pull Request Test Coverage Report for Build 1068
💛 - Coveralls |
| @@ -275,7 +269,7 @@ def compile_definition_to_module_definition(self, definition, module_definition) | |||
| def connect_input(self, module_definition, port, | |||
| output_ports): | |||
| if not port.isinput(): | |||
| if isinstance(port, TupleType): | |||
| if isinstance(port, (TupleType, ArrayType)): | |||
There was a problem hiding this comment.
good for now, but is there some more general way of doing this?
There was a problem hiding this comment.
Any ideas? The idea here is that if a type has mixed directions, we need to descend to its children to check whether there are any inputs that need to be wired. In this case, we just have a tuple of types that have mixed direction children. Maybe we could add a field marking these special types? I think this covers the desired functionality though so I'm not sure what benefit making this more general would provide, or in fact what making it more general even means? Does that mean if we add another type with mixed directionality that it will still work?
There was a problem hiding this comment.
Good point. I think the key thing is for new types with mixed direction (if such a use case even exists). The most right way to do it is require all types to implement some methods which provide some information about mixed types (or expose some tree like structure so we can generically explore the type hierarchy and reach the leaves). Again, for now it covers our cases. We could say that all such new types are not supported directly in CoreIR anyway so our backend doesn't support them unless you special case it.
There was a problem hiding this comment.
One thing is to consider whether these mixed directional types are built on top of array/tuple. If so, this should just work. If not, then I think we should consider this API for determining directionality that types will need to implement.
Addresses #241 (comment)