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

Add VTR backend #271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add VTR backend #271

wants to merge 1 commit into from

Conversation

jgoeders
Copy link

I'm working on adding a back-end to run the VTR tool (https://verilogtorouting.org/)

Signed-off-by: Jeff Goeders <[email protected]>
@mithro
Copy link

mithro commented Sep 15, 2021

FYI - @kgugala / @acomodi / @mtdudek / @kowalewskijan

@jgoeders
Copy link
Author

Since multiple small PRs is easier, I'll start with this PR which provides a basic backend, and will add more functionality through subsequent PRs.

@jgoeders jgoeders marked this pull request as ready for review September 15, 2021 20:54
@jgoeders jgoeders mentioned this pull request Sep 15, 2021
5 tasks
Copy link
Owner

@olofk olofk left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution. I agree that it's best to take it in smaller steps and this is a great first step (bonus points for adding tests!). There are still things I want to change. Most notably the xml file handling

"description": "The VTR backend runs the VTR open-source FPGA CAD tool, consisting of synthesis, tech mapping, packing, placement and routintg.",
"members": [
{
"name": "vtr_path",
Copy link
Owner

Choose a reason for hiding this comment

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

I think this is better handled through the EDALIZE_LAUNCHER mechanism. Yes, it's a bit more tedious for the users but I also think it's more flexible and in line with the other tools

Copy link
Author

Choose a reason for hiding this comment

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

OK, I'll take a look at EDALIZE_LAUNCHER

Copy link
Owner

Choose a reason for hiding this comment

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

I realize I didn't give much context here and I also haven't properly documented this feature. The idea is that all calls to EDA tools is prefixed by $EDALIZE_LAUNCHER. Normally this variable is unset and the tool will just be called normally.

By setting EDALIZE_LAUNCHER we can run a command where the rest of the command-line becomes arguments to this command. A neat use case is setting EDALIZE_LAUNCHER to this script. By doing that we will seamlessly launch containerized versions of the supported tools. In your case, I think you could set EDALIZE_LAUNCHER to PATH=/path/to/vtr:$PATH so that the the path to where your tools is located will get added to the PATH variable priior to launching.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I tried out this suggestion and it seemed to work well. I had to set it like so (with quotes and $$):
export EDALIZE_LAUNCHER='PATH=/path/to/vtr:$$PATH'

The only issue is related to your question below:

The remaining issue then is how the user would get this arch description. Is it always shipped with vtr or does the user need to find it themselves?

The normal use case is to use an xml architecture file provided by VTR. These are typically located within <vtr_installation>/vtr_flow/arch. If I forgo the "vtr_path" tool_option and use the EDALIZE_LAUNCHER mechanism, then users are forced to provide the full hard-coded path to the architecture file in the VTR installation.

Alternatively with "vtr_path", users could provide the arch file as a path relative to the installation directory. Thoughts?

I like the EDALIZE_LAUNCHER mechanism, and I don't mind requiring full paths for arch files, just wondering if you have a better solution in mind.

"desc": "The path to the VTR tool installation",
},
{
"name": "route_chan_width",
Copy link
Owner

Choose a reason for hiding this comment

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

Replace this with a vtr_options list instead so that we don't need to update the backend if there are more options we want to support eventually

Copy link
Author

Choose a reason for hiding this comment

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

OK will do.


# Default files
verilog_path = None
if vtr_path:
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this file default? I don't know much about vtr internals but it looks to me like it's a specific chip. I would prefer to require the user to specify a valid chip themselves.

This ties into a bigger question about these XML files. Depending on how Edalize is used, it's not certain that the user can know the path to these files. Most users of EDA tools are expecting to just tell the EDA tool which chip they are targeting and let the tools handle the rest. After speaking to @kgugala about how these xml files work in symbiflow, I did a quick hack (https://gist.github.com/olofk/0f3911e7fa1028cc4c445bb2039ba06e) that makes it possible to programmatically get the official arch description files for a certain chip. I understand though that there is a need for certain types of users to override this file, but in this case I propose adding that as a tool option instead that these users can set by themselves.

Generatlly, things that go in the files section of the EDAM format are those that are design-specific, not specific to a tool installation path

Copy link
Author

@jgoeders jgoeders Oct 11, 2021

Choose a reason for hiding this comment

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

Why is this file default?

This is a common "basic" FPGA architecture in VTR. I don't have to provide a default. I was just providing a default with the belief that it should work with minimum number of settings provided (for example the Xilinx backend works without providing a part # -- but perhaps that is a feature of Xilinx, and not a intended feature of Edalize).

Depending on how Edalize is used, it's not certain that the user can know the path to these files. Most users of EDA tools are expecting to just tell the EDA tool which chip they are targeting and let the tools handle the rest.

I see the concern here, but I'm not sure it completely applies to VTR. While Symbiflow (which uses VTR) targets real parts, the current version of VTR is more about targeting hypothetical architectures. These don't describe actual parts, but rather an architecture family. For example, the default behavior of VTR is to automatically size a chip based on the input design, so the user is often not dealing with actual "parts".

Copy link
Owner

Choose a reason for hiding this comment

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

You do have a point about the Xilinx backend. I use that feature occasionally myself when I just want to do a quick synthesis test and don't really care exactly which chip I'm targeting. My only remaining worry is if vtr itself would settle upon a default which would differ from what we do in Edalize. I tend to be quite careful about adding defaults for that reason. But then again, I'm not (yet) a VTR user myself, so I'll leave the decision of setting this as the default arch to you.

The remaining issue then is how the user would get this arch description. Is it always shipped with vtr or does the user need to find it themselves?

Copy link
Author

Choose a reason for hiding this comment

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

See question above re arch files.

I'm fine requiring the user to provide an architecture file; you make a good point about VTR not having a default architecture yet.

arch_path = pathlib.Path("k6_N10_mem32K_40nm.xml")

# Check all input file types
for f in self.files:
Copy link
Owner

Choose a reason for hiding this comment

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

Does VTR only support verilog netlists, or is e.g. blif intended to be supported as well?

Copy link
Author

Choose a reason for hiding this comment

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

blif is an intermediate format in the VTR flow, and it's possible to provide a blif and skip the front-end of the VTR flow. I was planning to add that in the future, but I could do that now if it's important.

Copy link
Owner

Choose a reason for hiding this comment

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

That's fine. Better to add more features later as you had planned. Was just making sure I wasn't missing anything

@mithro
Copy link

mithro commented Oct 11, 2021

@olofk - While SymbiFlow makes up a large number of users targeting real hardware, there is also a very large number of users that are working with VtR / VPR on virtual hardware. Being able to provide the architecture files (instead of a very limited part number) is a pretty important use case.

BTW It is also likely that providing a "hardware database" rather than a part number will become more common as the FPGA Interchange Format gains more adoption.

@olofk
Copy link
Owner

olofk commented Oct 11, 2021

@mithro My ambition is to support both use cases. People with virtual hardware or vtr devs will likely know where they have their architecture files. This is fine. We just need to provide an option for them to supply this file. For other users (like me) I'm hoping to avoid this for reasons cited above. They still need to know something about their target so that we can pick the correct description file, and to me it seems like the chip identifier is the most reasonable thing to use here. This however requires some kind of translation that is hidden to the users, which is what I attempted with https://gist.github.com/olofk/0f3911e7fa1028cc4c445bb2039ba06e . This approach might very well not work for other reasons but it would be good to figure out how to proceed

@olofk
Copy link
Owner

olofk commented Oct 11, 2021

Also, if there is a large mass of users who would supply these xml files anyway, then we can start by supporting that method and add the chipname-to-xml mapping later

@mithro
Copy link

mithro commented Oct 12, 2021

Also, if there is a large mass of users who would supply these xml files anyway, then we can start by supporting that method and add the chipname-to-xml mapping later

This seems like a good plan?

@mithro
Copy link

mithro commented Oct 12, 2021

@mithro My ambition is to support both use cases.

I agree!

This however requires some kind of translation that is hidden to the users, which is what I attempted with https://gist.github.com/olofk/0f3911e7fa1028cc4c445bb2039ba06e . This approach might very well not work for other reasons but it would be good to figure out how to proceed

We could "extend" the class Vtr(Edatool): with a class VtrWithSymbiFlowDeviceFiles(Vtr): that finds and provide the SymbiFlow created device files. (Maybe with composition rather than inheritance?)

@olofk
Copy link
Owner

olofk commented Oct 12, 2021

So, a couple of follow-up questions for @jgoeders et.al. after digging through the vtr docs a bit.

  1. Looking at the run_vtr_flow.py script I'm a bit confused about the arguments. It says usage = "%(prog)s circuit_file architecture_file [options]" but all the examples look like %(prog)s arch.xml circuit.v -v 2, i.e. architecture_file before circuit_file (assuming I got the nomenclature right).
  2. Is run_vtr_flow.py the entrypoint we should use? We do have the option to let Edalize take over the responsibility of this wrapper script and call the tools directly if we prefer. Another route is to teach run_vtr_flow.py about EDAM so that we can feed it an EDAM file directly. The reason for bringing this up is that in my experience all these wrappers tend to limit the flexibility of the underlying tools and also tend to change a bit more frequently over time so the closer we can get to the real binaries, the less moving parts and more flexibility we have

@mithro I think the most straight-forward solution for the symbiflow device files is to implement that as a FuseSoC generator if we expect things to change in a not too distant future. Once everything is in place I can cook up an example

@olofk
Copy link
Owner

olofk commented Oct 12, 2021

I did an attempt now to do a test run of the vtr backend with the dockerized version of vtr (available here ) Some notes so far:

  • The vtr backend must define argtypes = []. Otherwise FuseSoC gets upset. Long term I think FuseSoC should fail more gracefully here, but this is what we have now
  • The dockerized version doesn't seem to get built with the python interface. That's not the fault of the vtr backend but I think it's another reason for bypassing the python interface when using Edalize. We have no need for it (AFAIU) and might instead make things more complicated.

Also, where can I get a verilog input file (or create one myself) to test this with?


############ Find VTR Tool #############
vtr_path = None
run_vtr_flow_py_path = "run_vtr_flow.py"
Copy link

Choose a reason for hiding this comment

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

I think we want to replace the run_vtr_flow.py script and call the tools directly using edalize here.

@jgoeders
Copy link
Author

Is run_vtr_flow.py the entrypoint we should use? We do have the option to let Edalize take over the responsibility of this wrapper script and call the tools directly if we prefer. Another route is to teach run_vtr_flow.py about EDAM so that we can feed it an EDAM file directly.

We have discussed this at the VTR meeting, and I believe decided that we wanted to start with what I have proposed here, that is, have an edalize backend that wraps run_vtr_flow.py. The main motivation for this is that we would like to continue to build critical mass around edalize, and for those already familiar with edalize, it provides an easy entry point to try out VTR. We aren't really expecting the VTR developers to immediately jump over to Edalize, although I think that's a good long-term goal.

We do have the option to let Edalize take over the responsibility of this wrapper script and call the tools directly if we prefer.

Yes, we have discussed this. It's possible, but it's a big undertaking. The run_vtr_flow.py infrastructure is surprisingly large for just being a wrapper around some tools. Currently it sits at 4000+ lines of Verilog, so it's a big undertaking to essentially replace it with an Edalize version here (and replicating will mean two places where changes need to be made in the future).

Additionally, the run_vtr_flow.py infrastructure is really about calling multiple different CAD tools and chaining them together. In my opinion, replicating this as a single edalize back-end isn't the best way to do this. Ideally we would have separate edalize backends that could run these various tools, and then have some mechanism to stitch them together. I'm not sure if this is something you envision for the future of edalize, but I'm inclined to wait for that before the large task of migrating the python code to edalize.

Another route is to teach run_vtr_flow.py about EDAM so that we can feed it an EDAM file directly.

Yes, another approach would be to instead make edalize backends for the various tools tools (yosys, odin, abc, ace, vpr, etc) and then if the "stitching them together" mechanism isn't going to be a feature of edalize, then we could do that in run_vtr_flow.py and call into edalize for each stage. I would be fine with that as well.

I think the long-term goal is to move VTR to Edalize (assuming that's what the VTR group wants to do), but I'd like to know more about the long-term strategy of composing different flows from sub-tools before starting the process of migrating all that code, and I don't really want to create a situation where we have two large flows that we maintain in both places.

For these reasons I've been starting with a very "lightweight wrapper" approach as you see in this PR, as a way to help build the Edalize momentum. But I'd be happy to discuss this further.

@mithro
Copy link

mithro commented Oct 13, 2021

Yes, another approach would be to instead make edalize backends for the various tools tools (yosys, odin, abc, ace, vpr, etc) and then if the "stitching them together" mechanism isn't going to be a feature of edalize, then we could do that in run_vtr_flow.py and call into edalize for each stage. I would be fine with that as well.

I think this is the right long term strategy? @kgugala, What do you think?

EDALize should be responsible for controlling the various tools and potentially pulling together the tools into a flow. Eventually run_vtr_flow.py ends up being a thin wrapper around calling EDALize?

@olofk
Copy link
Owner

olofk commented Oct 19, 2021

I think the long-term goal is to move VTR to Edalize (assuming that's what the VTR group wants to do), but I'd like to know more about the long-term strategy of composing different flows from sub-tools before starting the process of migrating all that code, and I don't really want to create a situation where we have two large flows that we maintain in both places.

I'm glad you bring up the question of stitching together tools in a controlled manner, because that is exactly what I am working towards. I started writing down some thoughts about this last year here https://github.com/olofk/edalize/wiki/Edalize-(Slight-return) but haven't found enough time to take it all the way. The overarching goal is to view each toolchain as a graph (DAG) with each tool as a node and EDAM structures as the edges. Each tool will be given an EDAM file that describes the inputs and tool configuration, and is expected to create an EDAM file with its expected outputs. That allows us to stitch together various workflows. Bulding the graph itself is being taken out as a separate task. Typically, a makefile or ninja file will be created to execute the graph but it could also be some cloud orchestration tool that executes each task on a different node.

As a first step I have converted about a third of the existing backends to this new structure and the proposed VTR backend looks like it follows this approach as well. Now, one conceptual difference from previous Edalize is that we used to only have backends that fully described a flow. What we will have in the future is a number of tools, and the role of the backends will be pretty much limited to describe the connections between the tools. They might even end up as something declarative rather than code eventually, and it should be possible for users to somehow tell Edalize to use a custom flow. But the first step is to make sure all current tools/backends have EDAM input/outputs

Hope this gives a bit more insight in the current plans. Happy to talk more. I'm desperately trying to find some time to bring out all of this from my head and onto paper so that we can have a shared vision

For these reasons I've been starting with a very "lightweight wrapper" approach as you see in this PR, as a way to help build the Edalize momentum. But I'd be happy to discuss this further.

I think we can start with this, but I would need some instructions for how to actually run this Python script thing. It doesn't seem to get installed when I install VTR and it wasn't in the docker image I tried. The important thing if we start out with the run_vtr_flow is to be opaque enough so that we can change the internals without affecting the API. For that reason I propose we already now expose backend options called odin_options, vpr_options etc so that we can feed these directly to the individual tools eventually even if we start out by just passing them all to the run_vtr_flow script

@kgugala
Copy link
Collaborator

kgugala commented Oct 25, 2021

One of the things we may want to do is to extend the tools (toolchains) we can (the open source ones) with some "self-awareness" features - pkgconfig style. In VPR based flow, the config tool can give us e.g. paths to architecture files, rr_graphs or dictionaries with options (we can even go further and provide fixed options required when targeting certain flow, and options that user can manipulate).

Probably it makes sense to introduce some kind of a standard here?

@jgoeders
Copy link
Author

Some more followups:

@olofk

Looking at the run_vtr_flow.py script I'm a bit confused about the arguments. It says usage = "%(prog)s circuit_file architecture_file [options]" but all the examples look like %(prog)s arch.xml circuit.v -v 2, i.e. architecture_file before circuit_file (assuming I got the nomenclature right).

It is indeed circuit_file arch_file. https://docs.verilogtorouting.org/en/latest/vtr/run_vtr_flow/. However, I just noticed that VPR (the largest sub-tool of the VTR flow) is arch_file circuit_file. Maybe you were looking at examples of running vpr? I never realized they were backwards.

The vtr backend must define argtypes = []. Otherwise FuseSoC gets upset. Long term I think FuseSoC should fail more gracefully here, but this is what we have now

Could you elaborate? I'm not currently using FuseSoC so not sure how to reproduce what you're mentioning here.

The dockerized version doesn't seem to get built with the python interface.

The dockerized version looks like VTR8; however, since that release we have moved to a Python runner script (run_vtr_flow.py) and the old perl version has been removed.

Also, where can I get a verilog input file (or create one myself) to test this with?

https://github.com/verilog-to-routing/vtr-verilog-to-routing/tree/master/vtr_flow/benchmarks/verilog

I propose we already now expose backend options called odin_options, vpr_options etc so that we can feed these directly to the individual tools eventually even if we start out by just passing them all to the run_vtr_flow script

OK, that sounds good. I'll work on that.

@kgugala That sounds interesting. I don't have experience with pkgconfig, (aside from running it), but I would be happy to learn. This sounds like something to talk more about at the VTR meetings.

@jgoeders
Copy link
Author

jgoeders commented Oct 26, 2021

@olofk Thanks for sharing https://github.com/olofk/edalize/wiki/Edalize-(Slight-return). I looked through this and it seems exactly what we would want to fully transition VTR to Edalize (whereas the current PR is just a light wrapper).

It seems for terminology, you are proposing breaking "back-ends" into "Stages/Nodes" and "Flows". This seems reasonable to me. With such a case then VTR would become a "Flow" and the underlying tools (odin, yosys, abc, vpr, etc) would be stages.

Let me know once you think the framework is closer and I can start building "stages" for the VTR sub-tools.

@umarcor umarcor mentioned this pull request Nov 1, 2021
@olofk
Copy link
Owner

olofk commented Nov 5, 2021

@jgoeders Great to hear we're on the same page. I have actually done a bit of work on this over the past few weeks. You can find a PoC where I have converted the icestorm backend to this new architecture here. I need to clean it up a little bit before pushing it to main but I think you can get the idea what it should look like. Perhaps it could even get you started creating the tool wrappers for e.g. odin, abc and vpr. Having those would be helpful to determine whether this new architecture holds up, and we could also merge them one by one to keep the size of the PR smaller

@olofk
Copy link
Owner

olofk commented Dec 20, 2021

FYI @jgoeders I have pushed the initial version of the new flow API to the master branch so we should be in a good spot to add the vtr flow now

@jgoeders
Copy link
Author

OK, great, thanks for the update! I'm pretty tied up with other projects at the moment, so likely won't be able to get to it for a while. I have new students coming on at the end of April, and I hope to make it a summer project for one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants