-
Notifications
You must be signed in to change notification settings - Fork 108
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
Initial U-Ray database import #1651
Conversation
@@ -106,7 +106,7 @@ function(ADD_XC_ARCH_DEFINE) | |||
CELLS_SIM ${YOSYS_CELLS_SIM} ${VPR_CELLS_SIM} | |||
VPR_ARCH_ARGS ${VPR_ARCH_ARGS} | |||
RR_PATCH_TOOL | |||
${symbiflow-arch-defs_SOURCE_DIR}/xc/common/utils/prjxray_routing_import.py | |||
${symbiflow-arch-defs_SOURCE_DIR}/xc/common/utils/${PRJRAY_NAME}_routing_import.py |
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.
I'd prefer if don't hard fork these scripts. They aren't small pieces of code to begin with, and I'm not excited about needing to maintain two sets. Can we factor out the xc7 vs xcu specific stuff?
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.
Actually I agree. I took that approach with assign_tile_pin_direction.py
.
So the idea would be to have the common code in eg. rounting_import.py
plus two wrapper scripts prjxray_routing_import.py
and prjuray_routing_import.py
that would contain the specific code. These specific scripts would then be used in CMake. Does that make sense?
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.
Sounds good to me.
d606a60
to
572e9d3
Compare
7854e57
to
f627401
Compare
@litghost I created a doc with brief description of my ideas for the const network: https://docs.google.com/document/d/1Lhkqsa9YTY03ZpmeB6Q2POEYa8Tgm1yIrSBuV5bNADQ
|
f1eb8c3
to
af6d762
Compare
61d84dd
to
3c17d4d
Compare
@@ -0,0 +1,530 @@ | |||
#!/usr/bin/env python3 | |||
# -*- coding: utf-8 -*- |
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.
Why does this need a full copy/paste?
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.
I'm not entirely happy with that but I didn't wanted any U-Ray scripts depend on X-Ray stuff.
The content of this file is copied from third_party/prjxray/prjxray/timing.py
. This code is not present in prjuray-tools
so I couldn't import it from there. Since it is not directly related to X-Ray/U-Ray database but rather to how it is handled in arch-defs I decided to put it in arch-defs, at least for U-Ray import for now.
It may be worth to rename this to eg. timing.py
and make X-Ray import use it as well since it is common to both archs. Any thoughts on that?
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.
We don't have a uray routing timing model just yet, so can we stub this out, rather than assuming the 7-series and US+ route timing model will be the same?
# ============================================================================= | ||
|
||
|
||
def yield_downstream_nodes(conn, node_pkey): |
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.
Why is this specialized for prjuray? Should be the same between prjxray and prjuray?
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 function is only used in classify_const_nodes
which is prjuray specific so it is not needed in prjxray. Even though that it would work for xray data in the same way.
fed8c13
to
9a80f2f
Compare
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
…m happy Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
9a80f2f
to
acd94ea
Compare
@@ -0,0 +1,516 @@ | |||
#!/usr/bin/env python3 | |||
""" Classify 7-series nodes and generate channels for required nodes. |
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.
File level comment still needs to be updated? At a minimum 7-series -> UltraScale here?
|
||
def yield_downstream_nodes(conn, node_pkey): | ||
""" | ||
For the given node pkey yields pkeys of all its immediate downstream nodes. |
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 comment does mention that pseudo pips are avoided, etc. This function comment doesn't provide enough context of what and why this function is doing.
|
||
def identify_const_nodes(conn): | ||
""" | ||
Creates a new table for const nodes named "const _nodes". Identifies them |
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.
"const _nodes" -> "const_nodes"?
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.
Rather than add a table, maybe should add a node property on the nodes table?
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.
Comments in
This PR adds a skeleton for Xilinx US/US+ support in SymbiFlow. There were some minor changes to
xc/common/cmake
files to accomodate X-Ray and U-Ray specifics. I assumed that all db import scripts will be named likeprjxray_<something>.py
andprjuray_<something>.py
. Most of the required files that are not critical to early stages of db import are just templates/placeholders for now.Apart from all necessary directories and CMakes that allows
make env
to succeed it also adds the channel formation script.prjuray_form_channels.py
. The script has been adjusted to US/US+ specifics (eg. different const network connections) so that it succeeds and thechannels.db
can be successfully created.With this PR two submodules were added:
prjuray-db
andprjuray-tools
. The latter one is meant to provide U-Ray database access code after SymbiFlow/prjuray-tools#3 is successfully landed (without it the db import fails).