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

4.3.0 fails to build on 32bit #422

Closed
AdrianBunk opened this issue Nov 21, 2022 · 9 comments
Closed

4.3.0 fails to build on 32bit #422

AdrianBunk opened this issue Nov 21, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@AdrianBunk
Copy link

https://buildd.debian.org/status/logs.php?pkg=form&ver=4.3.0-1

reken.c: In function ‘Moebius’:
reken.c:3752:28: error: ‘struct R_const’ has no member named ‘numinprimelist’
 3752 |         for ( i = 0; i < AR.numinprimelist; i++ ) {
      |                            ^
reken.c:3753:23: error: ‘struct R_const’ has no member named ‘PrimeList’
 3753 |                 x = AR.PrimeList[i];
      |                       ^

@vermaseren Commit 0da2724 lacks an #if ( BITSINWORD == 32 ) similar to other code in this file, that is necessary due to commit 768b2d6.

@tueda tueda added the bug Something isn't working label Nov 22, 2022
@tueda
Copy link
Collaborator

tueda commented Nov 22, 2022

To check such pitfalls of 32-bit builds, probably we need to resurrect 32-bit build tests in our continuous integration environment, which was lost during the transition from Travis CI to GitHub Actions (#361).

tueda added a commit that referenced this issue Nov 23, 2022
Regression tests to prevent issues like #422.
@tueda
Copy link
Collaborator

tueda commented Nov 23, 2022

I have added a CI job with a 32-bit container, which fails due to this Moebius build error, as expected (log).

In local environments without Docker, one may try cross-compiling to test for building. For example, with zig 0.10 (on 64-bit Ubuntu 20.04 on WSL2), I can configure like

CC='zig cc -target i386-linux-gnu.2.28' CXX='zig c++ -target i386-linux-gnu.2.28' ./configure --build=x86_64-pc-linux-gnu --host=i386-pc-linux-gnu

though the resultant executables don't run on my machine (because I haven't installed 32-bit runtime libraries on that machine).

@sirving
Copy link

sirving commented Dec 5, 2022

The question that comes to my mind is does supporting a 32 bit version make sense?

What is the benefit of a 32 bit version? What is the downside of getting rid of this support? How many users?

What is the added level of work and complexity to support the 32 bit version?

I don't know the answers to the above questions, but it would be worth considering this. If there are only a few users it may be cost effective for them to upgrade to using the 64 bit version.

@alexmyczko
Copy link
Contributor

Unfortunately Debian popcon does not show which arches: https://qa.debian.org/popcon.php?package=form

But Microsoft Windows, macOS got rid of 32-bit, so did my workplace (at least 5 years ago if not 10)

And here is that other architectures:
https://buildd.debian.org/status/package.php?p=form&suite=sid

which according the main popcon page are hardly used:
https://popcon.debian.org/

@AdrianBunk
Copy link
Author

Raspberry Pi OS (based on Debian) was 32bit-only until 2 years ago, the original Raspberry Pi 1 and the cheapest Raspberry Pi Zero hardware do not support 64bit (but the latest Raspberry Pi Zero 2 W supports 64bit).

32bit is still popular in lower end embedded Linux.

There are more people using old/vintage hardware than many people think (Debian still has working unofficial m68k and hppa ports).

@vermaseren
Copy link
Owner

vermaseren commented Dec 6, 2022 via email

@AdrianBunk
Copy link
Author

@vermaseren I was replying to @alexmyczko regarding where remaining 32bit use in general is still today (on Linux and especially on Debian).

It is not clear to me why Form has 32bit specific codepaths at all.
Today the compromise is often that code is portable, but fast only on commonly used hardware. E.g. doing math with uint64_t works also on 32bit, but is fast only on 64bit.
But scientific software without remaining reasonable 32bit usecases often ends up being 64bit only, if writing portable code is hard for some reason.

@vermaseren
Copy link
Owner

vermaseren commented Dec 7, 2022 via email

tueda added a commit to tueda/form that referenced this issue Dec 15, 2022
* Build issue on 32-bit machines.

* Corner cases where n ~ MAXPOSITIVE.
@tueda
Copy link
Collaborator

tueda commented Dec 15, 2022

Though the necessity of the 32-bit version is still arguable, I have pushed a simple patch to fix the build failure. So, I close this issue.

For further comments on the 32-bit vs. 64-bit discussion, I suggest #426.

If the 32-bit moebius_ is still broken for any corner case, please comment on #430.

@tueda tueda closed this as completed Dec 15, 2022
Fenyutanchan added a commit to Fenyutanchan/Yggdrasil that referenced this issue Feb 19, 2023
Platforms `armv6l-linux-gnueabihf`, `armv7l-linux-gnueabihf`, `armv6l-linux-musleabihf` and `armv7l-linux-musleabihf` are done.
Platforms `i686-linux-gnu` and `i686-linux-musl` are not supported yet.
Platforms for Windows are not supported yet.
Fenyutanchan added a commit to Fenyutanchan/Yggdrasil that referenced this issue Feb 19, 2023
Platforms `armv6l-linux-gnueabihf`, `armv7l-linux-gnueabihf`, `armv6l-linux-musleabihf` and `armv7l-linux-musleabihf` are done.
Platforms `i686-linux-gnu` and `i686-linux-musl` are not supported yet.
Platforms for Windows are not supported yet.
Fenyutanchan added a commit to Fenyutanchan/Yggdrasil that referenced this issue Feb 21, 2023
Platforms `armv6l-linux-gnueabihf`, `armv7l-linux-gnueabihf`, `armv6l-linux-musleabihf` and `armv7l-linux-musleabihf` are done.
Platforms `i686-linux-gnu` and `i686-linux-musl` are not supported yet.
Platforms for Windows are not supported yet.
giordano added a commit to JuliaPackaging/Yggdrasil that referenced this issue Feb 21, 2023
* New Recipe: FORM v4.3.0

* add `MicrosoftMPI_jll` for Windows.

* Switch to git for 32-bit (vermaseren/form#422 and vermaseren/form#430)
Platforms `armv6l-linux-gnueabihf`, `armv7l-linux-gnueabihf`, `armv6l-linux-musleabihf` and `armv7l-linux-musleabihf` are done.
Platforms `i686-linux-gnu` and `i686-linux-musl` are not supported yet.
Platforms for Windows are not supported yet.

* Add some information for non-unsupported platforms

* All platforms except Windows.

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* Change the flags.

It seems to be `--disable-native` instead of `--enable_native=no`.

---------

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
tueda added a commit that referenced this issue Apr 20, 2023
* Build issue on 32-bit machines.

* Corner cases where n ~ MAXPOSITIVE.

Repatched 28e15ea (#441)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants