briansmith / ring

Safe, fast, small crypto using Rust
Other
3.64k stars 682 forks source link

Introduce SPARC64 support #2077

Closed Richard-Rogalski closed 1 month ago

Richard-Rogalski commented 1 month ago

Adds the minimum information to build on sparc64 / sparcv9. Tests pass 100%.

Finally a sore in my conscience gone. Much thanks to the devs from Gentoo and Debian who work very hard to keep these machines in the shape they're in.

For end users: remember to CFLAGS="-mcpu=ultrasparc" CXXFLAGS="-mcpu=ultrasparc" RUSTFLAGS="-C target-cpu=ultrasparc". If you want to build for a higher target, the C/CXX target can be found in man gcc, where for rust you can find it with rustc --print target-cpus.

Closes: #1512

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.24%. Comparing base (8238c00) to head (4063289).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2077 +/- ## ======================================= Coverage 97.24% 97.24% ======================================= Files 144 144 Lines 19995 19995 Branches 228 228 ======================================= Hits 19444 19444 Misses 525 525 Partials 26 26 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Richard-Rogalski commented 1 month ago

You're right, sparcv9/ultrasparc is just an ABI and doesn't specify whether the target is 64 or 32 bit. Sparc LEONs are v9 32 bit only CPUs, and many distros ship sparcv9 bins as 32bit for legacy reasons. Huge oversight, apologies

The -mcpu=ultrasparc probably isn't needed on 64bit like glaubitz pointed out, iirc it was for v9 32bit installs.

Updating now

Richard-Rogalski commented 1 month ago

Okay. It builds and still passes all tests, regardless of what {C, CXX, RUST}FLAGS are set. It is exclusively for 64 bit sparc.

@glaubitz : On gentoo, we don't package rust for our 32-bit userland profile. (Upstream doesn't provide binary toolchains for sparc at all, and it's a hassle enough as it is for 64bit). Does debian support(or have plans to continue support for) rust programs on 32bit sparc? If so, I can do another PR in the future for sparcv9 32-bit.

Sparcv8 and lower, even I am not insane enough for :b and neither are any linux distros

Richard-Rogalski commented 1 month ago

Actually, I just saw the few duplicate issues and PRs, and this comment. To me, this seems like a much better approach-- the only thing I can see this whitelist doing is determining 32/64 bit, which there are definitely better ways to do than listing every possible architecture ever. Can I do a new PR implementing this instead?

glaubitz commented 1 month ago

Okay. It builds and still passes all tests, regardless of what {C, CXX, RUST}FLAGS are set. It is exclusively for 64 bit sparc.

@glaubitz : On gentoo, we don't package rust for our 32-bit userland profile. (Upstream doesn't provide binary toolchains for sparc at all, and it's a hassle enough as it is for 64bit).

Well, upstream doesn't provide binary toolchains for sparc64 either. However, Debian's SPARC port is fully 64-bit these days.

However, I usually add 32-bit SPARC support when I add 64-bit SPARC support since there is still a commercially supported 32-bit SPARC platform, namely Leon, which is used by ESA, NASA and other high-reliability industry products.

Does debian support(or have plans to continue support for) rust programs on 32bit sparc? If so, I can do another PR in the future for sparcv9 32-bit.

Sparcv8 and lower, even I am not insane enough for :b and neither are any linux distros

32-bit SPARC defaults to SPARCv9 these days, except for Leon which is SPARCv7 with support for atomic operations.

glaubitz commented 1 month ago

Actually, I just saw the few duplicate issues and PRs, and this comment. To me, this seems like a much better approach-- the only thing I can see this whitelist doing is determining 32/64 bit, which there are definitely better ways to do than listing every possible architecture ever. Can I do a new PR implementing this instead?

Well, how about we merge this commit first so that the package finally builds on sparc64. The improved version can still be implemented later, can't it?

briansmith commented 1 month ago

Actually, I just saw the few duplicate issues and PRs, and this comment. To me, this seems like a much better approach-- the only thing I can see this whitelist doing is determining 32/64 bit, which there are definitely better ways to do than listing every possible architecture ever. Can I do a new PR implementing this instead?

First, I just merged this, so that we get the cargo.sh and install-build-tools.sh changes. Those are the most difficult for me to produce myself.

For generalizing target.h away from the allowlist approach, please see PR #2082.