Skip to content

Commit ead2221

Browse files
committed
Avoid memset and memcpy of undef from MaybeUninit::uninit
1 parent ae2fc97 commit ead2221

File tree

8 files changed

+128
-44
lines changed

8 files changed

+128
-44
lines changed

compiler/rustc_codegen_ssa/src/base.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ pub(crate) fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
298298
let (base, info) = match bx.load_operand(src).val {
299299
OperandValue::Pair(base, info) => unsize_ptr(bx, base, src_ty, dst_ty, Some(info)),
300300
OperandValue::Immediate(base) => unsize_ptr(bx, base, src_ty, dst_ty, None),
301-
OperandValue::Ref(..) | OperandValue::ZeroSized => bug!(),
301+
OperandValue::Ref(..) | OperandValue::ZeroSized | OperandValue::Uninit => bug!(),
302302
};
303303
OperandValue::Pair(base, info).store(bx, dst);
304304
}

compiler/rustc_codegen_ssa/src/mir/block.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_target::callconv::{ArgAbi, CastTarget, FnAbi, PassMode};
1717
use tracing::{debug, info};
1818

1919
use super::operand::OperandRef;
20-
use super::operand::OperandValue::{Immediate, Pair, Ref, ZeroSized};
20+
use super::operand::OperandValue::{Immediate, Pair, Ref, Uninit, ZeroSized};
2121
use super::place::{PlaceRef, PlaceValue};
2222
use super::{CachedLlbb, FunctionCx, LocalRef};
2323
use crate::base::{self, is_call_from_compiler_builtins_to_upstream_monomorphization};
@@ -527,10 +527,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
527527

528528
PassMode::Direct(_) | PassMode::Pair(..) => {
529529
let op = self.codegen_consume(bx, mir::Place::return_place().as_ref());
530-
if let Ref(place_val) = op.val {
531-
bx.load_from_place(bx.backend_type(op.layout), place_val)
532-
} else {
533-
op.immediate_or_packed_pair(bx)
530+
match op.val {
531+
Uninit => bx.const_undef(bx.immediate_backend_type(op.layout)),
532+
Ref(place_val) => bx.load_from_place(bx.backend_type(op.layout), place_val),
533+
_ => op.immediate_or_packed_pair(bx),
534534
}
535535
}
536536

@@ -557,6 +557,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
557557
place_val.llval
558558
}
559559
ZeroSized => bug!("ZST return value shouldn't be in PassMode::Cast"),
560+
Uninit => {
561+
bx.ret_void();
562+
return;
563+
}
560564
};
561565
load_cast(bx, cast_ty, llslot, self.fn_abi.ret.layout.align.abi)
562566
}
@@ -1587,6 +1591,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
15871591
}
15881592
_ => bug!("ZST {op:?} wasn't ignored, but was passed with abi {arg:?}"),
15891593
},
1594+
Uninit => {
1595+
let scratch = PlaceRef::alloca(bx, arg.layout);
1596+
(scratch.val.llval, scratch.val.align, true)
1597+
}
15901598
};
15911599

15921600
if by_ref && !arg.is_indirect() {

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
341341
OperandValue::ZeroSized => {
342342
// These never have a value to talk about
343343
}
344+
OperandValue::Uninit => {
345+
// Better not have a useful name
346+
}
344347
},
345348
LocalRef::PendingOperand => {}
346349
}

compiler/rustc_codegen_ssa/src/mir/operand.rs

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,14 @@ pub enum OperandValue<V> {
6767
/// `is_zst` on its `Layout` returns `true`. Note however that
6868
/// these values can still require alignment.
6969
ZeroSized,
70+
Uninit,
7071
}
7172

7273
impl<V: CodegenObject> OperandValue<V> {
74+
pub(crate) fn is_uninit(&self) -> bool {
75+
matches!(self, OperandValue::Uninit)
76+
}
77+
7378
/// Treat this value as a pointer and return the data pointer and
7479
/// optional metadata as backend values.
7580
///
@@ -100,6 +105,7 @@ impl<V: CodegenObject> OperandValue<V> {
100105
ty: TyAndLayout<'tcx>,
101106
) -> bool {
102107
match self {
108+
OperandValue::Uninit => true,
103109
OperandValue::ZeroSized => ty.is_zst(),
104110
OperandValue::Immediate(_) => cx.is_backend_immediate(ty),
105111
OperandValue::Pair(_, _) => cx.is_backend_scalar_pair(ty),
@@ -144,6 +150,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
144150
) -> Self {
145151
let layout = bx.layout_of(ty);
146152

153+
if val.all_bytes_uninit(bx.tcx()) {
154+
return OperandRef { val: OperandValue::Uninit, layout };
155+
}
156+
147157
let val = match val {
148158
ConstValue::Scalar(x) => {
149159
let BackendRepr::Scalar(scalar) = layout.backend_repr else {
@@ -442,6 +452,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
442452

443453
// Read the tag/niche-encoded discriminant from memory.
444454
let tag_op = match self.val {
455+
OperandValue::Uninit => bug!("shouldn't load from uninit"),
445456
OperandValue::ZeroSized => bug!(),
446457
OperandValue::Immediate(_) | OperandValue::Pair(_, _) => {
447458
self.extract_field(fx, bx, tag_field.as_usize())
@@ -591,6 +602,28 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
591602
}
592603

593604
impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, Result<V, abi::Scalar>> {
605+
fn update_uninit<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
606+
bx: &mut Bx,
607+
tgt: &mut Result<V, abi::Scalar>,
608+
) {
609+
let to_scalar = tgt.unwrap_err();
610+
let bty = bx.cx().type_from_scalar(to_scalar);
611+
*tgt = Ok(bx.const_undef(bty));
612+
}
613+
614+
fn update<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
615+
bx: &mut Bx,
616+
tgt: &mut Result<V, abi::Scalar>,
617+
src: V,
618+
from_scalar: rustc_abi::Scalar,
619+
) {
620+
let from_bty = bx.cx().type_from_scalar(from_scalar);
621+
let to_scalar = tgt.unwrap_err();
622+
let to_bty = bx.cx().type_from_scalar(to_scalar);
623+
let imm = transmute_immediate(bx, src, from_scalar, from_bty, to_scalar, to_bty);
624+
*tgt = Ok(imm);
625+
}
626+
594627
pub(crate) fn insert_field<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
595628
&mut self,
596629
bx: &mut Bx,
@@ -614,37 +647,48 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, Result<V, abi::Scalar>> {
614647
(field_layout.is_zst(), field_offset == Size::ZERO)
615648
};
616649

617-
let mut update = |tgt: &mut Result<V, abi::Scalar>, src, from_scalar| {
618-
let from_bty = bx.cx().type_from_scalar(from_scalar);
619-
let to_scalar = tgt.unwrap_err();
620-
let to_bty = bx.cx().type_from_scalar(to_scalar);
621-
let imm = transmute_immediate(bx, src, from_scalar, from_bty, to_scalar, to_bty);
622-
*tgt = Ok(imm);
623-
};
624-
625650
match (operand.val, operand.layout.backend_repr) {
626651
(OperandValue::ZeroSized, _) if expect_zst => {}
627652
(OperandValue::Immediate(v), BackendRepr::Scalar(from_scalar)) => match &mut self.val {
628653
OperandValue::Immediate(val @ Err(_)) if is_zero_offset => {
629-
update(val, v, from_scalar);
654+
Self::update(bx, val, v, from_scalar);
630655
}
631656
OperandValue::Pair(fst @ Err(_), _) if is_zero_offset => {
632-
update(fst, v, from_scalar);
657+
Self::update(bx, fst, v, from_scalar);
633658
}
634659
OperandValue::Pair(_, snd @ Err(_)) if !is_zero_offset => {
635-
update(snd, v, from_scalar);
660+
Self::update(bx, snd, v, from_scalar);
661+
}
662+
_ => bug!("Tried to insert {operand:?} into {v:?}.{f:?} of {self:?}"),
663+
},
664+
(OperandValue::Uninit, BackendRepr::Scalar(_)) => match &mut self.val {
665+
OperandValue::Immediate(val @ Err(_)) if is_zero_offset => {
666+
Self::update_uninit(bx, val);
667+
}
668+
OperandValue::Pair(fst @ Err(_), _) if is_zero_offset => {
669+
Self::update_uninit(bx, fst);
670+
}
671+
OperandValue::Pair(_, snd @ Err(_)) if !is_zero_offset => {
672+
Self::update_uninit(bx, snd);
636673
}
637674
_ => bug!("Tried to insert {operand:?} into {v:?}.{f:?} of {self:?}"),
638675
},
639676
(OperandValue::Pair(a, b), BackendRepr::ScalarPair(from_sa, from_sb)) => {
640677
match &mut self.val {
641678
OperandValue::Pair(fst @ Err(_), snd @ Err(_)) => {
642-
update(fst, a, from_sa);
643-
update(snd, b, from_sb);
679+
Self::update(bx, fst, a, from_sa);
680+
Self::update(bx, snd, b, from_sb);
644681
}
645682
_ => bug!("Tried to insert {operand:?} into {v:?}.{f:?} of {self:?}"),
646683
}
647684
}
685+
(OperandValue::Uninit, BackendRepr::ScalarPair(..)) => match &mut self.val {
686+
OperandValue::Pair(fst @ Err(_), snd @ Err(_)) => {
687+
Self::update_uninit(bx, fst);
688+
Self::update_uninit(bx, snd);
689+
}
690+
_ => bug!("Tried to insert {operand:?} into {v:?}.{f:?} of {self:?}"),
691+
},
648692
_ => bug!("Unsupported operand {operand:?} inserting into {v:?}.{f:?} of {self:?}"),
649693
}
650694
}
@@ -663,6 +707,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, Result<V, abi::Scalar>> {
663707
};
664708

665709
let val = match val {
710+
OperandValue::Uninit => OperandValue::Uninit,
666711
OperandValue::ZeroSized => OperandValue::ZeroSized,
667712
OperandValue::Immediate(v) => OperandValue::Immediate(unwrap(v)),
668713
OperandValue::Pair(a, b) => OperandValue::Pair(unwrap(a), unwrap(b)),
@@ -739,6 +784,13 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
739784
) {
740785
debug!("OperandRef::store: operand={:?}, dest={:?}", self, dest);
741786
match self {
787+
OperandValue::Uninit => {
788+
// Ideally we'd hint to the backend that the destination is deinitialized by the
789+
// store. But in practice the destination is almost always uninit already because
790+
// OperandValue::Uninit is pretty much only produced by MaybeUninit::uninit.
791+
// Attempting to generate a hint by calling memset with undef mostly seems to
792+
// confuse LLVM.
793+
}
742794
OperandValue::ZeroSized => {
743795
// Avoid generating stores of zero-sized values, because the only way to have a
744796
// zero-sized value is through `undef`/`poison`, and the store itself is useless.

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
6767
base::coerce_unsized_into(bx, scratch, dest);
6868
scratch.storage_dead(bx);
6969
}
70+
OperandValue::Uninit => {}
7071
OperandValue::Ref(val) => {
7172
if val.llextra.is_some() {
7273
bug!("unsized coercion on an unsized rvalue");
@@ -90,24 +91,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
9091
return;
9192
}
9293

93-
// When the element is a const with all bytes uninit, emit a single memset that
94-
// writes undef to the entire destination.
95-
if let mir::Operand::Constant(const_op) = elem {
96-
let val = self.eval_mir_constant(const_op);
97-
if val.all_bytes_uninit(self.cx.tcx()) {
98-
let size = bx.const_usize(dest.layout.size.bytes());
99-
bx.memset(
100-
dest.val.llval,
101-
bx.const_undef(bx.type_i8()),
102-
size,
103-
dest.val.align,
104-
MemFlags::empty(),
105-
);
106-
return;
107-
}
108-
}
109-
11094
let cg_elem = self.codegen_operand(bx, elem);
95+
// Normally the check for uninit is handled inside the operand helpers, but in this
96+
// one case we want to bail early so that we don't generate the loop form with an
97+
// empty body.
98+
if cg_elem.val.is_uninit() {
99+
return;
100+
}
111101

112102
let try_init_all_same = |bx: &mut Bx, v| {
113103
let start = dest.val.llval;
@@ -206,7 +196,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
206196
}
207197

208198
match src.val {
209-
OperandValue::Ref(..) | OperandValue::ZeroSized => {
199+
OperandValue::Ref(..) | OperandValue::ZeroSized | OperandValue::Uninit => {
210200
span_bug!(
211201
self.mir.span,
212202
"Operand path should have handled transmute \
@@ -251,6 +241,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
251241
let cast_kind = self.value_kind(cast);
252242

253243
match operand.val {
244+
OperandValue::Uninit => Some(OperandValue::Uninit),
254245
OperandValue::Ref(source_place_val) => {
255246
assert_eq!(source_place_val.llextra, None);
256247
assert_matches!(operand_kind, OperandValueKind::Ref);

library/core/src/mem/maybe_uninit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ impl<T> MaybeUninit<T> {
328328
#[inline(always)]
329329
#[rustc_diagnostic_item = "maybe_uninit_uninit"]
330330
pub const fn uninit() -> MaybeUninit<T> {
331-
MaybeUninit { uninit: () }
331+
const { MaybeUninit { uninit: () } }
332332
}
333333

334334
/// Creates a new `MaybeUninit<T>` in an uninitialized state, with the memory being

tests/codegen/maybeuninit.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//@ compile-flags: -Copt-level=3 -Cdebuginfo=0
2+
3+
// This is a regression test for https://github.com/rust-lang/rust/issues/139355 as well as
4+
// regressions I introduced while implementing a solution.
5+
6+
#![crate_type = "lib"]
7+
8+
use std::mem::MaybeUninit;
9+
10+
// CHECK-LABEL: @create_small_uninit_array
11+
#[no_mangle]
12+
fn create_small_uninit_array() -> [MaybeUninit<u8>; 4] {
13+
// CHECK-NEXT: start:
14+
// CHECK-NEXT: ret i32 undef
15+
[MaybeUninit::<u8>::uninit(); 4]
16+
}
17+
18+
// CHECK-LABEL: @create_nested_uninit_array
19+
#[no_mangle]
20+
fn create_nested_uninit_array() -> [[MaybeUninit<u8>; 4]; 100] {
21+
// CHECK-NEXT: start:
22+
// CHECK-NEXT: ret void
23+
[[MaybeUninit::<u8>::uninit(); 4]; 100]
24+
}
25+
26+
// CHECK-LABEL: @create_ptr
27+
#[no_mangle]
28+
fn create_ptr() -> MaybeUninit<&'static str> {
29+
// CHECK-NEXT: start:
30+
// CHECK-NEXT: ret { ptr, i64 } undef
31+
MaybeUninit::uninit()
32+
}

tests/codegen/uninit-consts.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,18 @@ pub struct PartiallyUninit {
1111
y: MaybeUninit<[u8; 10]>,
1212
}
1313

14-
// CHECK: [[FULLY_UNINIT:@.*]] = private unnamed_addr constant [10 x i8] undef
15-
1614
// CHECK: [[PARTIALLY_UNINIT:@.*]] = private unnamed_addr constant <{ [4 x i8], [12 x i8] }> <{ [4 x i8] c"{{\\EF\\BE\\AD\\DE|\\DE\\AD\\BE\\EF}}", [12 x i8] undef }>, align 4
1715

1816
// This shouldn't contain undef, since it contains more chunks
1917
// than the default value of uninit_const_chunk_threshold.
2018
// CHECK: [[UNINIT_PADDING_HUGE:@.*]] = private unnamed_addr constant [32768 x i8] c"{{.+}}", align 4
2119

22-
// CHECK: [[FULLY_UNINIT_HUGE:@.*]] = private unnamed_addr constant [16384 x i8] undef
23-
2420
// CHECK-LABEL: @fully_uninit
2521
#[no_mangle]
2622
pub const fn fully_uninit() -> MaybeUninit<[u8; 10]> {
2723
const M: MaybeUninit<[u8; 10]> = MaybeUninit::uninit();
28-
// CHECK: call void @llvm.memcpy.{{.+}}(ptr align 1 %_0, ptr align 1 {{.*}}[[FULLY_UNINIT]]{{.*}}, i{{(32|64)}} 10, i1 false)
24+
// CHECK-NEXT: start:
25+
// CHECK-NEXT: ret void
2926
M
3027
}
3128

@@ -49,6 +46,7 @@ pub const fn uninit_padding_huge() -> [(u32, u8); 4096] {
4946
#[no_mangle]
5047
pub const fn fully_uninit_huge() -> MaybeUninit<[u32; 4096]> {
5148
const F: MaybeUninit<[u32; 4096]> = MaybeUninit::uninit();
52-
// CHECK: call void @llvm.memcpy.{{.+}}(ptr align 4 %_0, ptr align 4 {{.*}}[[FULLY_UNINIT_HUGE]]{{.*}}, i{{(32|64)}} 16384, i1 false)
49+
// CHECK-NEXT: start:
50+
// CHECK-NEXT: ret void
5351
F
5452
}

0 commit comments

Comments
 (0)