codegen: use new {re,de,}allocator annotations in llvm #99574

durin42 · 2022-07-21T19:38:08Z

This obviates the patch that teaches LLVM internals about
rust{re,de}alloc functions by putting annotations directly in the IR
for the optimizer.

The sole test change is required to anchor FileCheck to the body of the
box_uninitialized method, so it doesn't see the allocalign on
__rust_alloc and get mad about the string alloca showing up. Since I
was there anyway, I added some checks on the attributes to prove the
right attributes got set.

r? @nikic

rust-highfive · 2022-07-21T19:38:11Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikic (or someone else) soon.

Please see the contribution instructions for more information.

rustbot · 2022-07-21T19:38:11Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

compiler/rustc_codegen_llvm/src/attributes.rs

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

library/alloc/src/alloc.rs

nikic

As you've found out, we should have a separate test with min-llvm-version for testing the attributes.

compiler/rustc_codegen_llvm/src/attributes.rs

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

durin42 · 2022-07-23T00:10:36Z

As you've found out, we should have a separate test with min-llvm-version for testing the attributes.

I ended up making "llvm14" variants of the tests that don't have any llvm-15 specific stuff, and marking the "main" tests as requiring LLVM 15, with an eye towards deleting the old-llvm version and only really having to think about the LLVM 15 flavor fairly soon.

Also, we have zero tests that emit __rust_realloc, but I have no idea how one would make that happen.

nikic

Looks good to me. Can you please squash the commits?

compiler/rustc_codegen_llvm/src/attributes.rs

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

bors · 2022-07-23T23:28:22Z

☔ The latest upstream changes (presumably #98208) made this pull request unmergeable. Please resolve the merge conflicts.

durin42 · 2022-07-25T14:27:00Z

Squashed as requested

nikic · 2022-07-25T14:28:00Z

Also needs a rebase :)

durin42 · 2022-07-25T15:38:04Z

ugh, I thought I had. I must have held git wrong somehow.

nikic · 2022-07-25T18:43:02Z

@bors r+ rollup=never

bors · 2022-07-25T18:43:04Z

📌 Commit 6b611ead820d00af78f0c5a410ada1007b305a36 has been approved by nikic

It is now in the queue for this repository.

bors · 2022-07-26T07:09:37Z

⌛ Testing commit 6b611ead820d00af78f0c5a410ada1007b305a36 with merge 9cbe145955675f6101de19d1f567e69a0fccda02...

bors · 2022-07-26T07:29:23Z

💔 Test failed - checks-actions

This obviates the patch that teaches LLVM internals about _rust_{re,de}alloc functions by putting annotations directly in the IR for the optimizer. The sole test change is required to anchor FileCheck to the body of the `box_uninitialized` method, so it doesn't see the `allocalign` on `__rust_alloc` and get mad about the string `alloca` showing up. Since I was there anyway, I added some checks on the attributes to prove the right attributes got set. While we're here, we also emit allocator attributes on __rust_alloc_zeroed. This should allow LLVM to perform more optimizations for zeroed blocks, and probably fixes rust-lang#90032. [This comment](rust-lang#24194 (comment)) mentions "weird UB-like behaviour with bitvec iterators in rustc_data_structures" so we may need to back this change out if things go wrong. The new test cases require LLVM 15, so we copy them into LLVM 14-supporting versions, which we can delete when we drop LLVM 14.

durin42 · 2022-07-26T13:44:30Z

Updated to not require i64 sizes in the allocator functions. Should pass on both 32 and 64 bit now. 😄

nikic · 2022-07-26T15:59:49Z

@bors r+

bors · 2022-07-26T15:59:51Z

📌 Commit 130a1df has been approved by nikic

It is now in the queue for this repository.

bors · 2022-07-26T19:36:00Z

⌛ Testing commit 130a1df with merge 4d6d601...

bors · 2022-07-26T22:20:42Z

☀️ Test successful - checks-actions
Approved by: nikic
Pushing 4d6d601 to master...

rust-timer · 2022-07-26T23:38:07Z

Finished benchmarking commit (4d6d601): comparison url.

Instruction count

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	1.2%	1.2%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	3.2%	3.2%	1
Improvements 🎉 (primary)	-1.8%	-2.5%	21
Improvements 🎉 (secondary)	-4.7%	-5.5%	2
All 😿🎉 (primary)	-1.8%	-2.5%	21

Cycles

Results

Primary benchmarks: 😿 relevant regression found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	7.8%	7.8%	1
Regressions 😿 (secondary)	2.1%	2.1%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	7.8%	7.8%	1

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

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

LLVM 15 compatibility fixes These are LLVM 15 compatibility fixes split out from rust-lang#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 rust-lang#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 rust-lang#99432 (and probably also rust-lang#95679). r? `@cuviper`

rust-highfive assigned nikic Jul 21, 2022

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 21, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 21, 2022

nikic reviewed Jul 21, 2022

View reviewed changes

durin42 force-pushed the allocator-patch-redux branch 2 times, most recently from 054147d to 5e9f978 Compare July 21, 2022 21:48

This comment has been minimized.

Sign in to view

nikic reviewed Jul 22, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

nikic approved these changes Jul 23, 2022

View reviewed changes

compiler/rustc_codegen_llvm/src/attributes.rs Outdated Show resolved Hide resolved

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs Outdated Show resolved Hide resolved

durin42 force-pushed the allocator-patch-redux branch from bb21871 to 88afb16 Compare July 25, 2022 14:26

durin42 force-pushed the allocator-patch-redux branch from 88afb16 to 6b611ea Compare July 25, 2022 15:37

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 25, 2022

This comment has been minimized.

Sign in to view

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 26, 2022

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 26, 2022

durin42 force-pushed the allocator-patch-redux branch from 6b611ea to 130a1df Compare July 26, 2022 13:43

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 26, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 26, 2022

bors merged commit 4d6d601 into rust-lang:master Jul 26, 2022

rustbot added this to the 1.64.0 milestone Jul 26, 2022

nikic mentioned this pull request Jul 27, 2022

LLVM 15 compatibility fixes #99512

Merged

est31 mentioned this pull request Jul 30, 2022

warning: enumeration values 'AllocatedPointer' and 'AllocAlign' not handled in switch #99955

Closed

durin42 deleted the allocator-patch-redux branch July 27, 2023 18:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

codegen: use new {re,de,}allocator annotations in llvm #99574

codegen: use new {re,de,}allocator annotations in llvm #99574

durin42 commented Jul 21, 2022

rust-highfive commented Jul 21, 2022

rustbot commented Jul 21, 2022

This comment has been minimized.

This comment has been minimized.

nikic left a comment

durin42 commented Jul 23, 2022

This comment has been minimized.

nikic left a comment

bors commented Jul 23, 2022

durin42 commented Jul 25, 2022

nikic commented Jul 25, 2022

durin42 commented Jul 25, 2022

nikic commented Jul 25, 2022

bors commented Jul 25, 2022

bors commented Jul 26, 2022

This comment has been minimized.

bors commented Jul 26, 2022

durin42 commented Jul 26, 2022

nikic commented Jul 26, 2022

bors commented Jul 26, 2022

bors commented Jul 26, 2022

bors commented Jul 26, 2022

rust-timer commented Jul 26, 2022

codegen: use new {re,de,}allocator annotations in llvm #99574

codegen: use new {re,de,}allocator annotations in llvm #99574

Conversation

durin42 commented Jul 21, 2022

rust-highfive commented Jul 21, 2022

rustbot commented Jul 21, 2022

This comment has been minimized.

This comment has been minimized.

nikic left a comment

Choose a reason for hiding this comment

durin42 commented Jul 23, 2022

This comment has been minimized.

nikic left a comment

Choose a reason for hiding this comment

bors commented Jul 23, 2022

durin42 commented Jul 25, 2022

nikic commented Jul 25, 2022

durin42 commented Jul 25, 2022

nikic commented Jul 25, 2022

bors commented Jul 25, 2022

bors commented Jul 26, 2022

This comment has been minimized.

bors commented Jul 26, 2022

durin42 commented Jul 26, 2022

nikic commented Jul 26, 2022

bors commented Jul 26, 2022

bors commented Jul 26, 2022

bors commented Jul 26, 2022

rust-timer commented Jul 26, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes