LLVM 23: Run AssignGUIDPass in some places#157055
Merged
Merged
Conversation
rustbot
added
A-LLVM
Collaborator
|
r? @cuviper rustbot has assigned @cuviper. Use Why was this reviewer chosen?The reviewer was selected based on:
|
rustbot
added
the
llvm-main
rustbot
added
S-waiting-on-author
Contributor
Author
|
Some FileCheck tests also need to be updated since the generated IR can now include |
Collaborator
|
Some changes occurred in tests/codegen-llvm/sanitizer cc @rcvalle |
rustbot
added
S-waiting-on-review
Contributor
|
This is a little over my head in some ways, but the patch matches what I naively expected from looking at the LLVM change. |
Member
|
I'm not 100% sure either, but it looks ok -- and we can always follow up later. :) @bors r+ rollup |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 1, 2026
…uwer Rollup of 13 pull requests Successful merges: - #156085 (miri: require (almost) all 1-ZST arguments to be actually passed) - #155193 (Check arguments of attributes where no arguments are expected) - #156516 (nix: remove some unneeded variables) - #156562 (Resolving Windows environment test failures) - #156588 (Don't drop uninit memory when `MapWindows::clone` panics) - #156673 (Privacy: small cleanups) - #156817 (Add `#[unsafe_eii]` to unsafe EII UI tests) - #156924 (Use #[panic_handler] rather than #[lang = "panic_impl"]) - #157055 (LLVM 23: Run AssignGUIDPass in some places) - #157108 (Add Xtensa va_arg assembly coverage) - #157220 (cg_ssa: a bit less `immediate_or_packed_pair`) - #157241 (Trace `?id.local_def_index` instead of `id` in `def_path_hash`) - #157242 (Tune backport Zulip messages)
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 1, 2026
…uwer Rollup of 13 pull requests Successful merges: - #156085 (miri: require (almost) all 1-ZST arguments to be actually passed) - #155193 (Check arguments of attributes where no arguments are expected) - #156516 (nix: remove some unneeded variables) - #156562 (Resolving Windows environment test failures) - #156588 (Don't drop uninit memory when `MapWindows::clone` panics) - #156673 (Privacy: small cleanups) - #156817 (Add `#[unsafe_eii]` to unsafe EII UI tests) - #156924 (Use #[panic_handler] rather than #[lang = "panic_impl"]) - #157055 (LLVM 23: Run AssignGUIDPass in some places) - #157108 (Add Xtensa va_arg assembly coverage) - #157220 (cg_ssa: a bit less `immediate_or_packed_pair`) - #157241 (Trace `?id.local_def_index` instead of `id` in `def_path_hash`) - #157242 (Tune backport Zulip messages)
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 1, 2026
…uwer Rollup of 13 pull requests Successful merges: - #156085 (miri: require (almost) all 1-ZST arguments to be actually passed) - #155193 (Check arguments of attributes where no arguments are expected) - #156516 (nix: remove some unneeded variables) - #156562 (Resolving Windows environment test failures) - #156588 (Don't drop uninit memory when `MapWindows::clone` panics) - #156673 (Privacy: small cleanups) - #156817 (Add `#[unsafe_eii]` to unsafe EII UI tests) - #156924 (Use #[panic_handler] rather than #[lang = "panic_impl"]) - #157055 (LLVM 23: Run AssignGUIDPass in some places) - #157108 (Add Xtensa va_arg assembly coverage) - #157220 (cg_ssa: a bit less `immediate_or_packed_pair`) - #157241 (Trace `?id.local_def_index` instead of `id` in `def_path_hash`) - #157242 (Tune backport Zulip messages)
rust-timer
added a commit
that referenced
this pull request
Jun 2, 2026
Rollup merge of #157055 - TimNN:guid, r=cuviper LLVM 23: Run AssignGUIDPass in some places llvm/llvm-project#184065 introduced the new pass. I'm not 100% confident that this is the correct patch, but it makes all the previously failing `run-make` "lto" tests pass on my machine. I added the pass to the `if (NeedThinLTOBufferPasses)` section to match the `addRequiredLTOPreLinkPasses` changes from the LLVM PR, and to the `LLVMRustModuleSerialize` section based on the stack trace of an LLVM assertion. The IR may now contain an additional `!guid` attribute, requiring some codegen test expectations to be updated. I mostly followed the pattern from the LLVM PR, and removed trailing `{`s. (For both, tests that were failing locally, as well as potentially problematic expectations identified via grep, in case the test wasn't being run locally). @rustbot label llvm-main
pull Bot
pushed a commit
to xtqqczze/rust-lang-miri
that referenced
this pull request
Jun 2, 2026
…uwer Rollup of 13 pull requests Successful merges: - rust-lang/rust#156085 (miri: require (almost) all 1-ZST arguments to be actually passed) - rust-lang/rust#155193 (Check arguments of attributes where no arguments are expected) - rust-lang/rust#156516 (nix: remove some unneeded variables) - rust-lang/rust#156562 (Resolving Windows environment test failures) - rust-lang/rust#156588 (Don't drop uninit memory when `MapWindows::clone` panics) - rust-lang/rust#156673 (Privacy: small cleanups) - rust-lang/rust#156817 (Add `#[unsafe_eii]` to unsafe EII UI tests) - rust-lang/rust#156924 (Use #[panic_handler] rather than #[lang = "panic_impl"]) - rust-lang/rust#157055 (LLVM 23: Run AssignGUIDPass in some places) - rust-lang/rust#157108 (Add Xtensa va_arg assembly coverage) - rust-lang/rust#157220 (cg_ssa: a bit less `immediate_or_packed_pair`) - rust-lang/rust#157241 (Trace `?id.local_def_index` instead of `id` in `def_path_hash`) - rust-lang/rust#157242 (Tune backport Zulip messages)
Contributor
|
The LLVM-side patch was reverted in llvm/llvm-project#201194 Should we revert this, or wait for the LLVM-side patch to come back? The builds will be broken in the meantime :/ E.g. https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/46237 |
Contributor
Author
|
Guess I didn't wait long enough 🙄 (I was watching the LLVM PR to see whether it would get rolled back…) For https://buildkite.com/llvm-project/rust-llvm-integrate-prototype at least, we could just post a draft PR for now with the Depends a bit on how quickly we expect this to be rolled forwards again. |
Contributor
No, getting this fix in early was good since it unblocked us.
Sounds good to me: #157365 |
Member
|
Is LLVM patch expected to reland soon? Otherwise, we will have to proceed with the revert. |
Contributor
|
I'm sure they're working on it, but I don't know if there's an ETA. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
llvm/llvm-project#184065 introduced the new pass.
I'm not 100% confident that this is the correct patch, but it makes all the previously failing
run-make"lto" tests pass on my machine.I added the pass to the
if (NeedThinLTOBufferPasses)section to match theaddRequiredLTOPreLinkPasseschanges from the LLVM PR, and to theLLVMRustModuleSerializesection based on the stack trace of an LLVM assertion.The IR may now contain an additional
!guidattribute, requiring some codegen test expectations to be updated. I mostly followed the pattern from the LLVM PR, and removed trailing{s. (For both, tests that were failing locally, as well as potentially problematic expectations identified via grep, in case the test wasn't being run locally).@rustbot label llvm-main