search

im-tomu / fomu-toolchain

A collection of tools for developing for Fomu
80 stars 11 forks source link

migrate from xobs/toolchain-* to open-tool-forge/fpga-toolchain #20

Closed umarcor closed 3 years ago

umarcor commented 3 years ago

Close #18 Close #19

NOTE: this PR is based on #19, which is based on #18.

This enhancement was discussed in: #16, im-tomu/fomu-workshop#319, im-tomu/fomu-workshop#334

Resources from xobs/toolchain-* repos are replaced with releases from open-tool-forge/fpga-toolchain. That is, the following tools are no longer explicitly included:

At the same time, fpga-toolchain includes multiple additional tools which are not available in the current Fomu toolchain:

The addition of ghdl-yosys-plugin is specially relevant because it provides built-in support for synthesising either VHDL or mixed-language designs, instead of requiring Docker as in im-tomu/fomu-workshop#334.

Since this PR is based on #19, these changes are tested in CI, ensuring that the tests from the workshop can be successfully executed on all three platforms. At the same time, artifacts are available for anyone to try: https://github.com/umarcor/fomu-toolchain/actions/runs/299916520. Feedback is very welcome.

xobs commented 3 years ago

Whoops, I didn't mean to mark it as mergeable. I just wanted to add some reviews and approve the general approach.

umarcor commented 3 years ago

@xobs, I converted it to a draft again. I think you (as a maintainer) could also see the option at the top of the sidebar. Not now, tho.

Please, place your comments as regular comments while the PR is draft.

umarcor commented 3 years ago

The README in this PR is updated now (see https://github.com/umarcor/fomu-toolchain/tree/fpga-toolchain). I believe it is ready for review. However, I would strongly suggest taking care of #18 and #19 first.

xobs commented 3 years ago

This builds, and generates an output image.

The only concern I have is that this really blows up the filesize -- nextpnr-ecp5 is 332 MB (although it does compress down to 68 MB). I'm inclined to say it's fine for now, though.

It does start to become apparent how the vendor toolchains got to 20 GB in size...

Any issues with merging this? @umarcor / @mithro ?

umarcor commented 3 years ago

This builds, and generates an output image.

Just for the record, previously it was executed in my fork only. After merging #19, this is now executed in this repo too.

The only concern I have is that this really blows up the filesize -- nextpnr-ecp5 is 332 MB (although it does compress down to 68 MB). I'm inclined to say it's fine for now, though.

I understand your concern. Overall the size of the package is increased 25-35%. From ~350MB to ~450MB. Yet, I think it is very desirable to bring the community together around a single toolchain build, for reducing fragmentation. Some day, all these tools will be available in "regular" package managers (apt, dnf, pacman...). But, meanwhile, we should try to reduce the fragmentation. From this point of view, I think it's ok to penalise Fomu users with a 30%, because that will allow them to use more languages and to try the designs with other open source boards, without having to deal with different and possibly conflicting toolchains.

Yet, there is a possible enhancement to be applied in fpga-toolchain. As discussed in YosysHQ/nextpnr#383, nextpnr might be built with -DARCH=ice40;ecp5. That might reduce the size, compared to building nextpnr-ice40 and nextpnr-ecp5. In ghdl/docker, three versions are provided: ICE40, ECP5 and all. See https://github.com/ghdl/docker/blob/master/cache_pnr.dockerfile#L116-L160 and https://hub.docker.com/r/ghdl/synth/tags:

/cc @edbordin.

It does start to become apparent how the vendor toolchains got to 20 GB in size...

Well, we are using less than 2.5% of that size yet šŸ¤£. I think that most of the size is taken by device models/data. Hence, as more devices are supported in the open source toolchains, I would expect the size to increase relatively fast. However, in the future, data should be decoupled from the tools.

Any issues with merging this? @umarcor / @mithro ?

I'm good.

xobs commented 3 years ago

Thanks again for this. I've flipped the switch and merged it into master.