Skip to content

codegen: store ScalarPair via memset when one side is undef and the other side can be memset #135335

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 4 commits into from
Jan 21, 2025
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
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ impl<'gcc, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
if type_is_pointer(typ) { self.context.new_null(typ) } else { self.const_int(typ, 0) }
}

fn is_undef(&self, _val: RValue<'gcc>) -> bool {
// FIXME: actually check for undef
false
}

fn const_undef(&self, typ: Type<'gcc>) -> RValue<'gcc> {
let local = self.current_func.borrow().expect("func").new_local(None, typ, "undefined");
if typ.is_struct().is_some() {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ impl<'ll, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
unsafe { llvm::LLVMGetUndef(t) }
}

fn is_undef(&self, v: &'ll Value) -> bool {
unsafe { llvm::LLVMIsUndef(v) == True }
}

fn const_poison(&self, t: &'ll Type) -> &'ll Value {
unsafe { llvm::LLVMGetPoison(t) }
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,7 @@ unsafe extern "C" {
pub fn LLVMMetadataTypeInContext(C: &Context) -> &Type;

// Operations on all values
pub fn LLVMIsUndef(Val: &Value) -> Bool;
pub fn LLVMTypeOf(Val: &Value) -> &Type;
pub fn LLVMGetValueName2(Val: &Value, Length: *mut size_t) -> *const c_char;
pub fn LLVMSetValueName2(Val: &Value, Name: *const c_char, NameLen: size_t);
Expand Down
59 changes: 37 additions & 22 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,30 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
let alloc_align = alloc.inner().align;
assert!(alloc_align >= layout.align.abi);

// Returns `None` when the value is partially undefined or any byte of it has provenance.
// Otherwise returns the value or (if the entire value is undef) returns an undef.
let read_scalar = |start, size, s: abi::Scalar, ty| {
let range = alloc_range(start, size);
match alloc.0.read_scalar(
bx,
alloc_range(start, size),
range,
/*read_provenance*/ matches!(s.primitive(), abi::Primitive::Pointer(_)),
) {
Ok(val) => bx.scalar_to_backend(val, s, ty),
Err(_) => bx.const_poison(ty),
Ok(val) => Some(bx.scalar_to_backend(val, s, ty)),
Err(_) => {
// We may have failed due to partial provenance or unexpected provenance,
// continue down the normal code path if so.
if alloc.0.provenance().range_empty(range, &bx.tcx())
// Since `read_scalar` failed, but there were no relocations involved, the
// bytes must be partially or fully uninitialized. Thus we can now unwrap the
// information about the range of uninit bytes and check if it's the full range.
&& alloc.0.init_mask().is_range_initialized(range).unwrap_err() == range
{
Some(bx.const_undef(ty))
} else {
None
}
}
}
};

Expand All @@ -222,16 +238,14 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
// check that walks over the type of `mplace` to make sure it is truly correct to treat this
// like a `Scalar` (or `ScalarPair`).
match layout.backend_repr {
BackendRepr::Scalar(s @ abi::Scalar::Initialized { .. }) => {
BackendRepr::Scalar(s) => {
let size = s.size(bx);
assert_eq!(size, layout.size, "abi::Scalar size does not match layout size");
let val = read_scalar(offset, size, s, bx.immediate_backend_type(layout));
OperandRef { val: OperandValue::Immediate(val), layout }
if let Some(val) = read_scalar(offset, size, s, bx.immediate_backend_type(layout)) {
return OperandRef { val: OperandValue::Immediate(val), layout };
}
}
BackendRepr::ScalarPair(
a @ abi::Scalar::Initialized { .. },
b @ abi::Scalar::Initialized { .. },
) => {
BackendRepr::ScalarPair(a, b) => {
let (a_size, b_size) = (a.size(bx), b.size(bx));
let b_offset = (offset + a_size).align_to(b.align(bx).abi);
assert!(b_offset.bytes() > 0);
Expand All @@ -247,20 +261,21 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
b,
bx.scalar_pair_element_backend_type(layout, 1, true),
);
OperandRef { val: OperandValue::Pair(a_val, b_val), layout }
}
_ if layout.is_zst() => OperandRef::zero_sized(layout),
_ => {
// Neither a scalar nor scalar pair. Load from a place
// FIXME: should we cache `const_data_from_alloc` to avoid repeating this for the
// same `ConstAllocation`?
let init = bx.const_data_from_alloc(alloc);
let base_addr = bx.static_addr_of(init, alloc_align, None);

let llval = bx.const_ptr_byte_offset(base_addr, offset);
bx.load_operand(PlaceRef::new_sized(llval, layout))
if let (Some(a_val), Some(b_val)) = (a_val, b_val) {
return OperandRef { val: OperandValue::Pair(a_val, b_val), layout };
}
}
_ if layout.is_zst() => return OperandRef::zero_sized(layout),
_ => {}
}
// Neither a scalar nor scalar pair. Load from a place
// FIXME: should we cache `const_data_from_alloc` to avoid repeating this for the
// same `ConstAllocation`?
let init = bx.const_data_from_alloc(alloc);
let base_addr = bx.static_addr_of(init, alloc_align, None);

let llval = bx.const_ptr_byte_offset(base_addr, offset);
bx.load_operand(PlaceRef::new_sized(llval, layout))
}

/// Asserts that this operand refers to a scalar and returns
Expand Down
26 changes: 24 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_middle::{bug, mir, span_bug};
use rustc_session::config::OptLevel;
use rustc_span::{DUMMY_SP, Span};
use tracing::{debug, instrument};
use tracing::{debug, instrument, trace};

use super::operand::{OperandRef, OperandValue};
use super::place::PlaceRef;
Expand Down Expand Up @@ -93,6 +93,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return;
}

// If `v` is an integer constant whose value is just a single byte repeated N times,
// emit a `memset` filling the entire `dest` with that byte.
let try_init_all_same = |bx: &mut Bx, v| {
let start = dest.val.llval;
let size = bx.const_usize(dest.layout.size.bytes());
Expand All @@ -117,13 +119,33 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
false
};

trace!(?cg_elem.val);
match cg_elem.val {
OperandValue::Immediate(v) => {
if try_init_all_same(bx, v) {
return;
}
}
_ => (),
OperandValue::Pair(a, b) => {
let a_is_undef = bx.cx().is_undef(a);
match (a_is_undef, bx.cx().is_undef(b)) {
// Can happen for uninit unions
(true, true) => {
// FIXME: can we produce better output here?
}
(false, true) | (true, false) => {
let val = if a_is_undef { b } else { a };
if try_init_all_same(bx, val) {
return;
}
}
(false, false) => {
// FIXME: if both are the same value, use try_init_all_same
}
}
}
OperandValue::ZeroSized => unreachable!("checked above"),
OperandValue::Ref(..) => {}
}

let count = self
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub trait ConstCodegenMethods<'tcx>: BackendTypes {
/// Generate an uninitialized value (matching uninitialized memory in MIR).
/// Whether memory is initialized or not is tracked byte-for-byte.
fn const_undef(&self, t: Self::Type) -> Self::Value;
fn is_undef(&self, v: Self::Value) -> bool;
/// Generate a fake value. Poison always affects the entire value, even if just a single byte is
/// poison. This can only be used in codepaths that are already UB, i.e., UB-free Rust code
/// (including code that e.g. copies uninit memory with `MaybeUninit`) can never encounter a
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ impl AllocError {
}

/// The information that makes up a memory access: offset and size.
#[derive(Copy, Clone)]
#[derive(Copy, Clone, PartialEq)]
pub struct AllocRange {
pub start: Size,
pub size: Size,
Expand Down
2 changes: 0 additions & 2 deletions tests/codegen/overaligned-constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ pub fn overaligned_constant() {
// CHECK-LABEL: @overaligned_constant
// CHECK: [[full:%_.*]] = alloca [32 x i8], align 8
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[full]], ptr align 8 @0, i64 32, i1 false)
// CHECK: %b.0 = load i32, ptr @0, align 4
// CHECK: %b.1 = load i32, ptr getelementptr inbounds ({{.*}}), align 4
let mut s = S(1);

s.0 = 3;
Expand Down
55 changes: 52 additions & 3 deletions tests/codegen/slice-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#![crate_type = "lib"]

use std::mem::MaybeUninit;

// CHECK-LABEL: @zero_sized_elem
#[no_mangle]
pub fn zero_sized_elem() {
Expand Down Expand Up @@ -76,17 +78,64 @@ pub fn u16_init_one_bytes() -> [u16; N] {
[const { u16::from_be_bytes([1, 1]) }; N]
}

// FIXME: undef bytes can just be initialized with the same value as the
// defined bytes, if the defines bytes are all the same.
// CHECK-LABEL: @option_none_init
#[no_mangle]
pub fn option_none_init() -> [Option<u8>; N] {
// CHECK-NOT: select
// CHECK-NOT: br
// CHECK-NOT: switch
// CHECK-NOT: icmp
// CHECK: call void @llvm.memset.p0
[const { None }; N]
}

// If there is partial provenance or some bytes are initialized and some are not,
// we can't really do better than initialize bytes or groups of bytes together.
// CHECK-LABEL: @option_maybe_uninit_init
#[no_mangle]
pub fn option_maybe_uninit_init() -> [MaybeUninit<u16>; N] {
// CHECK-NOT: select
// CHECK: br label %repeat_loop_header{{.*}}
// CHECK-NOT: switch
// CHECK: icmp
// CHECK-NOT: call void @llvm.memset.p0
[const {
let mut val: MaybeUninit<u16> = MaybeUninit::uninit();
let ptr = val.as_mut_ptr() as *mut u8;
unsafe {
ptr.write(0);
}
val
}; N]
}

#[repr(packed)]
struct Packed {
start: u8,
ptr: &'static (),
rest: u16,
rest2: u8,
}

// If there is partial provenance or some bytes are initialized and some are not,
// we can't really do better than initialize bytes or groups of bytes together.
// CHECK-LABEL: @option_maybe_uninit_provenance
#[no_mangle]
pub fn option_maybe_uninit_provenance() -> [MaybeUninit<Packed>; N] {
// CHECK-NOT: select
// CHECK: br label %repeat_loop_header{{.*}}
// CHECK-NOT: switch
// CHECK: icmp
// CHECK-NOT: call void @llvm.memset.p0
[None; N]
[const {
let mut val: MaybeUninit<Packed> = MaybeUninit::uninit();
unsafe {
let ptr = &raw mut (*val.as_mut_ptr()).ptr;
static HAS_ADDR: () = ();
ptr.write_unaligned(&HAS_ADDR);
}
val
}; N]
}

// Use an opaque function to prevent rustc from removing useless drops.
Expand Down
Loading