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

LLVM 15 compatibility fixes #99512

Merged
merged 6 commits into from
Jul 29, 2022
Merged

LLVM 15 compatibility fixes #99512

merged 6 commits into from
Jul 29, 2022

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jul 20, 2022

These are LLVM 15 compatibility fixes split out from #99464. There are three changes here:

  • Emit elementtype attribtue for ldrex/strex intrinsics. This is requires as part of the opaque pointers migration.
  • Make more tests compatible with opaque pointers. These are either new or aren't run on x86.
  • Remove a test for #[rustc_allocator]. Since codegen: use new {re,de,}allocator annotations in llvm #99574 there are more requirement on the function signature. I dropped the test entirely, since we already test the effect of the attribute elsewhere.
  • The main change: When a worker thread emits an error, wait for other threads to finish before unwinding the main thread and exiting. Otherwise workers may end up using globals for which destructors have already been run. This was probably never quite correct, but became an active problem with LLVM 15, because it started using global dtors in critical places, as part of ManagedStatic removal.

Fixes #99432 (and probably also #95679).

r? @cuviper

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 20, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2022
@nikic
Copy link
Contributor Author

nikic commented Jul 27, 2022

I ended up completely rewriting the fix for the global dtor issue. The new implementation drops the AbortCodegenOnDrop wrapper, and instead integrates the functionality into OngoingCodegen (or rather a separate Coordinator struct, to limit drop scope). This ensures that all panics during codegen will result in a CodegenAborted message + thread join. Previously, panics generated as part of join_codegen() were not handled, which is really the root issue here. Similarly, WorkerFatalError handling is switched to do the same thing as CodegenAborted.

I think this makes for a nicer and more consistent solution. The main casualty are various asserts that no longer hold, as codegen can now be aborted at more different points.

These intrinsics (and a few more, but there are the only ones
exposed by stdarch) require an elementtype attribute in LLVM 15.
Replace the separate AbortCodegenOnDrop guard by integrating this
functionality into OngoingCodegen (or rather, the Coordinator part
of it). This ensures that we send a CodegenAborted message and
wait for workers to finish even if the panic occurs outside
codegen_crate() (e.g. inside join_codegen()).

This requires some minor changes to the handling of CodegenAborted,
as it can now occur when the main thread is LLVMing rather than
Codegenning.
This means that codegen_aborted may be set when new codegen
requests arrive, so drop some related assertions. The new work
will simply be ignored.
This attribute now does more than just place noalias on the return,
and has specific requirements for the signature.

Drop the test entirely, as we already check __rust_alloc attributes
in other codegen tests.
@rust-log-analyzer

This comment has been minimized.

@cuviper
Copy link
Member

cuviper commented Jul 28, 2022

LGTM! I'm blocking rollup to be careful about the thread change, although I don't know if CI hits such errors anyway.

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jul 28, 2022

📌 Commit 1433b29 has been approved by cuviper

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2022
@bors
Copy link
Contributor

bors commented Jul 29, 2022

⌛ Testing commit 1433b29 with merge 9de7474...

@bors
Copy link
Contributor

bors commented Jul 29, 2022

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing 9de7474 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 29, 2022
@bors bors merged commit 9de7474 into rust-lang:master Jul 29, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 29, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9de7474): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
2.1% 2.1% 1
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
-2.6% -2.6% 1
All 😿🎉 (primary) N/A N/A 0

Cycles

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
9.2% 9.2% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 9.2% 9.2% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@nikic nikic mentioned this pull request Jul 29, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 12, 2022
Update to LLVM 15

For preliminary testing. Some LLVM 15 compatibility fixes were applied separately in rust-lang#99512.

Release timeline:
 * LLVM 15 branched on Jul 26.
 * The final LLVM 15.0.0 release is scheduled for Sep 6.
 * Current nightly (1.65.0) is scheduled for Nov 3.

Changes in this PR (apart from the LLVM update):
 * Pass `--set llvm.allow-old-toolchain` for many Docker images. LLVM 16 will require GCC >= 7.1, while LLVM 15 still allows older compilers with an option. Specify the option for builders still using GCC 5.4. rust-lang#95026 updated some of the used toolchains, but not all.
 * Use the `+atomics-32` target feature for thumbv6m.
 * Explicitly link libatomic when cross-compiling LLVM to 32-bit target.
 * Explicitly disable zstd support, to avoid libzstd.so dependency.

New LLVM patches ([commits](https://github.com/rust-lang/llvm-project/commits/rustc/15.0-2022-08-09)):
 * [rust-only] Fix ICE with GCC 5.4 (nikic/llvm-project@15be58d)
 * [rust-only] Fix build with GCC 5.4 (nikic/llvm-project@774edc1)
 * ~~[rust-only] Fix build with GCC 5.2 (nikic/llvm-project@1a6069a7bb35ace1e40d566035cbf7ed2fa3b1f7)~~
 * ~~[rust-only] Fix ICE with GCC 5.2 (nikic/llvm-project@493081f2909206e0ed55af68a4058a76c0ad7a64)~~
 * ~~[rust-only] Fix build with GCC 5.2 (nikic/llvm-project@0fc5979d738c3a1f9510fe2d62417f7d2af37817)~~
 * [backported] Addition of `+atomics` target feature (llvm/llvm-project@57bdd98).
 * [backported] Revert compiler-rt change that broke powerpc (llvm/llvm-project@9c68b43)
 * [awaiting backport] Fix RelLookupTableConverter on gnux32 (nikic/llvm-project@639388a / llvm/llvm-project#57021)

Tested images: dist-x86_64-linux, armhf-gnu, arm-android, dist-s390x-linux, dist-x86_64-illumos, dist-x86_64-freebsd, wasm32, dist-x86_64-musl, dist-various-1, dist-riscv64-linux, dist-mips-linux, dist-mipsel-linux, dist-powerpc-linux, dist-aarch64-linux, dist-x86_64-apple, x86_64-msvc-1, x86_64-msvc-2, dist-various-2, dist-arm-linux
Tested up to the usual ipv6 error: test-various, i686-gnu, x86_64-gnu-nopt

r? `@ghost`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc master has a flaky failure when compiled against LLVM main
7 participants