Skip to content

Sort mono items by symbol name #145358

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+60 −43
Open

Sort mono items by symbol name #145358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions compiler/rustc_middle/src/mir/mono.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,10 +566,27 @@ impl<'tcx> CodegenUnit<'tcx> {

let mut items: Vec<_> = self.items().iter().map(|(&i, &data)| (i, data)).collect();
if !tcx.sess.opts.unstable_opts.codegen_source_order {
// It's already deterministic, so we can just use it.
return items;
// In this case, we do not need to keep the items in any specific order, as the input
// is already deterministic.
//
// However, it seems that moving related things (such as different
// monomorphizations of the same function) close to one another is actually beneficial
// for LLVM performance.
// LLVM will codegen the items in the order we pass them to it, and when it handles
// similar things in succession, it seems that it leads to better cache utilization,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put the a, c::<u32>, b, c::<i16>, d, c::<bool> example in here? I think that makes it clearer what "similar things" are, which is still a bit vague without the example. Because in the absence of generics alphabetical order would be no better than random, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that even without monomorphizations it sort of makes sense, things that have a similar symbol typically live in a similar module, or they are methods on the same type (unless the symbol is heavily mangled or really random), so they will e.g. reference similar types, call similar functions etc. But the monomorphization example is indeed the best one, I added it.

// less branch mispredictions and in general to better performance.
// For example, if we have functions `a`, `c::<u32>`, `b`, `c::<i16>`, `d` and
// `c::<bool>`, it seems that it helps LLVM's performance to codegen the three `c`
// instantiations right after one another, as they will likely reference similar types,
// call similar functions, etc.
//
// See https://github.com/rust-lang/rust/pull/145358 for more details.
//
// Sorting by symbol name should not incur any new non-determinism.
items.sort_by_cached_key(|&(i, _)| i.symbol_name(tcx));
} else {
items.sort_by_cached_key(|&(i, _)| item_sort_key(tcx, i));
}
items.sort_by_cached_key(|&(i, _)| item_sort_key(tcx, i));
items
}

Expand Down
80 changes: 40 additions & 40 deletions tests/ui/intrinsics/non-integer-atomic.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@ error[E0511]: invalid monomorphization of `atomic_load` intrinsic: expected basi
LL | intrinsics::atomic_load::<_, { SeqCst }>(p);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `&dyn Fn()`
--> $DIR/non-integer-atomic.rs:65:5
|
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_load` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:35:5
|
LL | intrinsics::atomic_load::<_, { SeqCst }>(p);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `&dyn Fn()`
--> $DIR/non-integer-atomic.rs:60:5
|
LL | intrinsics::atomic_store::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:85:5
error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:45:5
|
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -28,71 +28,71 @@ error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected bas
LL | intrinsics::atomic_cxchg::<_, { SeqCst }, { SeqCst }>(p, v, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:40:5
error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `&dyn Fn()`
--> $DIR/non-integer-atomic.rs:60:5
|
LL | intrinsics::atomic_store::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:90:5
error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:50:5
|
LL | intrinsics::atomic_cxchg::<_, { SeqCst }, { SeqCst }>(p, v, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:80:5
|
LL | intrinsics::atomic_store::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:20:5
error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:40:5
|
LL | intrinsics::atomic_store::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `&dyn Fn()`
--> $DIR/non-integer-atomic.rs:65:5
|
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_load` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:75:5
|
LL | intrinsics::atomic_load::<_, { SeqCst }>(p);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:85:5
|
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_load` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:15:5
|
LL | intrinsics::atomic_load::<_, { SeqCst }>(p);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:30:5
error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:25:5
|
LL | intrinsics::atomic_cxchg::<_, { SeqCst }, { SeqCst }>(p, v, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:50:5
error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:90:5
|
LL | intrinsics::atomic_cxchg::<_, { SeqCst }, { SeqCst }>(p, v, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `Foo`
--> $DIR/non-integer-atomic.rs:45:5
error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `[u8; 100]`
--> $DIR/non-integer-atomic.rs:80:5
|
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | intrinsics::atomic_store::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_xchg` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:25:5
error[E0511]: invalid monomorphization of `atomic_cxchg` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:30:5
|
LL | intrinsics::atomic_xchg::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | intrinsics::atomic_cxchg::<_, { SeqCst }, { SeqCst }>(p, v, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0511]: invalid monomorphization of `atomic_store` intrinsic: expected basic integer or pointer type, found `bool`
--> $DIR/non-integer-atomic.rs:20:5
|
LL | intrinsics::atomic_store::<_, { SeqCst }>(p, v);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 16 previous errors

Expand Down
Loading