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

pkgs/top-level/haskell-packages.nix: llvm 12 -> llvm 15 #379039

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

RossComputerGuy
Copy link
Member

@RossComputerGuy RossComputerGuy commented Feb 3, 2025

Things done

Moves Haskell away from LLVM 12. #305146

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@maralorn
Copy link
Member

maralorn commented Feb 3, 2025

I am confused. If this works, why haven’t we done this before?

@sternenseemann
Copy link
Member

It is not supported. So far this has been a pure possibility we've entertained. Has anything been done to confirm that this is indeed possible? The burden of proof is on our side, as upstream doesn't claim this would work.

@RossComputerGuy
Copy link
Member Author

I built shellcheck using this PR and that seems to work just fine. This is going to staging so maybe we push into there and if there's too many failures then we just revert? I'm not familiar with Haskell stuff so I'm not entirely sure what or how we should be testing.

@sternenseemann
Copy link
Member

shellcheck uses GHC 9.6 which isn't even changed by this PR. Also note that LLVM isn't necessarily used on many platforms. Most notable use of the LLVM backend is aarch64 for GHC < 9.2. The problem is that regressions will likely occur in edge cases for which we don't have CI coverage (e.g. cross compilation to platforms that don't have native code generation (NCG) in GHC and there isn't even full CI coverage for e.g. GHC 8.10.7 aarch64).

I suspect we'll have to review the upstream code gen backend changes for LLVM between GHC versions to figure out whether it is feasible. I unfortunately don't have time for that particular adventure at the moment. (Already making extremely slow progress on #371032 which is arguably higher priority at the moment.)

@RossComputerGuy
Copy link
Member Author

shellcheck uses GHC 9.6 which isn't even changed by this PR.

Then why did GHC 9.6 stuff rebuild?

Also note that LLVM isn't necessarily used on many platforms. Most notable use of the LLVM backend is aarch64 for GHC < 9.2.

I did build and test on aarch64 but didn't see any problems so that's good at least. I'm just not sure what uses GHC < 9.2.

@sternenseemann
Copy link
Member

Then why did GHC 9.6 stuff rebuild?

You did touch ghc924Binary which is used to bootstrap GHC. The default useLLVM flag in that expression is actually not quite correct, so it pulls in LLVM unnecessarily.

I did build and test on aarch64 but didn't see any problems so that's good at least. I'm just not sure what uses GHC < 9.2.

Neither 9.2.4 nor 9.6.6 use the LLVM backend for aarch64-linux.

@RossComputerGuy
Copy link
Member Author

You did touch ghc924Binary which is used to bootstrap GHC. The default useLLVM flag in that expression is actually not quite correct, so it pulls in LLVM unnecessarily.

Gotcha

Neither 9.2.4 nor 9.6.6 use the LLVM backend for aarch64-linux.

So what GHC < 9.2 uses the LLVM backend on aarch64-linux and which packages are used by that version? I'd like to know so I can properly test it.

@sternenseemann
Copy link
Member

So what GHC < 9.2 uses the LLVM backend on aarch64-linux and which packages are used by that version? I'd like to know so I can properly test it.

I've just checked and remaining are

  • haskell.compiler.ghcjs810 which does have downstream users, though it's a bit of a second class citizen in CI because we can't build it on Hydra due to output size limitations. We do test haskell.compiler.ghcjs810.bootGhcjs.
  • elmPackages has various packages which internally use GHC 8.10.
  • 9.0.2 is not used a lot, except for bootstrapping other GHCs. I'm planning on dropping it soon since it can be replaced with 8.10.7 for bootstrapping purposes.
  • 8.10.7 is still heavily used for bootstrapping other GHC versions. Unfortunately we either need 9.0.2 or 8.10.7 to bootstrap 9.4.*.
  • 9.2.4 is used to bootstrap some GHC versions. Can probably be replaced by 9.4.8 or so.

Bootstrapping relations can be seen from haskell-packages.nix and release-haskell.nix shows some packages we test on the affected versions (see versionedCompilerJobs).

Also, note that the GHC configure script checks that the LLVM version is below a certain number, so you'll probably have to patch that somehow.

@sternenseemann
Copy link
Member

A cursory look at the commit history of configure.ac turned up this: https://gitlab.haskell.org/ghc/ghc/-/commit/0cc16aaf89d7dc3963764b7193ceac73e4e3329b. We'll probably need to backport at least that (which looks simple enough). No clue about the actual codegen yet.

May also be sensible to figure out how to run the GHC test suite in the derivation if we are going to do this…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants