Skip to content

atomicrmw on pointers: move integer-pointer cast hacks into backend #144192

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
24 changes: 12 additions & 12 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = amount.layout();
match layout.ty.kind() {
ty::Uint(_) | ty::Int(_) | ty::RawPtr(..) => {}
ty::Uint(_) | ty::Int(_) => {}
_ => {
report_atomic_type_validation_error(fx, intrinsic, source_info.span, layout.ty);
return Ok(());
Expand All @@ -982,7 +982,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let old =
fx.bcx.ins().atomic_rmw(ty, MemFlags::trusted(), AtomicRmwOp::Add, ptr, amount);

let old = CValue::by_val(old, layout);
let old = CValue::by_val(old, ret.layout());
ret.write_cvalue(fx, old);
}
sym::atomic_xsub => {
Expand All @@ -991,7 +991,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = amount.layout();
match layout.ty.kind() {
ty::Uint(_) | ty::Int(_) | ty::RawPtr(..) => {}
ty::Uint(_) | ty::Int(_) => {}
_ => {
report_atomic_type_validation_error(fx, intrinsic, source_info.span, layout.ty);
return Ok(());
Expand All @@ -1004,7 +1004,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
let old =
fx.bcx.ins().atomic_rmw(ty, MemFlags::trusted(), AtomicRmwOp::Sub, ptr, amount);

let old = CValue::by_val(old, layout);
let old = CValue::by_val(old, ret.layout());
ret.write_cvalue(fx, old);
}
sym::atomic_and => {
Expand All @@ -1013,7 +1013,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = src.layout();
match layout.ty.kind() {
ty::Uint(_) | ty::Int(_) | ty::RawPtr(..) => {}
ty::Uint(_) | ty::Int(_) => {}
_ => {
report_atomic_type_validation_error(fx, intrinsic, source_info.span, layout.ty);
return Ok(());
Expand All @@ -1025,7 +1025,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let old = fx.bcx.ins().atomic_rmw(ty, MemFlags::trusted(), AtomicRmwOp::And, ptr, src);

let old = CValue::by_val(old, layout);
let old = CValue::by_val(old, ret.layout());
ret.write_cvalue(fx, old);
}
sym::atomic_or => {
Expand All @@ -1034,7 +1034,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = src.layout();
match layout.ty.kind() {
ty::Uint(_) | ty::Int(_) | ty::RawPtr(..) => {}
ty::Uint(_) | ty::Int(_) => {}
_ => {
report_atomic_type_validation_error(fx, intrinsic, source_info.span, layout.ty);
return Ok(());
Expand All @@ -1046,7 +1046,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let old = fx.bcx.ins().atomic_rmw(ty, MemFlags::trusted(), AtomicRmwOp::Or, ptr, src);

let old = CValue::by_val(old, layout);
let old = CValue::by_val(old, ret.layout());
ret.write_cvalue(fx, old);
}
sym::atomic_xor => {
Expand All @@ -1055,7 +1055,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = src.layout();
match layout.ty.kind() {
ty::Uint(_) | ty::Int(_) | ty::RawPtr(..) => {}
ty::Uint(_) | ty::Int(_) => {}
_ => {
report_atomic_type_validation_error(fx, intrinsic, source_info.span, layout.ty);
return Ok(());
Expand All @@ -1067,7 +1067,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let old = fx.bcx.ins().atomic_rmw(ty, MemFlags::trusted(), AtomicRmwOp::Xor, ptr, src);

let old = CValue::by_val(old, layout);
let old = CValue::by_val(old, ret.layout());
ret.write_cvalue(fx, old);
}
sym::atomic_nand => {
Expand All @@ -1076,7 +1076,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = src.layout();
match layout.ty.kind() {
ty::Uint(_) | ty::Int(_) | ty::RawPtr(..) => {}
ty::Uint(_) | ty::Int(_) => {}
_ => {
report_atomic_type_validation_error(fx, intrinsic, source_info.span, layout.ty);
return Ok(());
Expand All @@ -1088,7 +1088,7 @@ fn codegen_regular_intrinsic_call<'tcx>(

let old = fx.bcx.ins().atomic_rmw(ty, MemFlags::trusted(), AtomicRmwOp::Nand, ptr, src);

let old = CValue::by_val(old, layout);
let old = CValue::by_val(old, ret.layout());
ret.write_cvalue(fx, old);
}
sym::atomic_max => {
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
dst: RValue<'gcc>,
src: RValue<'gcc>,
order: AtomicOrdering,
ret_ptr: bool,
) -> RValue<'gcc> {
let size = get_maybe_pointer_size(src);
let name = match op {
Expand Down Expand Up @@ -1683,14 +1684,18 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
let atomic_function = self.context.get_builtin_function(name);
let order = self.context.new_rvalue_from_int(self.i32_type, order.to_gcc());

// FIXME: If `ret_ptr` is true and `src` is an integer, we should really tell GCC
// that this is a pointer operation that needs to preserve provenance -- but like LLVM,
// GCC does not currently seems to support that.
let void_ptr_type = self.context.new_type::<*mut ()>();
let volatile_void_ptr_type = void_ptr_type.make_volatile();
let dst = self.context.new_cast(self.location, dst, volatile_void_ptr_type);
// FIXME(antoyo): not sure why, but we have the wrong type here.
let new_src_type = atomic_function.get_param(1).to_rvalue().get_type();
let src = self.context.new_bitcast(self.location, src, new_src_type);
let res = self.context.new_call(self.location, atomic_function, &[dst, src, order]);
self.context.new_cast(self.location, res, src.get_type())
let res_type = if ret_ptr { void_ptr_type } else { src.get_type() };
self.context.new_cast(self.location, res, res_type)
}

fn atomic_fence(&mut self, order: AtomicOrdering, scope: SynchronizationScope) {
Expand Down
14 changes: 6 additions & 8 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1326,15 +1326,13 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
&mut self,
op: rustc_codegen_ssa::common::AtomicRmwBinOp,
dst: &'ll Value,
mut src: &'ll Value,
src: &'ll Value,
order: rustc_middle::ty::AtomicOrdering,
ret_ptr: bool,
) -> &'ll Value {
// The only RMW operation that LLVM supports on pointers is compare-exchange.
let requires_cast_to_int = self.val_ty(src) == self.type_ptr()
&& op != rustc_codegen_ssa::common::AtomicRmwBinOp::AtomicXchg;
if requires_cast_to_int {
src = self.ptrtoint(src, self.type_isize());
}
// FIXME: If `ret_ptr` is true and `src` is not a pointer, we *should* tell LLVM that the
// LHS is a pointer and the operation should be provenance-preserving, but LLVM does not
// currently support that (https://github.com/llvm/llvm-project/issues/120837).
let mut res = unsafe {
llvm::LLVMBuildAtomicRMW(
self.llbuilder,
Expand All @@ -1345,7 +1343,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
llvm::False, // SingleThreaded
)
};
if requires_cast_to_int {
if ret_ptr && self.val_ty(res) != self.type_ptr() {
res = self.inttoptr(res, self.type_ptr());
}
res
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ codegen_ssa_invalid_monomorphization_basic_float_type = invalid monomorphization

codegen_ssa_invalid_monomorphization_basic_integer_type = invalid monomorphization of `{$name}` intrinsic: expected basic integer type, found `{$ty}`

codegen_ssa_invalid_monomorphization_basic_integer_or_ptr_type = invalid monomorphization of `{$name}` intrinsic: expected basic integer or pointer type, found `{$ty}`

codegen_ssa_invalid_monomorphization_cannot_return = invalid monomorphization of `{$name}` intrinsic: cannot return `{$ret_ty}`, expected `u{$expected_int_bits}` or `[u8; {$expected_bytes}]`

codegen_ssa_invalid_monomorphization_cast_wide_pointer = invalid monomorphization of `{$name}` intrinsic: cannot cast wide pointer `{$ty}`
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,14 @@ pub enum InvalidMonomorphization<'tcx> {
ty: Ty<'tcx>,
},

#[diag(codegen_ssa_invalid_monomorphization_basic_integer_or_ptr_type, code = E0511)]
BasicIntegerOrPtrType {
#[primary_span]
span: Span,
name: Symbol,
ty: Ty<'tcx>,
},

#[diag(codegen_ssa_invalid_monomorphization_basic_float_type, code = E0511)]
BasicFloatType {
#[primary_span]
Expand Down
82 changes: 66 additions & 16 deletions compiler/rustc_codegen_ssa/src/mir/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let invalid_monomorphization_int_type = |ty| {
bx.tcx().dcx().emit_err(InvalidMonomorphization::BasicIntegerType { span, name, ty });
};
let invalid_monomorphization_int_or_ptr_type = |ty| {
bx.tcx().dcx().emit_err(InvalidMonomorphization::BasicIntegerOrPtrType {
span,
name,
ty,
});
};

let parse_atomic_ordering = |ord: ty::Value<'tcx>| {
let discr = ord.valtree.unwrap_branch()[0].unwrap_leaf();
Expand Down Expand Up @@ -351,7 +358,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
sym::atomic_load => {
let ty = fn_args.type_at(0);
if !(int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr()) {
invalid_monomorphization_int_type(ty);
invalid_monomorphization_int_or_ptr_type(ty);
return Ok(());
}
let ordering = fn_args.const_at(1).to_value();
Expand All @@ -367,7 +374,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
sym::atomic_store => {
let ty = fn_args.type_at(0);
if !(int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr()) {
invalid_monomorphization_int_type(ty);
invalid_monomorphization_int_or_ptr_type(ty);
return Ok(());
}
let ordering = fn_args.const_at(1).to_value();
Expand All @@ -377,10 +384,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.atomic_store(val, ptr, parse_atomic_ordering(ordering), size);
return Ok(());
}
// These are all AtomicRMW ops
sym::atomic_cxchg | sym::atomic_cxchgweak => {
let ty = fn_args.type_at(0);
if !(int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr()) {
invalid_monomorphization_int_type(ty);
invalid_monomorphization_int_or_ptr_type(ty);
return Ok(());
}
let succ_ordering = fn_args.const_at(1).to_value();
Expand All @@ -407,7 +415,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

return Ok(());
}
// These are all AtomicRMW ops
sym::atomic_max | sym::atomic_min => {
let atom_op = if name == sym::atomic_max {
AtomicRmwBinOp::AtomicMax
Expand All @@ -420,7 +427,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let ordering = fn_args.const_at(1).to_value();
let ptr = args[0].immediate();
let val = args[1].immediate();
bx.atomic_rmw(atom_op, ptr, val, parse_atomic_ordering(ordering))
bx.atomic_rmw(
atom_op,
ptr,
val,
parse_atomic_ordering(ordering),
/* ret_ptr */ false,
)
} else {
invalid_monomorphization_int_type(ty);
return Ok(());
Expand All @@ -438,21 +451,44 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let ordering = fn_args.const_at(1).to_value();
let ptr = args[0].immediate();
let val = args[1].immediate();
bx.atomic_rmw(atom_op, ptr, val, parse_atomic_ordering(ordering))
bx.atomic_rmw(
atom_op,
ptr,
val,
parse_atomic_ordering(ordering),
/* ret_ptr */ false,
)
} else {
invalid_monomorphization_int_type(ty);
return Ok(());
}
}
sym::atomic_xchg
| sym::atomic_xadd
sym::atomic_xchg => {
let ty = fn_args.type_at(0);
let ordering = fn_args.const_at(1).to_value();
if int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr() {
let ptr = args[0].immediate();
let val = args[1].immediate();
let atomic_op = AtomicRmwBinOp::AtomicXchg;
bx.atomic_rmw(
atomic_op,
ptr,
val,
parse_atomic_ordering(ordering),
/* ret_ptr */ ty.is_raw_ptr(),
)
} else {
invalid_monomorphization_int_or_ptr_type(ty);
return Ok(());
}
}
sym::atomic_xadd
| sym::atomic_xsub
| sym::atomic_and
| sym::atomic_nand
| sym::atomic_or
| sym::atomic_xor => {
let atom_op = match name {
sym::atomic_xchg => AtomicRmwBinOp::AtomicXchg,
sym::atomic_xadd => AtomicRmwBinOp::AtomicAdd,
sym::atomic_xsub => AtomicRmwBinOp::AtomicSub,
sym::atomic_and => AtomicRmwBinOp::AtomicAnd,
Expand All @@ -462,14 +498,28 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
_ => unreachable!(),
};

let ty = fn_args.type_at(0);
if int_type_width_signed(ty, bx.tcx()).is_some() || ty.is_raw_ptr() {
let ordering = fn_args.const_at(1).to_value();
let ptr = args[0].immediate();
let val = args[1].immediate();
bx.atomic_rmw(atom_op, ptr, val, parse_atomic_ordering(ordering))
// The type of the in-memory data.
let ty_mem = fn_args.type_at(0);
// The type of the 2nd operand, given by-value.
let ty_op = fn_args.type_at(1);

let ordering = fn_args.const_at(2).to_value();
// We require either both arguments to have the same integer type, or the first to
// be a pointer and the second to be `usize`.
if (int_type_width_signed(ty_mem, bx.tcx()).is_some() && ty_op == ty_mem)
|| (ty_mem.is_raw_ptr() && ty_op == bx.tcx().types.usize)
{
let ptr = args[0].immediate(); // of type "pointer to `ty_mem`"
let val = args[1].immediate(); // of type `ty_op`
bx.atomic_rmw(
atom_op,
ptr,
val,
parse_atomic_ordering(ordering),
/* ret_ptr */ ty_mem.is_raw_ptr(),
)
} else {
invalid_monomorphization_int_type(ty);
invalid_monomorphization_int_or_ptr_type(ty_mem);
return Ok(());
}
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_ssa/src/traits/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,12 +548,15 @@ pub trait BuilderMethods<'a, 'tcx>:
failure_order: AtomicOrdering,
weak: bool,
) -> (Self::Value, Self::Value);
/// `ret_ptr` indicates whether the return type (which is also the type `dst` points to)
/// is a pointer or the same type as `src`.
fn atomic_rmw(
&mut self,
op: AtomicRmwBinOp,
dst: Self::Value,
src: Self::Value,
order: AtomicOrdering,
ret_ptr: bool,
) -> Self::Value;
fn atomic_fence(&mut self, order: AtomicOrdering, scope: SynchronizationScope);
fn set_invariant_load(&mut self, load: Self::Value);
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,16 +652,16 @@ pub(crate) fn check_intrinsic_type(
sym::atomic_store => (1, 1, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], tcx.types.unit),

sym::atomic_xchg
| sym::atomic_xadd
| sym::atomic_xsub
| sym::atomic_and
| sym::atomic_nand
| sym::atomic_or
| sym::atomic_xor
| sym::atomic_max
| sym::atomic_min
| sym::atomic_umax
| sym::atomic_umin => (1, 1, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], param(0)),
sym::atomic_xadd
| sym::atomic_xsub
| sym::atomic_and
| sym::atomic_nand
| sym::atomic_or
| sym::atomic_xor => (2, 1, vec![Ty::new_mut_ptr(tcx, param(0)), param(1)], param(0)),
sym::atomic_fence | sym::atomic_singlethreadfence => (0, 1, Vec::new(), tcx.types.unit),

other => {
Expand Down
Loading
Loading