Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
6 changes: 6 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3983,6 +3983,10 @@ dependencies = [
"winapi",
]

[[package]]
name = "rustc_erase"
version = "0.0.0"

[[package]]
name = "rustc_error_codes"
version = "0.0.0"
Expand Down Expand Up @@ -4373,6 +4377,7 @@ dependencies = [
"rustc_ast",
"rustc_attr",
"rustc_data_structures",
"rustc_erase",
"rustc_error_messages",
"rustc_errors",
"rustc_feature",
Expand Down Expand Up @@ -4571,6 +4576,7 @@ dependencies = [
"rustc-rayon-core",
"rustc_ast",
"rustc_data_structures",
"rustc_erase",
"rustc_errors",
"rustc_hir",
"rustc_index",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub mod stable_hasher;
mod atomic_ref;
pub mod fingerprint;
pub mod profiling;
pub mod remap;
pub mod sharded;
pub mod stack;
pub mod sync;
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_data_structures/src/remap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// Remaps the type with a different lifetime for 'tcx if applicable.
pub trait Remap {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is rustc_middle::ty::parametrized::ParameterizedOverTcx that does the same thing.

type Remap<'a>;
}

impl Remap for u32 {
type Remap<'a> = u32;
}

impl<T: Remap> Remap for Option<T> {
type Remap<'a> = Option<T::Remap<'a>>;
}

impl Remap for () {
type Remap<'a> = ();
}

impl<T0: Remap, T1: Remap> Remap for (T0, T1) {
type Remap<'a> = (T0::Remap<'a>, T1::Remap<'a>);
}

impl<T0: Remap, T1: Remap, T2: Remap> Remap for (T0, T1, T2) {
type Remap<'a> = (T0::Remap<'a>, T1::Remap<'a>, T2::Remap<'a>);
}

impl<T0: Remap, T1: Remap, T2: Remap, T3: Remap> Remap for (T0, T1, T2, T3) {
type Remap<'a> = (T0::Remap<'a>, T1::Remap<'a>, T2::Remap<'a>, T3::Remap<'a>);
}
6 changes: 1 addition & 5 deletions compiler/rustc_data_structures/src/sharded.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
use crate::fx::{FxHashMap, FxHasher};
use crate::sync::{Lock, LockGuard};
use crate::sync::{CacheAligned, Lock, LockGuard};
use std::borrow::Borrow;
use std::collections::hash_map::RawEntryMut;
use std::hash::{Hash, Hasher};
use std::mem;

#[derive(Clone, Default)]
#[cfg_attr(parallel_compiler, repr(align(64)))]
struct CacheAligned<T>(T);

#[cfg(parallel_compiler)]
// 32 shards is sufficient to reduce contention on an 8-core Ryzen 7 1700,
// but this should be tested on higher core count CPUs. How the `Sharded` type gets used
Expand Down
36 changes: 7 additions & 29 deletions compiler/rustc_data_structures/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ use std::hash::{BuildHasher, Hash};
use std::ops::{Deref, DerefMut};
use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};

mod worker_local;
pub use worker_local::{Registry, WorkerLocal};

pub use std::sync::atomic::Ordering;
pub use std::sync::atomic::Ordering::SeqCst;

Expand Down Expand Up @@ -178,33 +181,6 @@ cfg_if! {

use std::cell::Cell;

#[derive(Debug)]
pub struct WorkerLocal<T>(OneThread<T>);

impl<T> WorkerLocal<T> {
/// Creates a new worker local where the `initial` closure computes the
/// value this worker local should take for each thread in the thread pool.
#[inline]
pub fn new<F: FnMut(usize) -> T>(mut f: F) -> WorkerLocal<T> {
WorkerLocal(OneThread::new(f(0)))
}

/// Returns the worker-local value for each thread
#[inline]
pub fn into_inner(self) -> Vec<T> {
vec![OneThread::into_inner(self.0)]
}
}

impl<T> Deref for WorkerLocal<T> {
type Target = T;

#[inline(always)]
fn deref(&self) -> &T {
&self.0
}
}

pub type MTRef<'a, T> = &'a mut T;

#[derive(Debug, Default)]
Expand Down Expand Up @@ -324,8 +300,6 @@ cfg_if! {
};
}

pub use rayon_core::WorkerLocal;

pub use rayon::iter::ParallelIterator;
use rayon::iter::IntoParallelIterator;

Expand Down Expand Up @@ -365,6 +339,10 @@ pub fn assert_send<T: ?Sized + Send>() {}
pub fn assert_send_val<T: ?Sized + Send>(_t: &T) {}
pub fn assert_send_sync_val<T: ?Sized + Sync + Send>(_t: &T) {}

#[derive(Clone, Default)]
#[cfg_attr(parallel_compiler, repr(align(64)))]
pub struct CacheAligned<T>(pub T);

pub trait HashMapExt<K, V> {
/// Same as HashMap::insert, but it may panic if there's already an
/// entry for `key` with a value not equal to `value`
Expand Down
189 changes: 189 additions & 0 deletions compiler/rustc_data_structures/src/sync/worker_local.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
use crate::sync::Lock;
use std::cell::Cell;
use std::cell::OnceCell;
use std::ops::Deref;
use std::sync::Arc;

#[cfg(parallel_compiler)]
use {crate::cold_path, crate::sync::CacheAligned};

/// A pointer to the `RegistryData` which uniquely identifies a registry.
/// This identifier can be reused if the registry gets freed.
#[derive(Clone, Copy, Eq, PartialEq)]
struct RegistryId(usize);

impl RegistryId {
#[inline(always)]
/// Verifies that the current thread is associated with the registry and returns its unique
/// index within the registry. This panics if the current thread is not associated with this
/// registry.
///
/// Note that there's a race possible where the identifer in `THREAD_DATA` could be reused
/// so this can succeed from a different registry.
#[cfg(parallel_compiler)]
fn verify(self) -> usize {
let (id, index) = THREAD_DATA.with(|data| (data.registry_id.get(), data.index.get()));

if id == self {
index
} else {
cold_path(|| panic!("Unable to verify registry association"))
}
}
}

struct RegistryData {
thread_limit: usize,
threads: Lock<usize>,
}

/// Represents a list of threads which can access worker locals.
#[derive(Clone)]
pub struct Registry(Arc<RegistryData>);

thread_local! {
/// The registry associated with the thread.
/// This allows the `WorkerLocal` type to clone the registry in its constructor.
static REGISTRY: OnceCell<Registry> = OnceCell::new();
}

struct ThreadData {
registry_id: Cell<RegistryId>,
index: Cell<usize>,
}

thread_local! {
/// A thread local which contains the identifer of `REGISTRY` but allows for faster access.
/// It also holds the index of the current thread.
static THREAD_DATA: ThreadData = const { ThreadData {
registry_id: Cell::new(RegistryId(0)),
index: Cell::new(0),
}};
}

impl Registry {
/// Creates a registry which can hold up to `thread_limit` threads.
pub fn new(thread_limit: usize) -> Self {
Registry(Arc::new(RegistryData { thread_limit, threads: Lock::new(0) }))
}

/// Gets the registry associated with the current thread. Panics if there's no such registry.
pub fn current() -> Self {
REGISTRY.with(|registry| registry.get().cloned().expect("No assocated registry"))
}

/// Registers the current thread with the registry so worker locals can be used on it.
/// Panics if the thread limit is hit or if the thread already has an associated registry.
pub fn register(&self) {
let mut threads = self.0.threads.lock();
if *threads < self.0.thread_limit {
REGISTRY.with(|registry| {
if registry.get().is_some() {
drop(threads);
panic!("Thread already has a registry");
}
registry.set(self.clone()).ok();
THREAD_DATA.with(|data| {
data.registry_id.set(self.id());
data.index.set(*threads);
});
*threads += 1;
});
} else {
drop(threads);
panic!("Thread limit reached");
}
}

/// Gets the identifer of this registry.
fn id(&self) -> RegistryId {
RegistryId(&*self.0 as *const RegistryData as usize)
}
}

/// Holds worker local values for each possible thread in a registry. You can only access the
/// worker local value through the `Deref` impl on the registry associated with the thread it was
/// created on. It will panic otherwise.
pub struct WorkerLocal<T> {
#[cfg(not(parallel_compiler))]
local: T,
#[cfg(parallel_compiler)]
locals: Box<[CacheAligned<T>]>,
#[cfg(parallel_compiler)]
registry: Registry,
}

// This is safe because the `deref` call will return a reference to a `T` unique to each thread
// or it will panic for threads without an associated local. So there isn't a need for `T` to do
// it's own synchronization. The `verify` method on `RegistryId` has an issue where the the id
// can be reused, but `WorkerLocal` has a reference to `Registry` which will prevent any reuse.
#[cfg(parallel_compiler)]
unsafe impl<T: Send> Sync for WorkerLocal<T> {}

impl<T> WorkerLocal<T> {
/// Creates a new worker local where the `initial` closure computes the
/// value this worker local should take for each thread in the registry.
#[inline]
pub fn new<F: FnMut(usize) -> T>(mut initial: F) -> WorkerLocal<T> {
#[cfg(parallel_compiler)]
{
let registry = Registry::current();
WorkerLocal {
locals: {
let locals: Vec<_> =
(0..registry.0.thread_limit).map(|i| CacheAligned(initial(i))).collect();
locals.into_boxed_slice()
},
registry,
}
}
#[cfg(not(parallel_compiler))]
{
WorkerLocal { local: initial(0) }
}
}

/// Returns the worker-local value for each thread
#[inline]
pub fn into_inner(self) -> Vec<T> {
#[cfg(parallel_compiler)]
{
self.locals.into_vec().into_iter().map(|local| local.0).collect()
}
#[cfg(not(parallel_compiler))]
{
vec![self.local]
}
}
}

impl<T> WorkerLocal<Vec<T>> {
/// Joins the elements of all the worker locals into one Vec
pub fn join(self) -> Vec<T> {
self.into_inner().into_iter().flat_map(|v| v).collect()
}
}

impl<T: Default> Default for WorkerLocal<T> {
fn default() -> Self {
WorkerLocal::new(|_| T::default())
}
}

impl<T> Deref for WorkerLocal<T> {
type Target = T;

#[inline(always)]
#[cfg(not(parallel_compiler))]
fn deref(&self) -> &T {
&self.local
}

#[inline(always)]
#[cfg(parallel_compiler)]
fn deref(&self) -> &T {
// This is safe because `verify` will only return values less than
// `self.registry.thread_limit` which is the size of the `self.locals` array.
unsafe { &self.locals.get_unchecked(self.registry.id().verify()).0 }
}
}
6 changes: 6 additions & 0 deletions compiler/rustc_erase/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "rustc_erase"
Copy link
Member

Choose a reason for hiding this comment

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

Why is this inside a new crate and not just in rustc_data_structures? Creating a crate for 47 lines seems pointless, even if the code is very unsafe and self-contained

Copy link
Member

Choose a reason for hiding this comment

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

// This is a separate crate so that we can `allow(incomplete_features)` for just `generic_const_exprs`

Copy link
Member

Choose a reason for hiding this comment

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

Ah, didn't see that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that generic_const_exprs will cause ICEs in other crates than the one using the feature if any of it is used in public APIs

version = "0.0.0"
edition = "2021"

[lib]
46 changes: 46 additions & 0 deletions compiler/rustc_erase/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// This is a separate crate so that we can `allow(incomplete_features)` for just `generic_const_exprs`
#![feature(generic_const_exprs)]
Copy link
Member

@BoxyUwU BoxyUwU Feb 12, 2023

Choose a reason for hiding this comment

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

Please do not use generic_const_exprs, its incredibly incomplete/broken and also its unsound and will have changes made to it that breaks code that works currently in order to get closer to an actually functioning feature. I do not want to have to deal with cfg(bootstrap) pain and working around generic_const_expr bugs from usage of the feature inside of rustc whenever this feature gets worked on ^^'

Copy link
Member

Choose a reason for hiding this comment

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

I guess to explicitly answer

for an option on how stable feature(generic_const_exprs) is.

extremely very not stable 😅

#![allow(incomplete_features)]

#[cfg(debug_assertions)]
use std::intrinsics::type_name;
use std::{
fmt,
mem::{size_of, transmute_copy, MaybeUninit},
};

#[derive(Copy, Clone)]
pub struct Erased<const N: usize> {
data: MaybeUninit<[u8; N]>,
#[cfg(debug_assertions)]
type_id: &'static str,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we prefer std::any::TypeId?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That requires 'static.

}

impl<const N: usize> fmt::Debug for Erased<N> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Erased<{}>", N)
}
}

pub type Erase<T> = Erased<{ size_of::<T>() }>;

#[inline(always)]
pub fn erase<T: Copy>(src: T) -> Erased<{ size_of::<T>() }> {
Erased {
// SAFETY:: Is it safe to transmute to MaybeUninit
data: unsafe { transmute_copy(&src) },
#[cfg(debug_assertions)]
type_id: type_name::<T>(),
}
}

/// Restores an erased value.
///
/// This is only safe if `value` is a valid instance of `T`.
/// For example if `T` was erased with `erase` previously.
#[inline(always)]
pub unsafe fn restore<T: Copy>(value: Erased<{ size_of::<T>() }>) -> T {
#[cfg(debug_assertions)]
assert_eq!(value.type_id, type_name::<T>());
unsafe { transmute_copy(&value.data) }
}
Loading