Skip to content

Commit ece55d4

Browse files
committed
Auto merge of #94130 - erikdesjardins:partially, r=nikic
Use undef for (some) partially-uninit constants There needs to be some limit to avoid perf regressions on large arrays with undef in each element (see comment in the code). Fixes: #84565 Original PR: #83698 Depends on LLVM 14: #93577
2 parents f6a7993 + 5bf8303 commit ece55d4

File tree

7 files changed

+44
-66
lines changed

7 files changed

+44
-66
lines changed

compiler/rustc_codegen_llvm/src/consts.rs

+19-22
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::base;
22
use crate::common::CodegenCx;
33
use crate::debuginfo;
44
use crate::llvm::{self, True};
5+
use crate::llvm_util;
56
use crate::type_::Type;
67
use crate::type_of::LayoutLlvmExt;
78
use crate::value::Value;
@@ -37,7 +38,7 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) ->
3738
alloc: &'a Allocation,
3839
range: Range<usize>,
3940
) {
40-
let mut chunks = alloc
41+
let chunks = alloc
4142
.init_mask()
4243
.range_as_init_chunks(Size::from_bytes(range.start), Size::from_bytes(range.end));
4344

@@ -53,30 +54,26 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) ->
5354
}
5455
};
5556

56-
// Generating partially-uninit consts inhibits optimizations, so it is disabled by default.
57-
// See https://github.com/rust-lang/rust/issues/84565.
58-
let allow_partially_uninit =
59-
match cx.sess().opts.debugging_opts.partially_uninit_const_threshold {
60-
Some(max) => range.len() <= max,
61-
None => false,
62-
};
57+
// Generating partially-uninit consts is limited to small numbers of chunks,
58+
// to avoid the cost of generating large complex const expressions.
59+
// For example, `[(u32, u8); 1024 * 1024]` contains uninit padding in each element,
60+
// and would result in `{ [5 x i8] zeroinitializer, [3 x i8] undef, ...repeat 1M times... }`.
61+
let max = if llvm_util::get_version() < (14, 0, 0) {
62+
// Generating partially-uninit consts inhibits optimizations in LLVM < 14.
63+
// See https://github.com/rust-lang/rust/issues/84565.
64+
1
65+
} else {
66+
cx.sess().opts.debugging_opts.uninit_const_chunk_threshold
67+
};
68+
let allow_uninit_chunks = chunks.clone().take(max.saturating_add(1)).count() <= max;
6369

64-
if allow_partially_uninit {
70+
if allow_uninit_chunks {
6571
llvals.extend(chunks.map(chunk_to_llval));
6672
} else {
67-
let llval = match (chunks.next(), chunks.next()) {
68-
(Some(chunk), None) => {
69-
// exactly one chunk, either fully init or fully uninit
70-
chunk_to_llval(chunk)
71-
}
72-
_ => {
73-
// partially uninit, codegen as if it was initialized
74-
// (using some arbitrary value for uninit bytes)
75-
let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(range);
76-
cx.const_bytes(bytes)
77-
}
78-
};
79-
llvals.push(llval);
73+
// If this allocation contains any uninit bytes, codegen as if it was initialized
74+
// (using some arbitrary value for uninit bytes).
75+
let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(range);
76+
llvals.push(cx.const_bytes(bytes));
8077
}
8178
}
8279

compiler/rustc_interface/src/tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,6 @@ fn test_debugging_options_tracking_hash() {
758758
tracked!(osx_rpath_install_name, true);
759759
tracked!(panic_abort_tests, true);
760760
tracked!(panic_in_drop, PanicStrategy::Abort);
761-
tracked!(partially_uninit_const_threshold, Some(123));
762761
tracked!(pick_stable_methods_before_any_unstable, false);
763762
tracked!(plt, Some(true));
764763
tracked!(polonius, true);
@@ -789,6 +788,7 @@ fn test_debugging_options_tracking_hash() {
789788
tracked!(trap_unreachable, Some(false));
790789
tracked!(treat_err_as_bug, NonZeroUsize::new(1));
791790
tracked!(tune_cpu, Some(String::from("abc")));
791+
tracked!(uninit_const_chunk_threshold, 123);
792792
tracked!(unleash_the_miri_inside_of_you, true);
793793
tracked!(use_ctors_section, Some(true));
794794
tracked!(verify_llvm_ir, true);

compiler/rustc_middle/src/mir/interpret/allocation.rs

+1
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,7 @@ impl InitMask {
957957
}
958958

959959
/// Yields [`InitChunk`]s. See [`InitMask::range_as_init_chunks`].
960+
#[derive(Clone)]
960961
pub struct InitChunkIter<'a> {
961962
init_mask: &'a InitMask,
962963
/// Whether the next chunk we will return is initialized.

compiler/rustc_session/src/options.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1336,9 +1336,6 @@ options! {
13361336
"panic strategy for panics in drops"),
13371337
parse_only: bool = (false, parse_bool, [UNTRACKED],
13381338
"parse only; do not compile, assemble, or link (default: no)"),
1339-
partially_uninit_const_threshold: Option<usize> = (None, parse_opt_number, [TRACKED],
1340-
"allow generating const initializers with mixed init/uninit bytes, \
1341-
and set the maximum total size of a const allocation for which this is allowed (default: never)"),
13421339
perf_stats: bool = (false, parse_bool, [UNTRACKED],
13431340
"print some performance-related statistics (default: no)"),
13441341
pick_stable_methods_before_any_unstable: bool = (true, parse_bool, [TRACKED],
@@ -1483,6 +1480,9 @@ options! {
14831480
"in diagnostics, use heuristics to shorten paths referring to items"),
14841481
ui_testing: bool = (false, parse_bool, [UNTRACKED],
14851482
"emit compiler diagnostics in a form suitable for UI testing (default: no)"),
1483+
uninit_const_chunk_threshold: usize = (16, parse_number, [TRACKED],
1484+
"allow generating const initializers with mixed init/uninit chunks, \
1485+
and set the maximum number of chunks for which this is allowed (default: 16)"),
14861486
unleash_the_miri_inside_of_you: bool = (false, parse_bool, [TRACKED],
14871487
"take the brakes off const evaluation. NOTE: this is unsound (default: no)"),
14881488
unpretty: Option<String> = (None, parse_unpretty, [UNTRACKED],

src/test/codegen/consts.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// compile-flags: -C no-prepopulate-passes
2-
//
2+
// min-llvm-version: 14.0
33

44
#![crate_type = "lib"]
55

@@ -43,14 +43,14 @@ pub fn inline_enum_const() -> E<i8, i16> {
4343
#[no_mangle]
4444
pub fn low_align_const() -> E<i16, [i16; 3]> {
4545
// Check that low_align_const and high_align_const use the same constant
46-
// CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 2 %1, i8* align 2 getelementptr inbounds (<{ [8 x i8] }>, <{ [8 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false)
46+
// CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 2 %1, i8* align 2 getelementptr inbounds (<{ [4 x i8], [4 x i8] }>, <{ [4 x i8], [4 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false)
4747
*&E::A(0)
4848
}
4949

5050
// CHECK-LABEL: @high_align_const
5151
#[no_mangle]
5252
pub fn high_align_const() -> E<i16, i32> {
5353
// Check that low_align_const and high_align_const use the same constant
54-
// CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [8 x i8] }>, <{ [8 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false)
54+
// CHECK: memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [4 x i8], [4 x i8] }>, <{ [4 x i8], [4 x i8] }>* [[LOW_HIGH]], i32 0, i32 0, i32 0), i{{(32|64)}} 8, i1 false)
5555
*&E::A(0)
5656
}

src/test/codegen/uninit-consts-allow-partially-uninit.rs

-35
This file was deleted.

src/test/codegen/uninit-consts.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// compile-flags: -C no-prepopulate-passes
2+
// min-llvm-version: 14.0
23

34
// Check that we use undef (and not zero) for uninitialized bytes in constants.
45

@@ -12,7 +13,13 @@ pub struct PartiallyUninit {
1213
}
1314

1415
// CHECK: [[FULLY_UNINIT:@[0-9]+]] = private unnamed_addr constant <{ [10 x i8] }> undef
15-
// CHECK: [[PARTIALLY_UNINIT:@[0-9]+]] = private unnamed_addr constant <{ [16 x i8] }> <{ [16 x i8] c"\EF\BE\AD\DE\00\00\00\00\00\00\00\00\00\00\00\00" }>, align 4
16+
17+
// CHECK: [[PARTIALLY_UNINIT:@[0-9]+]] = private unnamed_addr constant <{ [4 x i8], [12 x i8] }> <{ [4 x i8] c"\EF\BE\AD\DE", [12 x i8] undef }>, align 4
18+
19+
// This shouldn't contain undef, since it contains more chunks
20+
// than the default value of uninit_const_chunk_threshold.
21+
// CHECK: [[UNINIT_PADDING_HUGE:@[0-9]+]] = private unnamed_addr constant <{ [32768 x i8] }> <{ [32768 x i8] c"{{.+}}" }>, align 4
22+
1623
// CHECK: [[FULLY_UNINIT_HUGE:@[0-9]+]] = private unnamed_addr constant <{ [16384 x i8] }> undef
1724

1825
// CHECK-LABEL: @fully_uninit
@@ -27,7 +34,15 @@ pub const fn fully_uninit() -> MaybeUninit<[u8; 10]> {
2734
#[no_mangle]
2835
pub const fn partially_uninit() -> PartiallyUninit {
2936
const X: PartiallyUninit = PartiallyUninit { x: 0xdeadbeef, y: MaybeUninit::uninit() };
30-
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [16 x i8] }>, <{ [16 x i8] }>* [[PARTIALLY_UNINIT]], i32 0, i32 0, i32 0), i{{(32|64)}} 16, i1 false)
37+
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [4 x i8], [12 x i8] }>, <{ [4 x i8], [12 x i8] }>* [[PARTIALLY_UNINIT]], i32 0, i32 0, i32 0), i{{(32|64)}} 16, i1 false)
38+
X
39+
}
40+
41+
// CHECK-LABEL: @uninit_padding_huge
42+
#[no_mangle]
43+
pub const fn uninit_padding_huge() -> [(u32, u8); 4096] {
44+
const X: [(u32, u8); 4096] = [(123, 45); 4096];
45+
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{(32|64)}}(i8* align 4 %1, i8* align 4 getelementptr inbounds (<{ [32768 x i8] }>, <{ [32768 x i8] }>* [[UNINIT_PADDING_HUGE]], i32 0, i32 0, i32 0), i{{(32|64)}} 32768, i1 false)
3146
X
3247
}
3348

0 commit comments

Comments
 (0)