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 a generator for Quartus VIRTUAL_PIN constraints #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add a generator for Quartus VIRTUAL_PIN constraints #2

wants to merge 2 commits into from

Conversation

GCHQDeveloper560
Copy link
Contributor

I have not incremented the version number, assuming 0.1.5 is new enough. Let me know if you'd rather see 0.1.6.

It's useful to be able to place and route a core by itself to get an intial feel for performance and area. The core will frequently require more I/O than the device has available. Rather than having to wrap the core with shift registers or the like, Quartus offers the useful VIRTUAL_PIN setting.

The Intel documentation suggests running synthesis and then a TCL script to perform this function, which is obviously complex and slow. This generator uses a Python HDL parser to do the same thing.

Basic pytest tests are included, though the directory structure and lack of any settings mean the tests must be run with "python -m pytest tests/" to get the current directory in the Python path.

It would be interesting to see future generators using HDL parsing to automate things like creating wrappers, HDL language conversion for tool/licensing issues, etc.

Basic pytest tests are included, though the directory structure and
lack of any settings mean the tests must be run with "python -m pytest
tests/" to get the current directory in the Python path.
@olofk
Copy link
Contributor

olofk commented Nov 21, 2019

This does sound like a useful thing. I'm always a bit sceptical towards HDL parsers as they tend to be fragile, but since this is a generator and not part of the core functionality, I guess it should be fine. @imphil suggested adding support for sv2v to make SystemVerilog designs usable with yosys, so there are other cases as well.

I read your comments about absolute paths to files. That is correct. Cores have no way of guessing the location of other cores. There are some cases where this might be handy, so I'm considering to push some more info through the generator API (e.g. a map of all cores and their respective files_root). Would that help here?

I can't find any issues with the patch, so I'm ready to push it in as soon as I have given it a quick test

@GCHQDeveloper560
Copy link
Contributor Author

Thanks for the quick review!

I'm sceptical too, and I think fragile is the right description. This was a bit of an experiment, and I would certainly think twice before using one in a critical spot.

The parser I settled on seems to be under very active development and the developer quickly fixed the one problem I saw, but I'm certainly open to something else if anyone knows of something better. I'd like VHDL support, so I'll avoid sv2v. 😃 I'd like to be able to parse entities/modules more completely to be able to make wrappers. Getting port names with hdlConvertor wasn't too bad, but getting types and widths seems a lot more difficult.

More information would probably be useful here. My test case has been making a "wrapper" core with no source of it's own other than a dependency on the core I'm testing. It then has targets for several devices, and uses this generator to wrap the core with the VIRTUAL_PIN settings. In this case files_root is for the "wrapper core", not the dependency. My workaround has been using absolute paths to the cached location of the core I'm wrapping. I suspect the cleanest solution for this case would be for the generator to be pointed to the right file with the name of the core and the relative path within that core. It would then hopefully have enough information to find files_root for that core rather than just for the "wrapper core."

@GCHQDeveloper560
Copy link
Contributor Author

After a little more thought if the generator had all of the information for the target (files and top) it could find the top module and do the vrtual pin settings without additional information.

@olofk
Copy link
Contributor

olofk commented Dec 5, 2019

I won't have time to look at this anytime soon unfortunately, but I think we should start by looking at what more information to expose to the generators. Top module, a list containing the dependency tree with path to their root directories? Perhaps the complete EDAM file that is sent to edalize? Not sure. It might make it more problematic for users using generators outside of FuseSoC. Are there such users? Again, not sure.

@GCHQDeveloper560
Copy link
Contributor Author

I'm afraid I deleted the fork that was the subject of this PR. I recreated it, but it still seems to be listed as "unknown repository."

Providing the complete set of EDAM information might be interesting to open up the possibilities of what generators might do. For example, a wrapper generator could modify toplevel to point to the wrapper instead of the wrapped core. If FuseSoC added support for building multiple targets then perhaps a generator could automate creating a bunch of targets with different constraint/device combinations for that fpga-tool-perf use case.

I took a look at edalizer but it appeared this was going to be non-trivial. Presumably we'd need to build the Edalizer.edalize structure along the way rather than combining snippets at the end and supply it instead of the current Ttptttg.generator_input. However, it wasn't clear to me how to handle generators and their position argument to build it up in the correct order.

I'll open a new FuseSoC issue to discuss providing more information to generators.

How would you like to handle this generator in the meantime? Should it go in as-is, knowing that absolute paths will be required, and then could take advantage of a new generator API later, or would you rather it wait until we decide what information to expose?

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.

2 participants