-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
llvm-wrapper: adapt for LLVM API changes #129749
Conversation
Updates the wrapper for 5c4lar/llvm-project@21eddfa.
@@ -1205,7 +1205,11 @@ struct LLVMRustThinLTOData { | |||
// Not 100% sure what these are, but they impact what's internalized and | |||
// what's inlined across modules, I believe. | |||
#if LLVM_VERSION_GE(18, 0) | |||
#if LLVM_VERSION_GE(20, 0) | |||
FunctionImporter::ImportListsTy ImportLists; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need this conditional. On older versions you can use the typedef as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the typedef FunctionImporter::ImportListsTy
is new as well: llvm/llvm-project@4f15039#diff-870a2d0fa51c9349d66da88fd1938cef621de9b4ac0c96d71d694b90dbefbfebR153.
@bors r+ rollup |
llvm-wrapper: adapt for LLVM API changes No functional changes intended. Updates the wrapper for 5c4lar/llvm-project@21eddfa. `@rustbot` label: +llvm-main r? `@nikic`
…kingjubilee Rollup of 16 pull requests Successful merges: - rust-lang#129123 (rustdoc-json: Add test for `Self` type) - rust-lang#129675 (allow BufReader::peek to be called on unsized types) - rust-lang#129678 (Deny imports of `rustc_type_ir::inherent` outside of type ir + new trait solver) - rust-lang#129723 (Simplify some extern providers) - rust-lang#129724 (Remove `Option<!>` return types.) - rust-lang#129725 (Stop using `ty::GenericPredicates` for non-predicates_of queries) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129762 (Update the `wasm-component-ld` binary dependency) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129780 (add crashtests for several old unfixed ICEs) - rust-lang#129782 (couple more crash tests) - rust-lang#129791 (mark joboet as on vacation) r? `@ghost` `@rustbot` modify labels: rollup
llvm-wrapper: adapt for LLVM API changes No functional changes intended. Updates the wrapper for 5c4lar/llvm-project@21eddfa. ``@rustbot`` label: +llvm-main r? ``@nikic``
llvm-wrapper: adapt for LLVM API changes No functional changes intended. Updates the wrapper for 5c4lar/llvm-project@21eddfa. ```@rustbot``` label: +llvm-main r? ```@nikic```
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129711 (Expand NLL MIR dumps) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129733 (Subtree update of `rust-analyzer`) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129785 (Miri subtree update) - rust-lang#129791 (mark joboet as on vacation) Failed merges: - rust-lang#129777 (Add `unreachable_pub`, round 4) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#129659 (const fn stability checking: also check declared language features) - rust-lang#129711 (Expand NLL MIR dumps) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129733 (Subtree update of `rust-analyzer`) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129785 (Miri subtree update) - rust-lang#129791 (mark joboet as on vacation) - rust-lang#129812 (interpret, codegen: tweak some comments and checks regarding Box with custom allocator) Failed merges: - rust-lang#129777 (Add `unreachable_pub`, round 4) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#129659 (const fn stability checking: also check declared language features) - rust-lang#129711 (Expand NLL MIR dumps) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129733 (Subtree update of `rust-analyzer`) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129785 (Miri subtree update) - rust-lang#129791 (mark joboet as on vacation) - rust-lang#129812 (interpret, codegen: tweak some comments and checks regarding Box with custom allocator) Failed merges: - rust-lang#129777 (Add `unreachable_pub`, round 4) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#129749 - krasimirgg:llvm-20-lto, r=nikic llvm-wrapper: adapt for LLVM API changes No functional changes intended. Updates the wrapper for 5c4lar/llvm-project@21eddfa. ````@rustbot```` label: +llvm-main r? ````@nikic````
This PR causes an assertion failure in one of our builds, using the default rustc invocation.
Backtrace
Unfortunately I don't have the exact commit used to produce the above backtrace, but it seems very likely that the assertion is firing from the first use of |
@@ -1414,13 +1418,13 @@ LLVMRustPrepareThinLTOInternalize(const LLVMRustThinLTOData *Data, | |||
return true; | |||
} | |||
|
|||
extern "C" bool LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, | |||
extern "C" bool LLVMRustPrepareThinLTOImport(LLVMRustThinLTOData *Data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem here is that this const
removal did not get propagated to ffi.rs
-- if it did, we'd probably get a borrow checker error :)
It looks like this function is called from optimize_thin_module, which I believe is run in parallel, so we might end up resizing the map while another thread holds a reference to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can't really fix this without LLVM exposing an additional API. I left a comment on the PR at llvm/llvm-project#106427 (comment).
Revert rust-lang#129749 to fix segfault in LLVM This reverts commit 8c7a7e3, reversing changes made to a00bd75. Reported in rust-lang#129749 (comment). `@nikic's` theory is that the LLVM API changed in a way that makes it impossible to use concurrently from multiple threads (llvm/llvm-project#106427 (comment)). I pinged `@krasimirgg` who was fine with reverting. r? `@rust-lang/wg-llvm`
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#129477 (Fix fluent diagnostics) - rust-lang#129674 (Add new_cyclic_in for Rc and Arc) - rust-lang#130452 (Update Trusty target maintainers) - rust-lang#130467 (Miri subtree update) - rust-lang#130477 (Revert rust-lang#129749 to fix segfault in LLVM) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130477 - tmandry:revert-llvm-20-lto, r=tmandry Revert rust-lang#129749 to fix segfault in LLVM This reverts commit 8c7a7e3, reversing changes made to a00bd75. Reported in rust-lang#129749 (comment). `@nikic's` theory is that the LLVM API changed in a way that makes it impossible to use concurrently from multiple threads (llvm/llvm-project#106427 (comment)). I pinged `@krasimirgg` who was fine with reverting. r? `@rust-lang/wg-llvm`
With llvm/llvm-project#109036 (comment) merged we should be able to keep using lookup() and only do the type change. Now the remaining question here is why we have modules for which seemingly no import list was computed. Based on some debug output during the libstd build we see something like this: https://gist.github.com/nikic/3f257e2a09b729ccb4b64c42446fa0de In other words, these are all modules that are either completely empty or only contain module-level inline assembly. If the module doesn't define functions, then no ImportList for it will be populated. While Rust probably shouldn't be submitting completely empty modules to LLVM (where do these come from...?) I don't think there's a fundamental bug here on the Rust side. |
Ah, it looks like the reason why this problem doesn't occur on the LLVM side is this code: https://github.com/llvm/llvm-project/blob/0dd56858fe188419182a57d0e03c8cd0aa693867/llvm/lib/LTO/ThinLTOCodeGenerator.cpp#L1102-L1111 It forces an entry to be created in all maps, even if that would normally not happen. |
Working on new version of this patch with lookup-s(). |
llvm-wrapper: adapt for LLVM API changes, second try This is a re-work of rust-lang#129749 after LLVM brought back the APIs used by rust. No functional changes intended. `@rustbot` label: +llvm-main r? `@nikic` cc: `@tmandry`
Rollup merge of rust-lang#130509 - krasimirgg:llvm-20-2, r=nikic llvm-wrapper: adapt for LLVM API changes, second try This is a re-work of rust-lang#129749 after LLVM brought back the APIs used by rust. No functional changes intended. `@rustbot` label: +llvm-main r? `@nikic` cc: `@tmandry`
[beta] backports - Don't warn empty branches unreachable for now rust-lang#129103 - Win: Add dbghelp to the list of import libraries rust-lang#130047 - `RepeatN`: use MaybeUninit rust-lang#130145 - Update LLVM to 19 327ca6c rust-lang#130212 - Revert rust-lang#129749 to fix segfault in LLVM rust-lang#130477 - Check params for unsafety in THIR rust-lang#130531 r? cuviper
[beta] backports - Don't warn empty branches unreachable for now rust-lang#129103 - Win: Add dbghelp to the list of import libraries rust-lang#130047 - `RepeatN`: use MaybeUninit rust-lang#130145 - Update LLVM to 19 327ca6c rust-lang#130212 - Revert rust-lang#129749 to fix segfault in LLVM rust-lang#130477 - Check params for unsafety in THIR rust-lang#130531 r? cuviper try-job: dist-various-1
[beta] backports - Don't warn empty branches unreachable for now rust-lang#129103 - Win: Add dbghelp to the list of import libraries rust-lang#130047 - `RepeatN`: use MaybeUninit rust-lang#130145 - Update LLVM to 19 327ca6c rust-lang#130212 - Revert rust-lang#129749 to fix segfault in LLVM rust-lang#130477 - Check params for unsafety in THIR rust-lang#130531 r? cuviper try-job: dist-various-1
[beta] backports - Don't warn empty branches unreachable for now rust-lang#129103 - Win: Add dbghelp to the list of import libraries rust-lang#130047 - `RepeatN`: use MaybeUninit rust-lang#130145 - Update LLVM to 19 327ca6c rust-lang#130212 - Revert rust-lang#129749 to fix segfault in LLVM rust-lang#130477 - Check params for unsafety in THIR rust-lang#130531 r? cuviper try-job: dist-various-1
Revert #129749 to fix segfault in LLVM This reverts commit 8c7a7e346be4cdf13e77ab4acbfb5ade819a4e60, reversing changes made to a00bd75b6c5c96d0a35afa2dc07ce3155112d278. Reported in rust-lang/rust#129749 (comment). `@nikic's` theory is that the LLVM API changed in a way that makes it impossible to use concurrently from multiple threads (llvm/llvm-project#106427 (comment)). I pinged `@krasimirgg` who was fine with reverting. r? `@rust-lang/wg-llvm`
[beta] backports - Don't warn empty branches unreachable for now rust-lang#129103 - Win: Add dbghelp to the list of import libraries rust-lang#130047 - `RepeatN`: use MaybeUninit rust-lang#130145 - Update LLVM to 19 327ca6c rust-lang#130212 - Revert rust-lang#129749 to fix segfault in LLVM rust-lang#130477 - Check params for unsafety in THIR rust-lang#130531 r? cuviper try-job: dist-various-1
[beta] backports - Don't warn empty branches unreachable for now rust-lang#129103 - Win: Add dbghelp to the list of import libraries rust-lang#130047 - `RepeatN`: use MaybeUninit rust-lang#130145 - Update LLVM to 19 327ca6c rust-lang#130212 - Revert rust-lang#129749 to fix segfault in LLVM rust-lang#130477 - Check params for unsafety in THIR rust-lang#130531 r? cuviper try-job: dist-various-1
No functional changes intended.
Updates the wrapper for 5c4lar/llvm-project@21eddfa.
@rustbot label: +llvm-main
r? @nikic