From 79b11f0802a17f5fce65e22b4934104d40a10e93 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 19 Sep 2023 17:08:13 +1000 Subject: [PATCH 1/9] rustc_arena: tweak some comments. --- compiler/rustc_arena/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 23fdd272ffd33..2f0be3f3e4ead 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -407,6 +407,8 @@ impl Default for DroplessArena { #[inline] fn default() -> DroplessArena { DroplessArena { + // We set both `start` and `end` to 0 so that the first call to + // alloc() will trigger a grow(). start: Cell::new(ptr::null_mut()), end: Cell::new(ptr::null_mut()), chunks: Default::default(), @@ -417,7 +419,7 @@ impl Default for DroplessArena { impl DroplessArena { fn grow(&self, layout: Layout) { // Add some padding so we can align `self.end` while - // stilling fitting in a `layout` allocation. + // still fitting in a `layout` allocation. let additional = layout.size() + cmp::max(DROPLESS_ALIGNMENT, layout.align()) - 1; unsafe { @@ -441,7 +443,7 @@ impl DroplessArena { let mut chunk = ArenaChunk::new(align_up(new_cap, PAGE)); self.start.set(chunk.start()); - // Align the end to DROPLESS_ALIGNMENT + // Align the end to DROPLESS_ALIGNMENT. let end = align_down(chunk.end().addr(), DROPLESS_ALIGNMENT); // Make sure we don't go past `start`. This should not happen since the allocation From a11f7e4c0f39c4665ec3b6c03b59fc9c9b7251a2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 08:51:38 +1000 Subject: [PATCH 2/9] Remove some unnecessary lifetimes. --- compiler/rustc_arena/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 2f0be3f3e4ead..d23ff64d66708 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -715,10 +715,10 @@ pub macro declare_arena([$($a:tt $name:ident: $ty:ty,)*]) { } #[allow(clippy::mut_from_ref)] - pub fn alloc_from_iter<'a, T: ArenaAllocatable<'tcx, C>, C>( - &'a self, + pub fn alloc_from_iter, C>( + &self, iter: impl ::std::iter::IntoIterator, - ) -> &'a mut [T] { + ) -> &mut [T] { T::allocate_from_iter(self, iter) } } From 0001eddb933ff9e5beeb8dd00c507b7abbd85e94 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 07:23:42 +1000 Subject: [PATCH 3/9] Inline and remove `DroplessArena::grow_and_alloc`. It has a single callsite. --- compiler/rustc_arena/src/lib.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index d23ff64d66708..0712f1efd6465 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -463,12 +463,6 @@ impl DroplessArena { self.alloc_raw_without_grow(layout).unwrap() } - #[inline(never)] - #[cold] - fn grow_and_alloc(&self) -> *mut u8 { - self.grow_and_alloc_raw(Layout::new::()) - } - /// Allocates a byte slice with specified layout from the current memory /// chunk. Returns `None` if there is no free space left to satisfy the /// request. @@ -517,7 +511,7 @@ impl DroplessArena { } else { // No free space left. Allocate a new chunk to satisfy the request. // On failure the grow will panic or abort. - self.grow_and_alloc::() + self.grow_and_alloc_raw(Layout::new::()) } as *mut T; unsafe { From 55de23ed5dff82a300cf379c9c756f82df48a987 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 07:32:12 +1000 Subject: [PATCH 4/9] Inline and remove `TypedArena::ensure_capacity`. It has a single callsite. --- compiler/rustc_arena/src/lib.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 0712f1efd6465..a8a4ad89c4f4f 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -250,25 +250,20 @@ impl TypedArena { available_bytes >= additional_bytes } - /// Ensures there's enough space in the current chunk to fit `len` objects. - #[inline] - fn ensure_capacity(&self, additional: usize) { - if !self.can_allocate(additional) { - self.grow(additional); - debug_assert!(self.can_allocate(additional)); - } - } - #[inline] unsafe fn alloc_raw_slice(&self, len: usize) -> *mut T { assert!(mem::size_of::() != 0); assert!(len != 0); - self.ensure_capacity(len); + // Ensure the current chunk can fit `len` objects. + if !self.can_allocate(len) { + self.grow(len); + debug_assert!(self.can_allocate(len)); + } let start_ptr = self.ptr.get(); - // SAFETY: `self.ensure_capacity` makes sure that there is enough space - // for `len` elements. + // SAFETY: `can_allocate`/`grow` ensures that there is enough space for + // `len` elements. unsafe { self.ptr.set(start_ptr.add(len)) }; start_ptr } From 51edc219906f0973dd66b4b6ff5ff0ac857a4cc6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 07:36:19 +1000 Subject: [PATCH 5/9] Remove `unsafe` from `TypedArena::alloc_raw_slice`. There's no good reason for it. --- compiler/rustc_arena/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index a8a4ad89c4f4f..5c183afc087da 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -172,8 +172,8 @@ impl IterExt for std::array::IntoIter { return &mut []; } // Move the content to the arena by copying and then forgetting it. + let start_ptr = arena.alloc_raw_slice(len); unsafe { - let start_ptr = arena.alloc_raw_slice(len); self.as_slice().as_ptr().copy_to_nonoverlapping(start_ptr, len); mem::forget(self); slice::from_raw_parts_mut(start_ptr, len) @@ -189,8 +189,8 @@ impl IterExt for Vec { return &mut []; } // Move the content to the arena by copying and then forgetting it. + let start_ptr = arena.alloc_raw_slice(len); unsafe { - let start_ptr = arena.alloc_raw_slice(len); self.as_ptr().copy_to_nonoverlapping(start_ptr, len); self.set_len(0); slice::from_raw_parts_mut(start_ptr, len) @@ -206,8 +206,8 @@ impl IterExt for SmallVec { return &mut []; } // Move the content to the arena by copying and then forgetting it. + let start_ptr = arena.alloc_raw_slice(len); unsafe { - let start_ptr = arena.alloc_raw_slice(len); self.as_ptr().copy_to_nonoverlapping(start_ptr, len); self.set_len(0); slice::from_raw_parts_mut(start_ptr, len) @@ -251,7 +251,7 @@ impl TypedArena { } #[inline] - unsafe fn alloc_raw_slice(&self, len: usize) -> *mut T { + fn alloc_raw_slice(&self, len: usize) -> *mut T { assert!(mem::size_of::() != 0); assert!(len != 0); From 98d97b7323a55fce821bab6606bf216dbb97523a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 07:45:46 +1000 Subject: [PATCH 6/9] Use `Layout::new` consistently in `DroplessArena::alloc`. --- compiler/rustc_arena/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 5c183afc087da..d793158d85f4b 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -501,12 +501,13 @@ impl DroplessArena { assert!(!mem::needs_drop::()); assert!(mem::size_of::() != 0); - let mem = if let Some(a) = self.alloc_raw_without_grow(Layout::for_value::(&object)) { + let layout = Layout::new::(); + let mem = if let Some(a) = self.alloc_raw_without_grow(layout) { a } else { // No free space left. Allocate a new chunk to satisfy the request. // On failure the grow will panic or abort. - self.grow_and_alloc_raw(Layout::new::()) + self.grow_and_alloc_raw(layout) } as *mut T; unsafe { From 25407bc0bb2fd4aa2c1a37c81a72084160d097fd Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 07:51:24 +1000 Subject: [PATCH 7/9] Make `DroplessArena::alloc` call `DroplessArena::alloc_raw`. They're very similar. --- compiler/rustc_arena/src/lib.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index d793158d85f4b..fae147e98b1e5 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -501,14 +501,7 @@ impl DroplessArena { assert!(!mem::needs_drop::()); assert!(mem::size_of::() != 0); - let layout = Layout::new::(); - let mem = if let Some(a) = self.alloc_raw_without_grow(layout) { - a - } else { - // No free space left. Allocate a new chunk to satisfy the request. - // On failure the grow will panic or abort. - self.grow_and_alloc_raw(layout) - } as *mut T; + let mem = self.alloc_raw(Layout::new::()) as *mut T; unsafe { // Write into uninitialized memory. From 55a1a5223a0930df1cf98c0ea759d15700da9e5d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 07:54:27 +1000 Subject: [PATCH 8/9] Reduce `grow_and_alloc_raw` to a single call site. The current structure is clumsy, calling `alloc_raw_without_grow` in one function, and then if that fails, calling another function that calls `alloc_raw_without_grow` again. --- compiler/rustc_arena/src/lib.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index fae147e98b1e5..73ce5fed93d6a 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -412,6 +412,8 @@ impl Default for DroplessArena { } impl DroplessArena { + #[inline(never)] + #[cold] fn grow(&self, layout: Layout) { // Add some padding so we can align `self.end` while // still fitting in a `layout` allocation. @@ -451,13 +453,6 @@ impl DroplessArena { } } - #[inline(never)] - #[cold] - fn grow_and_alloc_raw(&self, layout: Layout) -> *mut u8 { - self.grow(layout); - self.alloc_raw_without_grow(layout).unwrap() - } - /// Allocates a byte slice with specified layout from the current memory /// chunk. Returns `None` if there is no free space left to satisfy the /// request. @@ -488,12 +483,17 @@ impl DroplessArena { #[inline] pub fn alloc_raw(&self, layout: Layout) -> *mut u8 { assert!(layout.size() != 0); - if let Some(a) = self.alloc_raw_without_grow(layout) { - return a; + + // This loop executes once or twice: if allocation fails the first + // time, the `grow` ensures it will succeed the second time. + loop { + if let Some(a) = self.alloc_raw_without_grow(layout) { + return a; + } + // No free space left. Allocate a new chunk to satisfy the request. + // On failure the grow will panic or abort. + self.grow(layout); } - // No free space left. Allocate a new chunk to satisfy the request. - // On failure the grow will panic or abort. - self.grow_and_alloc_raw(layout) } #[inline] From bb5344a1bff6df6c1d5a932c1c5727d7f72a2f55 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Sep 2023 08:04:14 +1000 Subject: [PATCH 9/9] Inline and remove `DroplessArena::alloc_raw_without_grow`. It has a single call site. I find the code clearer with it gone. --- compiler/rustc_arena/src/lib.rs | 51 ++++++++++++++------------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 73ce5fed93d6a..06eb8a185be96 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -453,33 +453,6 @@ impl DroplessArena { } } - /// Allocates a byte slice with specified layout from the current memory - /// chunk. Returns `None` if there is no free space left to satisfy the - /// request. - #[inline] - fn alloc_raw_without_grow(&self, layout: Layout) -> Option<*mut u8> { - let start = self.start.get().addr(); - let old_end = self.end.get(); - let end = old_end.addr(); - - // Align allocated bytes so that `self.end` stays aligned to DROPLESS_ALIGNMENT - let bytes = align_up(layout.size(), DROPLESS_ALIGNMENT); - - // Tell LLVM that `end` is aligned to DROPLESS_ALIGNMENT - unsafe { intrinsics::assume(end == align_down(end, DROPLESS_ALIGNMENT)) }; - - let new_end = align_down(end.checked_sub(bytes)?, layout.align()); - if start <= new_end { - let new_end = old_end.with_addr(new_end); - // `new_end` is aligned to DROPLESS_ALIGNMENT as `align_down` preserves alignment - // as both `end` and `bytes` are already aligned to DROPLESS_ALIGNMENT. - self.end.set(new_end); - Some(new_end) - } else { - None - } - } - #[inline] pub fn alloc_raw(&self, layout: Layout) -> *mut u8 { assert!(layout.size() != 0); @@ -487,9 +460,29 @@ impl DroplessArena { // This loop executes once or twice: if allocation fails the first // time, the `grow` ensures it will succeed the second time. loop { - if let Some(a) = self.alloc_raw_without_grow(layout) { - return a; + let start = self.start.get().addr(); + let old_end = self.end.get(); + let end = old_end.addr(); + + // Align allocated bytes so that `self.end` stays aligned to + // DROPLESS_ALIGNMENT. + let bytes = align_up(layout.size(), DROPLESS_ALIGNMENT); + + // Tell LLVM that `end` is aligned to DROPLESS_ALIGNMENT. + unsafe { intrinsics::assume(end == align_down(end, DROPLESS_ALIGNMENT)) }; + + if let Some(sub) = end.checked_sub(bytes) { + let new_end = align_down(sub, layout.align()); + if start <= new_end { + let new_end = old_end.with_addr(new_end); + // `new_end` is aligned to DROPLESS_ALIGNMENT as `align_down` + // preserves alignment as both `end` and `bytes` are already + // aligned to DROPLESS_ALIGNMENT. + self.end.set(new_end); + return new_end; + } } + // No free space left. Allocate a new chunk to satisfy the request. // On failure the grow will panic or abort. self.grow(layout);