Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Conversation

@eddyb
Copy link
Contributor

@eddyb eddyb commented Mar 29, 2022

Before this PR, spirv-builder depended on rustc_codegen_spirv not just in Cargo.toml, but also used some of its exports, which meant rustc was linking anything using spirv-builder (such as our example runners, that support hot reloading) against librustc_codegen_spirv.so.

The problem with that is that kind of dylib dependency is rustc_codegen_spirv depends on rustc_driver which depends on rustc_codegen_llvm which links (statically or dynamically) against LLVM, so LLVM's global ctors run.

And then the Vulkan driver also gets loaded, and if it uses LLVM, it will also run LLVM global ctors.
Depending on dynamic linking/loading semantics, the LLVMs may get partially deduplicated and conflicts occur:

: CommandLine Error: Option 'use-dbg-addr' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options

This problem is the same as the one mentioned in the recent blog post on Mesa's NIR:

Third is that some games actually link against LLVM and, historically, LLVM hasn’t done well with two different versions of it loaded at the same time. Some of this is LLVM and some of it is the way C++ shared library loading is handled on Linux.


This PR moves the couple definitions spirv-builder needs, out of rustc_codegen_spirv, and into a new rustc_codegen_spirv-types crate. That allows to avoid linking against any dylibs that would bring LLVM in.

That makes example-runner-wgpu work again on Mesa's AMD Radeon Vulkan (RADV) driver, on Mesa 21.3.7 (though I am not entirely sure if the original breaking change was in Mesa, distro packaging, or spirv-builder).

Also, for the GPU I was testing with (reported by RADV as "FIJI"), I've had to also drive-by halving of max_push_constant_size, to match the maxPushConstantsSize = 128 reported by the driver.

@eddyb eddyb requested a review from VZout as a code owner March 29, 2022 16:18
Copy link
Contributor

@repi repi left a comment

Choose a reason for hiding this comment

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

ouch, good catch. sounds like a reasonable solution.

removing viktor from review as he is out on vacation this week.

@repi repi removed the request for review from VZout March 29, 2022 19:21
@hrydgard hrydgard merged commit 5ac500d into EmbarkStudios:main Mar 30, 2022
@eddyb eddyb deleted the llvm-conflict branch March 31, 2022 08:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants