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

llvmPackages_latest: 14 -> 15 #213202

Closed
wants to merge 9 commits into from
Closed

llvmPackages_latest: 14 -> 15 #213202

wants to merge 9 commits into from

Conversation

rrbutani
Copy link
Contributor

Description of changes

Follow up to #194634 that targets staging and bumps llvmPackages_latest.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • 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.

@rrbutani rrbutani mentioned this pull request Jan 28, 2023
Closed
48 tasks
@rrbutani
Copy link
Contributor Author

@vcunat Can we have a separate Hyrda jobset for this PR?

@wegank
Copy link
Member

wegank commented Jan 29, 2023

I don't think there should be as many rebuilds, since mesa now points to llvmPackages_15 in staging. Could you push the commit again?

@vcunat
Copy link
Member

vcunat commented Jan 30, 2023 •
edited
Loading

Oh, I didn't notice my subscription here was because of a mention.

This doesn't cause that many rebuilds, but it's based on some staging commit for which we don't have binaries, so the jobset would be rebuilding everything (most likely). What about basing this on current staging-next? For example, this particular commit and eval as comparison base: https://hydra.nixos.org/eval/1789916#tabs-inputs

EDIT: I forgot to write that mesa is switched at that point already.

@vcunat
Copy link
Member

vcunat commented Feb 1, 2023

Now mesa is using llvmPackages_15 in nixpkgs master already.

@vcunat vcunat changed the base branch from staging to master February 14, 2023 12:38
@vcunat
Copy link
Member

vcunat commented Feb 15, 2023 •
edited
Loading

OfBorg still showed around 4.7k rebuilds (total summed over all platforms).

Anyway, for fixing regressions I rebased on an OK master commit and here's Hydra's comparison: https://hydra.nixos.org/eval/1790712?compare=1790698 (*-linux only, at least for now)

EDIT: marked as "draft" to make it clear that this shouldn't be merged yet (and probably not to master anyway).

@vcunat vcunat marked this pull request as draft February 15, 2023 09:21
@rrbutani
Copy link
Contributor Author

It looks like a lot of the failures are coming from libclc; looks like some kind of LLVM version mismatch. I can take a look later today.

@vcunat is it okay if I hack on stuff on this branch for now? Not sure if the Hydra jobset was a one-off thing or if it'll trigger on updates to this PR.

@vcunat
Copy link
Member

vcunat commented Feb 15, 2023

Yes, I meant for this branch to keep getting fixes on top, and Hydra picking them up.

@rrbutani
Copy link
Contributor Author

rrbutani commented Feb 21, 2023 •
edited
Loading

Current status: 35 new failures.

Pretty much all of these (excluding timeouts) look like they're coming from swift which looks like its failing because of compiler lints (unused variables). It doesn't look like the lint in question is new to LLVM 15 so I'm not totally sure why we're only seeing this error now; will investigate.

EDIT: the variable in question and it's usage is here, Clang 15's -Wunused-but-set-variable was updated to include pre/post increment operators (commit here). It's unfortunate that the usual lint-suppressing (void)var doesn't seem to silence the lint here. I guess we can either just disable that lint entirely (-Wno-unused-but-set-variable in the compile flags) or patch the source to have this variable use __attribute__((unused)) for now.

EDIT: I misunderstood; the lint is firing on the first assignment of spins in the above (where _dispatch_preemption_yield expands to (void)++spins), (void)spins actually does fix this but was only added to upstream recently: swiftlang/swift-corelibs-libdispatch@915f251. We should just apply this patch.

@rrbutani
Copy link
Contributor Author

rrbutani commented Feb 21, 2023 •
edited
Loading

@vcunat I'm waiting on Hydra to confirm but I think all the new failures caused by this PR have been resolved.

How should we proceed from here? Should we have Hydra test *-darwin next or is it time to rebase and merge into staging, etc.

@vcunat
Copy link
Member

vcunat commented Apr 14, 2023

I could test Intel (Tiger Lake) if it doesn't need restart of X and someone tells me what to test.

@K900
Copy link
Contributor

K900 commented Apr 14, 2023

Basically anything involving OpenCL compute. Raytracing would also be nice but you don't have that.

@RaitoBezarius RaitoBezarius changed the base branch from master to staging April 14, 2023 14:59
RaitoBezarius pushed a commit that referenced this pull request Apr 14, 2023
@rrbutani @RaitoBezarius
swift: fix build with LLVM 15
4d30a7e 
See here for context: #213202 (comment)
rrbutani added 8 commits April 16, 2023 16:52
@rrbutani @RaitoBezarius
llvmPackages_latest: 14 -> 15
37250c5
@rrbutani @RaitoBezarius
libclc: use the same LLVM in spirv-llvm-translator as in the build
8197b8a
@rrbutani @RaitoBezarius
llvmPackages_git.openmp: apply NixOS#197674 (fix cross compile)
f49825c 
this was backported to `llvmPackages_15` in
81ef82a
@rrbutani @RaitoBezarius
libclc: move into llvmPackages
7ee7141 
this should let us avoid version mismatch between libclc and the LLVM
version it uses; i.e. trying to build libclc 15.0.6 with LLVM 14.x

note the TODO within; `llvmPackages_git.libclc` does not currently eval
@rrbutani @RaitoBezarius
spirv-llvm-translator: fix llvm-spirv binary on macOS
c99f1c8
@rrbutani @RaitoBezarius
spirv-llvm-translator: add a version test for llvm-spirv
96de861 
hopefully this will catch issues like the macOS rpath problem
@rrbutani @RaitoBezarius
llvmPackages_*.libclc: fix build on macOS
0ecb762
@rrbutani @RaitoBezarius
swift: fix build with LLVM 15
fc726d5 
See here for context: NixOS#213202 (comment)
@RaitoBezarius
swift: pin to llvmPackages_14
693a1b5 
Some swiftPackages cannot be built with LLVM15 on aarch64-darwin for
now:

> Latest run: https://hydra.nixos.org/eval/1792520?compare=1792488#tabs-now-fail

> It looks like all of the remaining (38) failures can be traced back to:

> swiftPackages.XCTest on aarch64-darwin (seems to have timed out but there's no log output..)
> edit: when building locally this (or swift anyways) does fail to build
> swiftPackages.swift-unwrapped on x86_64-darwin: log here, missing intrinsics for __builtin_elementwise_add_sat/__builtin_elementwise_sub_sat
@RaitoBezarius
Copy link
Member

Swift pinned to LLVM14, testing the OpenCL stuff and merging into staging if it's okay with everyone?

@RaitoBezarius RaitoBezarius marked this pull request as ready for review April 16, 2023 14:59
@RaitoBezarius
Copy link
Member

We decided to wait probably past 23.05 for this PR, it could be eventually backported if we justify security benefits or important fixes.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 16, 2023
@rrbutani rrbutani mentioned this pull request May 4, 2023
Merged
12 tasks
@rrbutani
Copy link
Contributor Author

rrbutani commented May 4, 2023

Assuming #229852 goes through I'll update this PR to bump the now pinned LLVM version in all the packages that successfully built with llvmPackages_latest = llvmPackages_15.

There are also a few extant changes to the LLVM package sets on this branch that we do actually want (and that should be applied to LLVM 16).

@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 10, 2023
@reckenrode
Copy link
Contributor

reckenrode commented May 27, 2023 •
edited
Loading

After this is merged, is it planned to bump llvmPackages from 11 to 15 on Linux?

Or to put it another way, what should Darwin be doing to track this? The stdenv currently hardcodes a version (11.1.0). #229786 decouples the version used in the stdenv from the one in the bootstrap LLVM, so it should be able to track the latest more easily. I’m implementing the bump by updating llvmPackages on Darwin, but I’d like to follow whatever the recommended pattern is for tracking the latest LLVM if that is intended to stay at the minimally supported version.

@wegank
Copy link
Member

wegank commented May 27, 2023

By the way, the title is no longer accurate: llvmPackages_latest is already at 16.

@reckenrode
Copy link
Contributor

By the way, the title is no longer accurate: llvmPackages_latest is already at 16.

Thanks. Is this issue still applicable? Is the scope changing to something else now that llvmPackages_latest is updated?

I talked with @alyssais and @toonn on Matrix about what to do for Darwin. The approach I’d like to take is to use the value of llvmPackages_lateset at the time of the bump. I understand now that tracking llvmPackages_latest directly would be a bad idea. I assume it would be prudent to wait on LLVM to finish releasing the scheduled patch releases to keep churn down and avoid imposing a requirement the updates go through staging because of Darwin (though it may not matter by the time the requisite PRs for #234710 are merged).

@alyssais alyssais closed this Nov 1, 2023
@Et7f3
Copy link
Contributor

Et7f3 commented Nov 12, 2023

The jobsets is still present and not disabled in Hydra

@vcunat
Copy link
Member

vcunat commented Nov 12, 2023

OK, thanks. I cleaned that up now. (disabled + hidden; hydra doesn't really do deletions)

@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
@rrbutani rrbutani mentioned this pull request May 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@inclyc inclyc inclyc approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@sternenseemann sternenseemann Awaiting requested review from sternenseemann

@lovek323 lovek323 Awaiting requested review from lovek323

@dtzWill dtzWill Awaiting requested review from dtzWill

@primeos primeos Awaiting requested review from primeos

@gloaming gloaming Awaiting requested review from gloaming

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 2501-5000
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@rrbutani @wegank @vcunat @yu-re-ka @RaitoBezarius @K900 @reckenrode @Et7f3 @inclyc @alyssais