Skip to content

Remove precise box allocation #36

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

Merged
merged 5 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 3 additions & 17 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,25 +524,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let box_layout = bx.cx().layout_of(bx.tcx().mk_box(content_ty));
let llty_ptr = bx.cx().backend_type(box_layout);

let (bitmap, bitmap_size) =
content_ty.gc_layout(bx.tcx(), ty::ParamEnv::reveal_all());
let llbitmap = bx.cx().const_usize(bitmap);
let llbitmap_size = bx.cx().const_usize(bitmap_size);

// Allocate space:
let mut alloc_kind = LangItem::ExchangeMalloc;
if bx.tcx().sess.opts.cg.gc_precise_marking {
alloc_kind = if content_ty.is_conservative(
bx.cx().tcx().at(rustc_span::DUMMY_SP),
ty::ParamEnv::reveal_all(),
) {
LangItem::ExchangeMallocConservative
} else if content_ty
alloc_kind = if content_ty
.is_no_trace(bx.tcx().at(rustc_span::DUMMY_SP), ty::ParamEnv::reveal_all())
{
LangItem::ExchangeMallocUntraceable
} else {
LangItem::ExchangeMallocPrecise
LangItem::ExchangeMallocConservative
};
}
let def_id = match bx.tcx().lang_items().require(alloc_kind) {
Expand All @@ -554,11 +544,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let instance = ty::Instance::mono(bx.tcx(), def_id);
let r = bx.cx().get_fn_addr(instance);

let mut call = bx.call(r, &[llsize, llalign], None);
if bx.tcx().sess.opts.cg.gc_precise_marking {
call = bx.call(r, &[llsize, llalign, llbitmap, llbitmap_size], None);
}

let call = bx.call(r, &[llsize, llalign], None);
let val = bx.pointercast(call, llty_ptr);

let operand = OperandRef { val: OperandValue::Immediate(val), layout: box_layout };
Expand Down
15 changes: 6 additions & 9 deletions compiler/rustc_mir/src/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,15 +624,12 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
let source_ty = self.monomorphize(ty);
let mut alloc_kind = LangItem::ExchangeMalloc;
if tcx.sess.opts.cg.gc_precise_marking {
alloc_kind = if source_ty
.is_conservative(tcx.at(DUMMY_SP), ty::ParamEnv::reveal_all())
{
LangItem::ExchangeMallocConservative
} else if source_ty.is_no_trace(tcx.at(DUMMY_SP), ty::ParamEnv::reveal_all()) {
LangItem::ExchangeMallocUntraceable
} else {
LangItem::ExchangeMallocPrecise
};
alloc_kind =
if source_ty.is_no_trace(tcx.at(DUMMY_SP), ty::ParamEnv::reveal_all()) {
LangItem::ExchangeMallocUntraceable
} else {
LangItem::ExchangeMallocConservative
};
}
let exchange_malloc_fn_def_id = tcx.require_lang_item(alloc_kind, None);
let instance = Instance::mono(tcx, exchange_malloc_fn_def_id);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
"force use of the frame pointers"),
force_unwind_tables: Option<bool> = (None, parse_opt_bool, [TRACKED],
"force use of unwind tables"),
gc_precise_marking: bool = (false, parse_bool, [TRACKED],
gc_precise_marking: bool = (true, parse_bool, [TRACKED],
"allocate objects with support for precise marking"),
incremental: Option<String> = (None, parse_opt_string, [UNTRACKED],
"enable incremental compilation"),
Expand Down
61 changes: 28 additions & 33 deletions library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub unsafe fn alloc_untraceable(layout: Layout) -> *mut u8 {
#[allow(missing_docs)]
#[inline]
pub unsafe fn alloc_conservative(layout: Layout) -> *mut u8 {
unsafe { __rust_alloc_conservative(layout.size(), layout.align()) }
unsafe { __rust_alloc(layout.size(), layout.align()) }
}

#[stable(feature = "global_alloc", since = "1.28.0")]
Expand All @@ -190,14 +190,6 @@ pub unsafe fn alloc_precise(layout: Layout, bitmap: usize, bitmap_size: usize) -
unsafe { __rust_alloc_precise(layout.size(), layout.align(), bitmap, bitmap_size) }
}

#[cfg(not(bootstrap))]
#[cfg(not(test))]
enum AllocType {
Untraceable,
Conservative,
Precise(usize, usize),
}

#[cfg(not(test))]
impl Global {
#[inline]
Expand All @@ -213,25 +205,38 @@ impl Global {
}
}

#[inline]
#[cfg(not(bootstrap))]
fn alloc_untraceable_impl(
&self,
layout: Layout,
_zeroed: bool,
) -> Result<NonNull<[u8]>, AllocError> {
match layout.size() {
0 => Ok(NonNull::slice_from_raw_parts(layout.dangling(), 0)),
// SAFETY: `layout` is non-zero in size,
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what this comment is supposed to be attached to (presumably the second clause?).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rustc has a lint which won't allow an unsafe block without a comment explaining why the unsafe block is ok to use.

Copy link
Member

Choose a reason for hiding this comment

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

Oh! Would it be clearer to write this as:

x => y {
   // comment
  unsafe { ... }
}

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree this is better and I went to change it, but then x.py fmt reverted my changes. I think we should leave it because fighting with the formatter is just going to cause pain.

Copy link
Member

Choose a reason for hiding this comment

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

If it reverts it, I don't think we have much choice :)

size => unsafe {
let raw_ptr = alloc_untraceable(layout);
let ptr = NonNull::new(raw_ptr).ok_or(AllocError)?;
Ok(NonNull::slice_from_raw_parts(ptr, size))
},
}
}

#[inline]
fn alloc_impl_typed(
#[cfg(not(bootstrap))]
fn alloc_precise_impl(
&self,
layout: Layout,
bitmap: usize,
bitmap_size: usize,
_zeroed: bool,
aty: AllocType,
) -> Result<NonNull<[u8]>, AllocError> {
match layout.size() {
0 => Ok(NonNull::slice_from_raw_parts(layout.dangling(), 0)),
// SAFETY: `layout` is non-zero in size,
size => unsafe {
let raw_ptr = match aty {
AllocType::Untraceable => alloc_untraceable(layout),
AllocType::Conservative => alloc_conservative(layout),
AllocType::Precise(bitmap, bitmap_size) => {
alloc_precise(layout, bitmap, bitmap_size)
}
};
let raw_ptr = alloc_precise(layout, bitmap, bitmap_size);
let ptr = NonNull::new(raw_ptr).ok_or(AllocError)?;
Ok(NonNull::slice_from_raw_parts(ptr, size))
},
Expand Down Expand Up @@ -376,13 +381,13 @@ unsafe impl Allocator for Global {
#[cfg(not(bootstrap))]
#[inline]
fn alloc_untraceable(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
self.alloc_impl_typed(layout, false, AllocType::Untraceable)
self.alloc_untraceable_impl(layout, false)
}

#[cfg(not(bootstrap))]
#[inline]
fn alloc_conservative(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
self.alloc_impl_typed(layout, false, AllocType::Conservative)
self.alloc_impl(layout, false)
}

#[cfg(not(bootstrap))]
Expand All @@ -393,7 +398,7 @@ unsafe impl Allocator for Global {
bitmap: usize,
bitmap_size: usize,
) -> Result<NonNull<[u8]>, AllocError> {
self.alloc_impl_typed(layout, false, AllocType::Precise(bitmap, bitmap_size))
self.alloc_precise_impl(layout, bitmap, bitmap_size, false)
}
}
#[cfg(not(test))]
Expand Down Expand Up @@ -432,12 +437,7 @@ unsafe fn exchange_malloc_precise(
#[cfg_attr(not(bootstrap), lang = "exchange_malloc_untraceable")]
#[allow(dead_code)]
#[inline]
unsafe fn exchange_malloc_untraceable(
size: usize,
align: usize,
_bitmap: usize,
_bitmap_size: usize,
) -> *mut u8 {
unsafe fn exchange_malloc_untraceable(size: usize, align: usize) -> *mut u8 {
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
match Global.alloc_untraceable(layout) {
Ok(ptr) => ptr.as_mut_ptr(),
Expand All @@ -449,12 +449,7 @@ unsafe fn exchange_malloc_untraceable(
#[cfg_attr(not(bootstrap), lang = "exchange_malloc_conservative")]
#[allow(dead_code)]
#[inline]
unsafe fn exchange_malloc_conservative(
size: usize,
align: usize,
_bitmap: usize,
_bitmap_size: usize,
) -> *mut u8 {
unsafe fn exchange_malloc_conservative(size: usize, align: usize) -> *mut u8 {
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
match Global.alloc_conservative(layout) {
Ok(ptr) => ptr.as_mut_ptr(),
Expand Down
14 changes: 14 additions & 0 deletions library/alloc/src/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use core::{
ptr::{null_mut, NonNull},
};

use core::gc::NoTrace;

use boehm::GcAllocator;

#[cfg(test)]
Expand Down Expand Up @@ -59,6 +61,8 @@ struct GcPointer<T: ?Sized>(NonNull<GcBox<T>>);
unsafe impl<T> Send for GcPointer<T> {}
unsafe impl<T> Sync for GcPointer<T> {}

impl<T: ?Sized> !NoTrace for Gc<T> {}

#[unstable(feature = "gc", issue = "none")]
impl<T: ?Sized + Unsize<U> + Send, U: ?Sized + Send> CoerceUnsized<Gc<U>> for Gc<T> {}
#[unstable(feature = "gc", issue = "none")]
Expand Down Expand Up @@ -218,7 +222,17 @@ struct GcBox<T: ?Sized>(ManuallyDrop<T>);
impl<T> GcBox<T> {
fn new(value: T) -> *mut GcBox<T> {
let layout = Layout::new::<T>();

#[cfg(bootstrap)]
let ptr = ALLOCATOR.allocate(layout).unwrap().as_ptr() as *mut GcBox<T>;

#[cfg(not(bootstrap))]
let ptr = if core::gc::needs_tracing::<T>() {
ALLOCATOR.allocate(layout).unwrap().as_ptr()
} else {
ALLOCATOR.alloc_untraceable(layout).unwrap().as_ptr()
} as *mut GcBox<T>;

let gcbox = GcBox(ManuallyDrop::new(value));

unsafe {
Expand Down
8 changes: 0 additions & 8 deletions library/core/src/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,5 @@ pub unsafe fn gc_layout<T>() -> Trace {
Trace { bitmap: layout[0], size: layout[1] }
}

impl !NoTrace for usize {}

#[cfg(target_pointer_width = "64")]
impl !NoTrace for u64 {}

#[cfg(target_pointer_width = "32")]
impl !NoTrace for u32 {}

impl<T: ?Sized> !NoTrace for *mut T {}
impl<T: ?Sized> !NoTrace for *const T {}
5 changes: 5 additions & 0 deletions library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::cmp::Ordering;
use crate::convert::From;
use crate::fmt;
use crate::gc::NoTrace;
use crate::hash;
use crate::marker::Unsize;
use crate::mem::{self, MaybeUninit};
Expand Down Expand Up @@ -61,6 +62,10 @@ impl<T: ?Sized> !Send for NonNull<T> {}
#[stable(feature = "nonnull", since = "1.25.0")]
impl<T: ?Sized> !Sync for NonNull<T> {}

/// `NonNull` pointers are `NoTrace` if `T` is.
#[stable(feature = "nonnull", since = "1.25.0")]
impl<T: NoTrace> NoTrace for NonNull<T> {}

impl<T: Sized> NonNull<T> {
/// Creates a new `NonNull` that is dangling, but well-aligned.
///
Expand Down
5 changes: 5 additions & 0 deletions library/core/src/ptr/unique.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use crate::marker::{PhantomData, Unsize};
use crate::mem;
use crate::ops::{CoerceUnsized, DispatchFromDyn};

use crate::gc::NoTrace;

/// A wrapper around a raw non-null `*mut T` that indicates that the possessor
/// of this wrapper owns the referent. Useful for building abstractions like
/// `Box<T>`, `Vec<T>`, `String`, and `HashMap<K, V>`.
Expand Down Expand Up @@ -57,6 +59,9 @@ unsafe impl<T: Send + ?Sized> Send for Unique<T> {}
#[unstable(feature = "ptr_internals", issue = "none")]
unsafe impl<T: Sync + ?Sized> Sync for Unique<T> {}

#[unstable(feature = "ptr_internals", issue = "none")]
impl<T: NoTrace> NoTrace for Unique<T> {}

#[unstable(feature = "ptr_internals", issue = "none")]
impl<T: Sized> Unique<T> {
/// Creates a new `Unique` that is dangling, but well-aligned.
Expand Down
2 changes: 1 addition & 1 deletion src/test/codegen/alloc-optimisation.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//
// ignore-test
// min-llvm-version: 10.0.1
// compile-flags: -O
#![crate_type="lib"]
Expand Down
39 changes: 39 additions & 0 deletions src/test/ui/intrinsics/no-trace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// run-pass
#![feature(gc)]
#![allow(dead_code)]

use std::gc::needs_tracing;
use std::rc::Rc;
use std::cell::Cell;


struct A {
inner1: B,
inner2: *mut u8
}

enum B {
W(Vec<C>),
X((usize, Vec<C>)),
Y(Vec<B>),
Z(String),
}

struct C {
a: Rc<usize>,
b: Cell<usize>
}

struct D(Vec<A>);

const CONST_A: bool = needs_tracing::<A>();
const CONST_B: bool = needs_tracing::<B>();
const CONST_C: bool = needs_tracing::<C>();
const CONST_D: bool = needs_tracing::<D>();

fn main() {
assert!(CONST_A);
assert!(!CONST_B);
assert!(!CONST_C);
assert!(CONST_D);
}
Loading