Skip to content

Commit

Permalink
Auto merge of rust-lang#135335 - oli-obk:push-zxwssomxxtnq, r=saethlin
Browse files Browse the repository at this point in the history
codegen: store ScalarPair via memset when one side is undef and the other side can be memset

Basically since `undef` can be any byte, it can also be the byte(s) that are in the non-undef parts of a value. So we can just treat the `undef` at not being there and only look at the initialized bytes and memset over them

fixes rust-lang#104290

based on rust-lang#135258
  • Loading branch information
bors committed Jan 21, 2025
2 parents ebbe638 + 8f5f5e5 commit a7a6c64
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 30 deletions.
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

0 comments on commit a7a6c64

Please sign in to comment.