Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add_prim_lib should use indexed strings from DeviceResources instead of nested LogicalNetlist #47

Closed
gatecat opened this issue Apr 1, 2021 · 2 comments · Fixed by #49
Closed

add_prim_lib should use indexed strings from DeviceResources instead of nested LogicalNetlist #47

gatecat opened this issue Apr 1, 2021 · 2 comments · Fixed by #49
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@gatecat
Copy link
Contributor

gatecat commented Apr 1, 2021

I found some subtle breakage in the current nexus data because the nexus primLibs netlist contains local string indices, stored within the netlist's strList, whereas the existing code assumes that primLibs shares a strList with the parent device resources.

It's unclear which of the two options is better here - if the current approach, as used by the xilinx database is preferred then the primitive library patching tool will need updating.

cc @litghost
@litghost
Copy link
Contributor

litghost commented Apr 1, 2021 •
edited

The solution is to use the Xilinx database approach, which is to use the DeviceResource strList for instances of LogicalNetlist. This is partially to avoid to much complexity in the capnp <-> plaintext translation logic and annotations.

The code in python-fpga-interchange already have some support for this for reading, but it might be missing for writing.

@litghost litghost transferred this issue from chipsalliance/fpga-interchange-schema Apr 1, 2021
@litghost litghost changed the title Indexed strings in primLibs add_prim_lib should use indexed strings from DeviceResources instead of nested LogicalNetlist Apr 1, 2021
@litghost
Copy link
Contributor

litghost commented Apr 1, 2021

it might be missing for writing.

Ya, it is missing. The reader takes an argument to a string indexer (see here: https://github.com/SymbiFlow/python-fpga-interchange/blob/b4331eff339790b9eaa095083aec5e628712bb75/fpga_interchange/interchange_capnp.py#L572)

The writer should take a string indexer, which is optional. If the string indexer isn't provided, it builds an internal one. Then the patch tool can provide a string indexer seeded from the DeviceResource's, and update the strList after patching in the prims library.

@litghost litghost added good first issue Good for newcomers bug Something isn't working labels Apr 1, 2021
litghost added a commit to litghost/python-fpga-interchange that referenced this issue Apr 3, 2021
@litghost
Have add_prim_lib use DeviceResources.strList when patching.
4b3d682 
When nesting LogicalNetlist.strList in another schema, the root strList
is used.

Fixes chipsalliance#47

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>
@litghost litghost mentioned this issue Apr 3, 2021
Merged
FPGA interchange bootstrapping automation moved this from To Do to Done Apr 5, 2021
@gatecat gatecat mentioned this issue Jun 14, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Have add_prim_lib use DeviceResources.strList when patching. litghost/python-fpga-interchange
2 participants
@litghost @gatecat