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.

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

Modified prjxray import to add tiles tags #653

Merged
merged 21 commits into from May 7, 2019

Conversation

acomodi
Copy link
Contributor

@acomodi acomodi commented Apr 26, 2019

This PR is still a WIP and addresses #582. It aims at creating the tiles tag into the arch xml description.

For now there is only the possibility to add the tiles, without any reference to the possible equivalent ones. Once also the equivalent tiles are added to the tile tag I will remove the WIP.

For now I left the top level-only tags (e.g. fc, pinlocations, etc.) also in the top level pb_types in the complexblocklist to let CI go green.

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@probot-autolabeler probot-autolabeler bot added lang-python Issue uses (or requires) Python language. type-utils Issues is related to the scripts inside the repo. labels Apr 26, 2019
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi acomodi force-pushed the prjxray-import-add-tile-tag branch from 8c39df9 to 0974f28 Compare April 29, 2019 13:43
@acomodi
Copy link
Contributor Author

acomodi commented Apr 29, 2019

With the most recent updates tiles are added to the arch.xml file. I have added make targets to produce a <tile_name>.tile.xml file that will be expanded when merging the whole architecture.

The only problem is that the tile tag creation depends on the generated pb_types.xml files to extract the pins of the equivalent tiles and add the corresponding mapping (this applies only to the tiles that allow other equivalent tiles). This prevents to have two equivalent tiles of the same type (e.g. CLBLM_L cannot have CLBLM_R as equivalent tile and vice-versa) as CMake will cause an error because the pb_type.xml target of CLBLM_R is not yet created.

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi acomodi changed the title WIP: Modified prjxray import to add tiles tags Modified prjxray import to add tiles tags Apr 29, 2019
@acomodi acomodi requested a review from mkurc-ant April 30, 2019 12:37
Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what the purpose of the new script xc7/utils/prjxray_tile_type_import.py does versus xc7/utils/prjxray_tile_import.py? I'm not sure how they work together. Either expand the relevant docstrings, or add a .md document explaining why both are needed.

@acomodi
Copy link
Contributor Author

acomodi commented May 1, 2019

Can you explain what the purpose of the new script xc7/utils/prjxray_tile_type_import.py does versus xc7/utils/prjxray_tile_import.py? I'm not sure how they work together. Either expand the relevant docstrings, or add a .md document explaining why both are needed.

@litghost Sure, I will add a generic README.md and fill it with the tile import. Maybe it could be filled and completed in the future with the other scripts.

Anyway, the main reason for having a separate tile_import script for the tiles tag is that all the pb_types.xml have to be created first as they contain the IO pin names that are needed to fill the equivalent_tiles pin mapping. Therefore the creation of the tile.xml files depends on (possibly) all the top level pb_types.xml.

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@probot-autolabeler probot-autolabeler bot added the type-docs Issue is related to documentation. label May 1, 2019
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
# TILE name of the tile that has to be generated (e.g. CLBLM_R, BRAM_L, etc.)
# SITE_TYPES list of sites contained in the considered tile (e.g. CLBLM_R contains a SLICEM and SLICEL sites)
# EQUIVALENT_TILES list of tiles equivalent to the considered one (e.g. CLBLL_R is equivalent to CLBLM_R and CLBLM_L)
# PIN_PREFIX translation pattern between prefixes of two equivalent tiles. This is needed as CLBLLs and CLBLMs have different
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the tile split is done, no prefix translation will be required. How will that be expressed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have deleted the PIN_PREFIX, given that the prefix translation is not required anymore as you correctly said. I need to delete it from the

xc7/make/project_xray.cmake Outdated Show resolved Hide resolved
xc7/utils/prjxray_tile_type_import.py Outdated Show resolved Hide resolved
@acomodi
Copy link
Contributor Author

acomodi commented May 6, 2019

Equivalent tiles seem to work as expected. Better placement solutions are selected as shown in the following figures (the test is the buttons one):

Without equivalent tiles:
without_equivalent_tile

With equivalent tiles:
with_equivalent_tiles

The placer places the blocks closer to the synthetic IOs.

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
…biflow-arch-defs into prjxray-import-add-tile-tag

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi acomodi requested a review from litghost May 6, 2019 14:10
…t-add-tile-tag

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi acomodi requested review from litghost and removed request for litghost May 6, 2019 16:01
@litghost
Copy link
Contributor

litghost commented May 6, 2019

Please merge with master, and I'll review again.

@acomodi
Copy link
Contributor Author

acomodi commented May 6, 2019

Please merge with master, and I'll review again.

@litghost Done

Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, let's fix up the merge and I think this is ready.

xc7/make/project_xray.cmake Outdated Show resolved Hide resolved
PORT_TAGS = ['input', 'output', 'clock']


def prefix_name(tile):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than c/p, can you import these functions from prjxray_tile_import?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


return ports

def add_pinlocations(xml, fc_xml, pin_assignments, tile):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we de-dup these functions from prjxray_tile_import?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi acomodi requested a review from litghost May 6, 2019 21:27
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, wait for green CI then merge.

@acomodi acomodi merged commit 38d7f79 into f4pga:master May 7, 2019
@acomodi acomodi deleted the prjxray-import-add-tile-tag branch June 19, 2019 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-python Issue uses (or requires) Python language. type-docs Issue is related to documentation. type-utils Issues is related to the scripts inside the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants