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

llvmPackages_rocm.llvm: don't build shared libs #163698

Merged
merged 1 commit into from
Mar 22, 2022
Merged

llvmPackages_rocm.llvm: don't build shared libs #163698

merged 1 commit into from
Mar 22, 2022

Conversation

InternetUnexplorer
Copy link
Contributor

@InternetUnexplorer InternetUnexplorer commented Mar 11, 2022

Description of changes

This seems to fix the notorious "CommandLine Error: Option 'xxxxx' registered more than once!" error in applications that use both Mesa and ROCm.

Since Mesa is built with llvmPackages_latest and ROCm stuff is built with llvmPackages_rocm, applications that use both (such as Blender) end up with two different libLLVM*.sos loaded, which breaks things.

This seems like a straightforward way to fix the problem, and since the ROCm stack seems to be the only thing in Nixpkgs that uses llvmPackages_rocm this hopefully shouldn't break anything.

While there might be another way to fix this problem that doesn't require disabling the shared libraries, I haven't been able to find it yet, and since this issue seems to affect a lot of people I think it might make sense to merge this fix for now and revisit it later if a better solution is found.

This also removes a small patch to rocm-comgr since there are no longer LLVM shared libraries for it to link against.

Closes #97401, closes #157457.

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/)
  • 22.05 Release Notes (or backporting 21.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.

This seems to fix the notorious "CommandLine Error: Option 'xxxxx'
registered more than once!" error in applications that use both Mesa and
ROCm.

Since Mesa is built with llvmPackages_latest and ROCm stuff is built
with llvmPackages_rocm, applications that use both (such as Blender) end
up with two different `libLLVM*.so`s loaded, which breaks things.

This seems like a straightforward way to fix the problem, and since the
ROCm stack seems to be the only thing in Nixpkgs that uses
llvmPackages_rocm this hopefully shouldn't break anything.

While there might be another way to fix this problem that doesn't
require disabling the shared libraries, I haven't been able to find it
yet, and since this issue seems to affect a lot of people I think it
might make sense to merge this fix for now and revisit it later if a
better solution is found.

This also removes a small patch to rocm-comgr since there are no longer
LLVM shared libraries for it to link against.
@athas
Copy link
Contributor

athas commented Mar 15, 2022

I can confirm that this works for me and solves a very long-standing problem.

@Flakebi
Copy link
Member

Flakebi commented Mar 15, 2022

Nice! This looks like a good change.
It probably makes sense to link mesas llvm statically too, apparently there exist games that link to llvm, which then fails for the same reason (two different llvm versions, one from the game, one from mesa) (mentioned by a mesa developer here: https://www.jlekstrand.net/jason/blog/2022/01/in-defense-of-nir/).

@InternetUnexplorer
Copy link
Contributor Author

It probably makes sense to link mesas llvm statically too, apparently there exist games that link to llvm, which then fails for the same reason (two different llvm versions, one from the game, one from mesa) (mentioned by a mesa developer here: https://www.jlekstrand.net/jason/blog/2022/01/in-defense-of-nir/).

That's really interesting! I wouldn't really feel comfortable making that decision though, since a) it's used by a ton of people and b) I've personally never encountered any issues like that before. Do any other distros link it statically?

@athas
Copy link
Contributor

athas commented Mar 15, 2022

Linking LLVM statically seems like it would always be the right thing to do for libraries that expect to be loaded dynamically. LLVM seems very unwilling to share address space with other instances of itself. But I think that would be a separate change from this PR.

Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

ROCm still compiles and the OpenCL samples I tried still work.

Copy link
Contributor

@athas athas left a comment

Choose a reason for hiding this comment

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

I have tested that ROCm works after this and that processes can have both OpenGL and ROCm running simultaneously. This fixes a long-standing bug.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/787

@acowley acowley merged commit 29a988f into NixOS:master Mar 22, 2022
@acowley
Copy link
Contributor

acowley commented Mar 22, 2022

Wonderful, 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/blender-error-when-i-install-the-radeon-pro-render-plugin/16678/9

@InternetUnexplorer InternetUnexplorer deleted the rocm-llvm-disable-shared-libs branch March 22, 2022 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants