-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Clang link time optimization for darwin #19312
Conversation
…llvm that clang is for.
@dipinhora, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vcunat, @copumpkin and @edolstra to be potential reviewers. |
Please let me know if you have any questions regarding this change or if anything needs to be altered before it can be merged. |
@copumpkin Seeing as this changes If not, what is the process for getting this merged? Does this need to go into |
Yeah sorry for not being in touch. My plan was to merge this into staging this weekend with some other changes I was going to make, but staging is currently broken on Darwin so now I'm looking into that. I think it's generally fine though, thanks!
|
So I think that in principle it's fine, but in practice it complicates the stdenv bootstrap a bit. I know you made the changes needed to the bootstrap, but it leads to 3 complete builds of LLVM per stdenv build instead of 1 before this change. I'm hoping to make some changes to make it easier to tie together bootstraps, but for now I'd rather not merge at least the LLVM 3.7 changes because it'll make it a nightmare for me to rebuild the stdenv for the other changes I'm making (since LLVM builds are by far the majority of the stdenv build time this makes the whole thing a lot slower) |
@copumpkin Thanks for reviewing the changes. Much appreciated. I had noticed that 3 complete builds of llvm/clang were done but didn't do a before/after comparison so didn't realize it was due to my changes. The LLVM 3.7 changes at https://github.com/NixOS/nixpkgs/pull/19312/files#diff-cf6aa7f087ebdb6ee5061fdbfad9375aR31 and https://github.com/NixOS/nixpkgs/pull/19312/files#diff-cf6aa7f087ebdb6ee5061fdbfad9375aR41 that set the compatibility version are unfortunately required to make it all work because it's not possible to change the compatibility version that One way to avoid the llvm 3.7 changes would be if there was some way to change the compatibility version for |
Interesting. One thing I'd done a few weeks ago was upgrade the darwin stdenv to LLVM 3.8 but some strange things started happening and I didn't have time to figure them out so I undid it and reverted to 3.7. I don't really know what the best solution is here. My inclination would be to ask you to wait until we get the stdenv to 3.8 and/or the stdenv bootstrapping machinery rework (so we can minimize the LLVM rebuilds) but I don't know how urgently you need this. It doesn't seem like the LLVM stuff changes all that often so it doesn't feel like your PR will bitrot very quickly. How about in the next few days I start a Hydra build (against one of my custom branches) of a 3.8-based stdenv, and then we can see how that affects this? I'm fearful of putting 3.8 straight into staging because I want to have a full Hydra build to compare to see what we're breaking by upgrading. |
No rush. I'm currently using a fork of nixpkgs with the changes I need so it's not a blocker for me. I just wanted it in upstream so there's less reason for me to keep a fork and have to build pretty much everything from source instead of being able to rely on the Hydra caches. I think your plan to try a build of a 3.8 based stdenv and to switch over to it is a good one. Let me know if I can help in converting over to a 3.8 based stdenv. |
I'd assume that LTO is also broken on Linux and that it would need a (DY)LD_LIBRARY_PATH change in this PR. |
What is the status of this PR? |
I'll let some things build using this and see how it looks. |
The http://hydra.nixos.org/eval/1320568?compare=trunk
|
I've now merged an upgrade of the stdenv to 4.0, and plan to get LTO into that soon. Sorry it's taken me so long! |
Beep boop! |
Are there any updates on this pull request, please? |
I'm hitting what I think is this problem. I'd love to see an update on this PR. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Thank you for your contributions.
|
Needs #19312 or equivalent to work.
Still important to me, see 31579c6 |
There is a different PR (#46730), now merged, which claims to make LTO work on Darwin. There is even a comment pinging the original author of this PR. This suggests, that it's a replacement of this PR. But now, two years later, LTO is broken again and has been for at least one and a half years, see for example the workaround in #56740. |
Disabling Link Time Optimization is useful for Nix on Darwin because LTO is broken there and has been for a long time, see NixOS/nixpkgs#19312. This is currently worked around in the Nix package with a patch that removes the lines that add -flto to the compiler flags.
Needs NixOS#19312 or equivalent to work. (cherry picked from commit 31579c6)
LTO in general is broken on Darwin (see NixOS#19312).
I marked this as stale due to inactivity. → More info |
Closing as dead |
Motivation for this change
Clang link time optimization was not working on OSX.
Resolves #19098.
Trying to follow CONTRIBUTING.md (hopefully I didn't make too many mistakes):
cc @copumpkin (for cctool/darwin stdenv changes)
cc @lovek323 @raskin @viric (for clang 3.6/llvm 3.7/clang 3.7/clang 3.8/clang 3.9 changes)
cc @fpletz @globin (for cc-wrapper changes)
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)NOTE: Not all tests were successful but I don't believe the issues are caused by any changes in this PR but I am not sure because I didn't do a baseline of the tests without the changes first.
The following is are the results of the testing combinations as represented by checkboxes above:
Technical details
uname -a
:nix-env (Nix) 1.11.4
"17.03pre93030.548bcd0"
mac:
NIX_PATH=nixpkgs=/Users/dipinhora/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed
works fine
NIX_PATH=nixpkgs=/Users/dipinhora/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --option build-use-sandbox true
error:
NIX_PATH=nixpkgs=/Users/dipinhora/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --option build-use-sandbox relaxed
error: (known issue: NixOS/nix#759)
NIX_PATH=nixpkgs=/Users/dipinhora/nixpkgs/ nix-shell -p nox --run "nox-review wip" --pure --keep-failed
error:
When run a second time:
nixos: (without and with nix.useSandbox = true) (on aws: ubuntu/images/hvm-ssd/ubuntu-xenial-16.04-amd64-server-20160907.1 (ami-2ef48339))
NIX_PATH=nixpkgs=/root/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --max-jobs 8 --cores 8
works fine
NIX_PATH=nixpkgs=/root/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --max-jobs 8 --cores 8 --option build-use-sandbox true
works fine
NIX_PATH=nixpkgs=/root/nixpkgs/ nix-shell -p nox --run "nox-review wip" --pure --keep-failed --max-jobs 8 --cores 8
error:
ubuntu: (on aws: nixos-16.09.666.3738950-x86_64-hvm-ebs (ami-93347684))
NIX_PATH=nixpkgs=/home/ubuntu/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --max-jobs 8 --cores 8
works fine
NIX_PATH=nixpkgs=/home/ubuntu/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --option build-use-sandbox true
error:
NIX_PATH=nixpkgs=/home/ubuntu/nixpkgs/ nix-shell -p llvmPackages_38.clang --pure --keep-failed --option build-use-sandbox relaxed
error:
NIX_PATH=nixpkgs=/home/ubuntu/nixpkgs/ nix-shell -p nox --run "nox-review wip" --pure --keep-failed --max-jobs 8 --cores 8
error:
When run a second time: