-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
List BEL bindings available for cell instances of macro expansions #76
base: main
Are you sure you want to change the base?
Conversation
Looks like there's an issue that's related to this PR: #44 |
Signed-off-by: Krzysztof Boronski <[email protected]>
interchange/LogicalNetlist.capnp
Outdated
struct SiteTypeRef { | ||
type @0 :Ref.ReferenceType = rootValue; | ||
field @1 :Text = "siteTypeList"; | ||
} | ||
annotation siteTypeRef(*) :SiteTypeRef; | ||
using SiteTypeIdx = UInt32; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a DeviceResources
construct and should probably stay there to keep the schema clean.
interchange/LogicalNetlist.capnp
Outdated
placements @4 : List(CellInstancePlacement); | ||
} | ||
|
||
# Acceptable physical placement for a cell instance | ||
struct CellInstancePlacement { | ||
siteType @0 : SiteTypeIdx $siteTypeRef; | ||
bel @1 : StringIdx $stringRef; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a need to provide this information, but the LogicalNetlist
would probably not be the place to do it. The LogicalNetlist
is meant to be device agnostic. This information is device specific and would be better served to be placed in DeviceResources
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If LogicalNetlist
was meant to be device agnostic, but there's a need to store device-specific data related to entries in primitive libraries, then is it even a good container for primitive libraries?
A possible way out of this, without severely breaking backwards compatibility, would be to introduce a list field in DeviceResources
that would have an entry for each instance in instList
of primLibs
which is not a pretty solution but at least it would move that out of LogicalNetlist
. I'm not sure if an elegant solution is possible without changing too much about primLibs
, because given your suggestion, we would have to store some data related to primitive libraries outside of the container that's supposed to encapsulate it.
Signed-off-by: Krzysztof Boronski <[email protected]>
@clavin-xlnx I moved the BEL locations to DeviceResources. I made an extra structure called |
perCellInstances @0 : List(PerCellInstance); | ||
|
||
struct PerCellInstance { | ||
# List of possible cell placements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would this work in practice? I don't see how the placements would get mapped backed to the primitive macro components. Seems like you'd need a name or reference to the library/macro cell component the placement is intended for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mapping is done by simply using same indices as in Device.primLibs.instList
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the association, please add comments so that someone examining the schema could easily know that is the case. Also, it would seem that some macros might not need a placement constraint or would have more permissive placement constraints than others. How are those details communicated? Relative offsets are another question such as multi-LUTRAM memories. Some might need to be adjacent, but don't necessarily need to fit into a specific location.
struct PrimLibsExtra { | ||
perCellInstances @0 : List(PerCellInstance); | ||
|
||
struct PerCellInstance { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of PerCellInstance
and perCellInstances
, could we use the name PrimCellInstance
and primCellInstances
?
perCellInstances @0 : List(PerCellInstance); | ||
|
||
struct PerCellInstance { | ||
# List of possible cell placements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the association, please add comments so that someone examining the schema could easily know that is the case. Also, it would seem that some macros might not need a placement constraint or would have more permissive placement constraints than others. How are those details communicated? Relative offsets are another question such as multi-LUTRAM memories. Some might need to be adjacent, but don't necessarily need to fit into a specific location.
Structures relevant for macro expansions are insufficient to specify precise locations for cell instances.
This is an issue with expansions like DRAM, which can contain multiple cells of the same type (eg. MUXF7), yet these cells have to be wired together in a specific way to avoid ending up with unroutable placements.
This PR proposes a solution to that issue with
CellInstance
struct now providing a list of acceptable bindings.