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

LLVM: fix cross compilation (llvm_6 & llvm_7) #52031

Merged
merged 2 commits into from
Dec 14, 2018
Merged

LLVM: fix cross compilation (llvm_6 & llvm_7) #52031

merged 2 commits into from
Dec 14, 2018

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Dec 14, 2018 •
edited
Loading

Motivation for this change

road to gtk

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Mic92 Mic92 requested a review from matthewbauer as a code owner December 14, 2018 11:38
@Mic92 Mic92 requested a review from Ericson2314 December 14, 2018 11:38
Mic92
Mic92 commented Dec 14, 2018
View reviewed changes
pkgs/development/compilers/llvm/7/llvm.nix
, debugVersion ? false
, enableManpages ? false
, enableSharedLibraries ? true
, targets ? [ stdenv.hostPlatform stdenv.targetPlatform ]
Copy link
Member Author

Choose a reason for hiding this comment

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

@Ericson2314 changed the name as it might be ambiguous when somebody adds a targets package.

Mic92
Mic92 commented Dec 14, 2018
View reviewed changes
pkgs/development/compilers/llvm/7/llvm.nix
, debugVersion ? false
, enableManpages ? false
, enableSharedLibraries ? true
, targets ? [ stdenv.hostPlatform stdenv.targetPlatform ]
, enableWasm ? true # TODO fold this into `targets` somehow
Copy link
Member Author

Choose a reason for hiding this comment

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

I did not want to have this as an API, because it should be in targets once it leaves the experimental stage.

Ericson2314
Ericson2314 approved these changes Dec 14, 2018
View reviewed changes
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Looks great!

@Ericson2314 Ericson2314 merged commit 4ccba8b into NixOS:staging Dec 14, 2018
@Mic92 Mic92 deleted the cross-llvm branch December 14, 2018 15:15
@Mic92 Mic92 mentioned this pull request Dec 19, 2018
Closed
@jtojnar jtojnar mentioned this pull request Dec 24, 2018
Merged
10 tasks
jtojnar added a commit to jtojnar/nixpkgs that referenced this pull request Dec 24, 2018
@jtojnar
mesa_noglu: fix build
ff22d90 
LLVM’s `LLVM_TARGETS_TO_BUILD` build flag defauls to `all`, which contains
`AMDGPU` among others. [1] Changes in llvm [2] switched to explicitly listing
host and target platforms, excluding the AMDGPU target, which is required
for Mesa to build.

[1]: https://github.com/llvm-mirror/llvm/blob/db50b6fe399b8ad8caef80fd8ee77607fb051fa5/CMakeLists.txt#L286-L302
[2]: NixOS#52031
@dtzWill
Copy link
Member

dtzWill commented Jan 14, 2019

cc #53920 -- thoughts? :)

@orivej
Copy link
Contributor

orivej commented Jan 14, 2019

Is there a reason not to build all targets by default?

@Mic92
Copy link
Member Author

Mic92 commented Jan 14, 2019 •
edited
Loading

@orivej closure size maybe? My intention was that it is also used for jit-compiling like in mesa and for cross-compiling we would have to rebuild it anyway as the default target changes. I have not measured the size difference though.

@orivej orivej mentioned this pull request Jan 14, 2019
Merged
10 tasks
@andrewrk
Copy link
Member

andrewrk commented Feb 7, 2019 •
edited
Loading

This commit broke Zig, which depends on all LLVM targets being enabled. LLVM should include all default targets by default.

@andrewrk andrewrk mentioned this pull request Feb 7, 2019
Closed
@andrewrk
Copy link
Member

andrewrk commented Feb 7, 2019

Never mind, I think this was fixed. Sorry for the noise.

@corngood corngood mentioned this pull request Jan 3, 2020
Merged
10 tasks
@corngood
Copy link
Contributor

corngood commented Jan 3, 2020

@Mic92 @Ericson2314 looks like _8 and _9 have been broken since they were added. Could you take a look at #76887?

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

@Ericson2314 Ericson2314 Ericson2314 approved these changes

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Mic92 @dtzWill @orivej @andrewrk @corngood @Ericson2314 @GrahamcOfBorg