YoWASP / nextpnr

Unofficial nextpnr WebAssembly packages
https://yowasp.org
ISC License
15 stars 3 forks source link

Add support for nextpnr-gowin #13

Closed pepijndevos closed 3 years ago

pepijndevos commented 3 years ago

Apycula seems to have a bit different build process than most other FOSS FPGA tools in that it's a Python package that bundles the chipdb, which is built in CI, so the only sensible way to install it is via pip. This means that bundling Apicula as a YoWasp package would be a huge PITA, but there would not be much benefit because it's already a Python package. So this PR only seeks to enable the nextpnr-gowin binary, which requires Apycula as a build dependency.

This PR is WIP, but just want to put it out there to see all the missing pieces and CI failures. The first commit only adds gowin to the nextpnr build command, and does not yet bundle it into a PyPi package.

whitequark commented 3 years ago

This means that bundling Apicula as a YoWasp package would be a huge PITA, but there would not be much benefit because it's already a Python package.

What has to happen here is that yowasp-nextpnr-gowin should require the exact version of Apicula used when building it as a dependency, so that installing it would pull a compatible Apicula package in.

pepijndevos commented 3 years ago

Hm, you don't run CI on PRs it seems. I've added a pypi package that "works on my machine"... at least it generates a wheel, I have yet to test it.

Apycula is not a runtime dependency of yowasp-nextpnr-gowin though, so on the one hand it makes sense on the other hand... not. It'd pull in numpy in case someone wants to generate a bitstream in their CI pipeline or whatever. Maybe it could be an extra dependency?

whitequark commented 3 years ago

Apycula is not a runtime dependency of yowasp-nextpnr-gowin though, so on the one hand it makes sense on the other hand... not. It'd pull in numpy in case someone wants to generate a bitstream in their CI pipeline or whatever. Maybe it could be an extra dependency?

All other YoWASP packages include tools to generate bitstreams, so the Gowin one must, too, and the version mismatches must be prevented.

pepijndevos commented 3 years ago

Trying to figure out how to do the plumbing for the version. You'd think setuptools would be able to tell me what version of apycula it just installed, but I'm coming up short. ./apycula-prefix/bin/pip freeze | grep Apycula works so I could shell out from the setup.py but feels a bit dirty. Oddly the pip python module does not seem to expose this AFAICT.

whitequark commented 3 years ago

Give me a few minutes, I'll figure something out for this case.

whitequark commented 3 years ago
>>> import importlib.metadata
>>> importlib.metadata.version('apycula')
'0.0.1a9'
whitequark commented 3 years ago

Ideally you would use it with a shim like this.

In that case, you should also add "importlib_metadata; python_version<'3.8'" to setup_requires.

pepijndevos commented 3 years ago

Hah, I found this https://pip.pypa.io/en/latest/user_guide/#using-pip-from-your-program eh, up to you if you prefer your solution.

whitequark commented 3 years ago

I've updated the CI configuration so that it runs on pull requests. To make it work, you'll need to update the package workflow in your PR.

pepijndevos commented 3 years ago

Alright. So I think what remains to be done is to add the pypi-gowin to the CI and test that the flow actually produces working bitstreams.

pepijndevos commented 3 years ago

Working bitstream produced!

whitequark commented 3 years ago

Everything seems reasonable now--I'm going to do a few more checks and then merge.

Thank you a lot for the PR and the quick updates on the review comments!

pepijndevos commented 3 years ago

Awesome!

My plan is to use this to build the examples in Apicula using this as a way to catch the most dumb mistakes. Probably best I can do short of setting up a hardware-in-the-loop CI.

whitequark commented 3 years ago

Do you need the find_package(PythonInterp 3.6 REQUIRED) line in nextpnr-gowin, by the way? You never actually use PYTHON_EXECUTABLE as far as I can see, only GOWIN_BBA_EXECUTABLE, which may use a Python completely different from what find_package(PythonInterp) finds.

pepijndevos commented 3 years ago

Actually can't really remember... probably not? Might be a historical leftover from the nextpnr-generic heritage or some thing I did for debugging.

pepijndevos commented 3 years ago

I think I only added the flag so if you use the Python scripting interface that it ends up using the same interpreter as the virtualenv. I don't think the cmake line is actually needed.

whitequark commented 3 years ago

Removed it in https://github.com/YosysHQ/nextpnr/pull/765.

pepijndevos commented 3 years ago

Lol https://github.com/YosysHQ/nextpnr/pull/766