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

Gowin: Remove nextpnr-gowin #1318

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

Conversation

yrabbit
Copy link
Contributor

@yrabbit yrabbit commented May 1, 2024

Boards with Gowin chips are supported in the Himbaechel architecture with much greater correctness and a wider range of primitives.

In fact, at the moment the advice “use himbaechel-gowin” solves a
significant part of the issues opened by users.

Of course, we need to wait for amendments to oss-cad-suite, at least YosysHQ/oss-cad-suite-build#109

Boards with Gowin chips are supported in the Himbaechel architecture
with much greater correctness and a wider range of primitives.

In fact, at the moment the advice “use himbaechel-gowin” immediately
solves a
significant part of the issues opened by users.

Of course, you need to wait for amendments to oss-cad-suite, at least
YosysHQ/oss-cad-suite-build#109

Signed-off-by: YRabbit <[email protected]>
@whitequark
Copy link
Member

This will break YoWASP, which has previously never had a package be withdrawn.

@whitequark
Copy link
Member

Actually, how does Himbaechel deal with per-architecture builds?

Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

I need to understand a way forward for YoWASP before we remove this.

@pepijndevos
Copy link
Member

Hum yeah aiui himbaechel makes a single binary that can support multiple micro architectures so either yowasp would have a single himbaechel package or maybe multiple renamed copies of it or...

My only other thought is a plugin system where you can dynamically load microarchitectures.

@whitequark
Copy link
Member

Right so since it's not viable to ship a single unified himbaechel binary due to PyPI binary size restrictions I need to understand what the solution to that is.

@Ravenslofty
Copy link
Collaborator

Ravenslofty commented May 1, 2024

But Himbaechel loads the chipdbs at runtime; they're not baked into the binary. That makes Himbaechel executables notably smaller than other architectures.

$ ls -lh nextpnr-ice40
-rwxr-xr-x 1 lofty lofty 223M Feb 22 22:44 nextpnr-ice40
$ ls -lh nextpnr-ecp5
-rwxr-xr-x 1 lofty lofty 184M Apr 30 12:11 nextpnr-ecp5
$ ls -lh nextpnr-himbaechel
-rwxr-xr-x 1 lofty lofty 40M Apr 26 16:47 nextpnr-himbaechel

Copy link
Collaborator

@Ravenslofty Ravenslofty left a comment

Choose a reason for hiding this comment

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

o7 nextpnr-gowin.

@whitequark
Copy link
Member

But Himbaechel loads the chipdbs at runtime; they're not baked into the binary. That makes Himbaechel executables notably smaller than other architectures.

Embedding chipdbs in the binary or not doesn't matter since one way or another they're shipped together. The question is that of the size of the final redistributable. If a build requires all chipdbs then it's a non-starter.

@Ravenslofty
Copy link
Collaborator

But Himbaechel loads the chipdbs at runtime; they're not baked into the binary. That makes Himbaechel executables notably smaller than other architectures.

Embedding chipdbs in the binary or not doesn't matter since one way or another they're shipped together. The question is that of the size of the final redistributable. If a build requires all chipdbs then it's a non-starter.

A build does not require all (or any!) chipdbs. Only at runtime do the chipdbs matter.

@whitequark
Copy link
Member

A build does not require all (or any!) chipdbs. Only at runtime do the chipdbs matter.

This isn't constructive. Forget it, I'll just read the source when I have a bit of time.

@Ravenslofty
Copy link
Collaborator

Ravenslofty commented May 1, 2024

I mean, in the most literal sense, that:

mkdir build
cd build
cmake .. -D ARCH=himbaechel
make -j $(nproc)

will produce a binary that can load any compatible himbaechel chipdb, and a path to a chipdb can be specified with the --chipdb argument. That lends itself naturally to a system of "distribute the main executable and chipdbs separately".

For the absolute literal "I want as close to just nextpnr-gowin as I can get", there's

cmake .. -D ARCH=himbaechel -D HIMBAECHEL_GOWIN_DEVICES=all -D APYCULA_INSTALL_PREFIX=path/to/apycula

or so.

@whitequark
Copy link
Member

Sure, but you can already build nextpnr with split chipdbs (I implemented that, if I recall).

My understanding is that there's also a C++ component even in Himbaechel, so the binary is tightly linked to the chipdb, and you have to build it for every architecture simultaneously and then split the final build products into multiple packages for file size reasons. If this is the case, this means that you can't have e.g. mismatched versions of nextpnr-himbaechel-ecp5 and nextpnr-himbaechel-ice40 be installed simultaneously, which would be a regression from the current state.

@Ravenslofty
Copy link
Collaborator

I think the answer to that is the --chipdb option (or perhaps an extension to --chipdbpath). If you can keep the incompatible chipdbs in separate locations, then the different versions would reference those locations.

@whitequark
Copy link
Member

That doesn't help if the C++ part changes (and historically at least nextpnr's C++ code changed in lockstep with chipdbs much of the time).

@whitequark
Copy link
Member

Wanda has made a good point, which is that Yosys has a similar single-flow approach and bugs in it are not generally a problem to the point where it's often necessary to keep several incompatible versions of Yosys co-installed.

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