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.

Sign up for GitHub

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

Jump to bottom

compiler-rt: fix build on ARMv6 #205176

Merged
merged 1 commit into from
Mar 13, 2023
Merged

compiler-rt: fix build on ARMv6 #205176

merged 1 commit into from
Mar 13, 2023

Conversation

lopsided98
Copy link
Contributor

Description of changes

compiler-rt has accumulated several regressions that prevent it from building on ARMv6. It is important to note that there are two major versions of ARMv6: base ARMv6 and ARMv6K. ARMv6K includes several important new instructions, such as non-word size atomic operations (ldrexd, strexd, etc.) and the yield instruction. Most ARMv6 CPUs actually implement ARMv6K, including all those used in Raspberry Pis, but nixpkgs' "raspberryPi" platform targets base ARMv6.

compiler-rt versions 8-14 fail to build on ARMv6 and ARMv6K. compiler-rt 15 (not yet in nixpkgs) builds on ARMv6K but not ARMv6. This patch fixes versions 9-14 on both ARMv6 variants. The patches don't apply cleanly to version 8, and I figured it wasn't worth carrying another version of the patches for such an old version.

A total of five patches are required to get compiler-rt building on ARMv6:

  • armv6-mcr-dmb.patch: use mcr to provide the equivalent of dmb on ARMv6. Included in LLVM 15.
  • armv6-sync-ops-no-thumb.patch: prevent certain atomic operation functions from using Thumb mode. Included in LLVM 15.
  • armv6-no-ldrexd-strexd.patch: don't use ldrexd or strexd, which are not available in base ARMv6. Submitted upstream by me.
  • armv6-scudo-no-yield.patch: use nop instead of yield on ARMv6 in standalone scudo. Required by versions >=13, since they enable standalone scudo. Submitted upstream by me.
  • armv6-scudo-libatomic.patch: link standlone scudo to libatomic on ARMv6 (and any other platforms that need it). Not yet submitted because the backport is a bit different from the upstream version and I need to test it.

This PR also adds the armv6k architecture to lib.systems. This makes it possible to build nixpkgs for ARMv6K.

cc @samueldr @rnhmjoj

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • armv6l-linux (cross)
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added the 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild label Dec 8, 2022
@samueldr
Copy link
Member

samueldr commented Dec 8, 2022 •
edited
Loading

Mostly off-topic, but anyone who is tempted to just change the raspberryPi platform to armv6k in Nixpkgs, it's probably not the right thing to do. We probably want to deprecate all "product-named" platforms entirely, and use the proper generic names.

As for reviewing the changes: I don't know enough about the details of compilers to judge.

@lopsided98
Copy link
Contributor Author

I updated armv6-no-ldrexd-strexd.patch based on upstream feedback.

@alyssais
Copy link
Member

This PR also adds the armv6k architecture to lib.systems. This makes it possible to build nixpkgs for ARMv6K.

This was already possible before, with { system = "armv6l-linux"; gcc.arch = "armv6k"; }, but if you say it makes sense to treat as a separate architecture, that's fine with me.

alyssais
alyssais approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

This is great, thanks! I notice some of your patches have been accepted upstream. Would it be possible to fetchpatch them from the LLVM repo? This makes it clearer that they're backports and so would be less scary to people looking at the code and seeing assembly patches. (But if they don't apply and need to be in the Nixpkgs repo, that's fine. A note of their upstream commit hash as well as the differential revision would be nice.)

@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Feb 1, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/building-libcamera-for-raspberry-pi/26133/3

@lopsided98
Copy link
Contributor Author

This was already possible before, with { system = "armv6l-linux"; gcc.arch = "armv6k"; }, but if you say it makes sense to treat as a separate architecture, that's fine with me.

It seems that change is only needed if you want to use a target triple like armv6k-unknown-linux-gnueabihf. I think I tried this at one point, but I don't remember if it broke something or there was just no real benefit to using it.

@lopsided98
compiler-rt: fix build on ARMv6
98ebcd2 
compiler-rt has accumulated several regressions that prevent it from building
on ARMv6. It is important to note that there are two major versions of ARMv6:
base ARMv6 and ARMv6K. ARMv6K includes several important new instructions,
such as non-word size atomic operations (ldrexd, strexd, etc.) and the yield
instruction. Most ARMv6 CPUs actually implement ARMv6K, including all those used
in Raspberry Pis, but nixpkgs' "raspberryPi" platform targets base ARMv6.

compiler-rt versions 8-14 fail to build on ARMv6 and ARMv6K. compiler-rt 15 (not
yet in nixpkgs) builds on ARMv6K but not ARMv6. This patch fixes versions 9-14
on both ARMv6 variants. The patches don't apply cleanly to version 8, and I
figured it wasn't worth carrying another version of the patches for such an old
version.

A total of five patches are required to get compiler-rt building on ARMv6:
* armv6-mcr-dmb.patch: use `mcr` to provide the equivalent of `dmb` on ARMv6.
  Included in LLVM 15.
* armv6-sync-ops-no-thumb.patch: prevent certain atomic operation functions
  from using Thumb mode. Included in LLVM 15.
* armv6-no-ldrexd-strexd.patch: don't use ldrexd or strexd, which are not
  available in base ARMv6. Submitted upstream by me.
* armv6-scudo-no-yield.patch: use nop instead of yield on ARMv6 in standalone
  scudo. Required by versions >=13, since they enable standalone scudo.
  Submitted upstream by me.
* armv6-scudo-libatomic.patch: link standlone scudo to libatomic on ARMv6 (and
  any other platforms that need it). Not yet submitted because the backport is
  a bit different from the upstream version and I need to test it.
@ofborg ofborg bot requested a review from sternenseemann March 8, 2023 19:52
@eliasnaur
Copy link
Contributor

I can confirm this PR fixes the cross building of libcamera for Raspberry Pi. Thank you.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/zbar-for-raspberry-pi-zero/26151/1

@7c6f434c 7c6f434c merged commit b66eafc into NixOS:staging Mar 13, 2023
@eliasnaur eliasnaur mentioned this pull request Mar 31, 2023
Closed
@lopsided98 lopsided98 deleted the compiler-rt-armv6 branch August 29, 2023 03:50
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixos-on-raspberry-pi-zero-w/38018/11

@tie tie mentioned this pull request Jul 17, 2024
Merged
13 tasks
@symphorien symphorien mentioned this pull request Aug 11, 2024
Merged
13 tasks
symphorien added a commit to symphorien/nixpkgs that referenced this pull request Sep 14, 2024
@symphorien
compiler-rt: apply armv6l patches to llvm >= 15 where applicable
7b6139f 
the patches were written for
NixOS#205176 but not kept for llvm >= 15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alyssais alyssais alyssais approved these changes

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@nbp nbp Awaiting requested review from nbp

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@np np Awaiting requested review from np

@adevress adevress Awaiting requested review from adevress

@dtzWill dtzWill Awaiting requested review from dtzWill

@edolstra edolstra Awaiting requested review from edolstra

@7c6f434c 7c6f434c Awaiting requested review from 7c6f434c

@primeos primeos Awaiting requested review from primeos

@lovek323 lovek323 Awaiting requested review from lovek323

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

@sternenseemann sternenseemann Awaiting requested review from sternenseemann

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@lopsided98 @samueldr @alyssais @nixos-discourse @eliasnaur @mweinelt @7c6f434c