Skip to content

Commit c2c6f74

Browse files
committed
Auto merge of #152864 - TKanX:bugfix/123183-array-cast-abi-noundef, r=RalfJung
perf(codegen): Restore `noundef` On `PassMode::Cast` Args In Rust ABI ### Summary: #### Problem: Small aggregate arguments passed via `PassMode::Cast` in the Rust ABI (e.g. `[u32; 2]` cast to `i64`) are missing `noundef` in the emitted LLVM IR, even when the type contains no uninit bytes: ```rust #[no_mangle] pub fn f(v: [u32; 2]) -> u32 { v[0] } ``` ```llvm ; expected: define i32 @f(i64 noundef %0) ; actual: define i32 @f(i64 %0) ← noundef missing ``` This blocks LLVM from applying optimizations that require value-defined semantics on function arguments. #### Root Cause: `adjust_for_rust_abi` calls `arg.cast_to(Reg::Integer)`, which internally creates a `CastTarget` with `ArgAttributes::new()` — always empty. Any validity attribute that was present before the cast is silently dropped. This affects all `PassMode::Cast` arguments and return values in the Rust ABI: plain arrays, newtype wrappers, and any `BackendRepr::Memory` type small enough to fit in a register. A prior attempt (#127210) used `Ty`/`repr` attributes to detect padding. #### Solution: After `adjust_for_rust_abi`, iterate all `PassMode::Cast` args and the return value. For each, call `layout_is_noundef` on the original layout; if it returns `true`, set `NoUndef` on the `CastTarget`'s `attrs`. `layout_is_noundef` uses only the computed layout — `BackendRepr`, `FieldsShape`, `Variants`, `Scalar::is_uninit_valid()` — and never touches `Ty` or repr attributes. **Anything it cannot prove returns `false`.** Covered cases: - `Scalar` / `ScalarPair` (both halves initialized, fields contiguous) - `FieldsShape::Array` (element type recursively uninit-free) - `FieldsShape::Arbitrary` with `Variants::Single` (fields cover `0..size` with no gaps, each recursively uninit-free) — handles newtype wrappers, multi-field structs, single-variant enums, `repr(transparent)`, `repr(C)` wrappers Conservatively excluded with FIXMEs: - Multi-variant enums (per-variant padding analysis needed) - Foreign-ABI casts (cast target may exceed layout size, needs a size guard) ### Changes: - `compiler/rustc_ty_utils/src/abi.rs`: add restoration loop after `adjust_for_rust_abi`; add `layout_is_noundef` and `fields_cover_layout`. - `tests/codegen-llvm/abi-noundef-cast.rs`: new FileCheck test covering arrays, newtype wrappers (`repr(Rust)`, `repr(transparent)`, `repr(C)`), multi-field structs, single-variant enums, return values, and negative cases (`MaybeUninit`, struct with trailing padding). - `tests/codegen-llvm/debuginfo-dse.rs`: update one CHECK pattern — `Aggregate_4xi8` (`struct { i8, i8, i8, i8 }`) now correctly gets `noundef`. Fixes #123183. r? @RalfJung
2 parents 8215374 + 97bd985 commit c2c6f74

5 files changed

Lines changed: 282 additions & 5 deletions

File tree

compiler/rustc_target/src/callconv/mod.rs

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use std::{fmt, iter};
22

33
use rustc_abi::{
4-
AddressSpace, Align, BackendRepr, CanonAbi, ExternAbi, HasDataLayout, Primitive, Reg, RegKind,
5-
Scalar, Size, TyAbiInterface, TyAndLayout,
4+
AddressSpace, Align, BackendRepr, CanonAbi, ExternAbi, FieldsShape, HasDataLayout, Primitive,
5+
Reg, RegKind, Scalar, Size, TyAbiInterface, TyAndLayout, Variants,
66
};
77
use rustc_macros::HashStable_Generic;
88

@@ -514,6 +514,11 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
514514
self.mode = PassMode::Cast { cast: Box::new(target.into()), pad_i32: false };
515515
}
516516

517+
pub fn cast_to_with_attrs<T: Into<CastTarget>>(&mut self, target: T, attrs: ArgAttributes) {
518+
self.mode =
519+
PassMode::Cast { cast: Box::new(target.into().with_attrs(attrs)), pad_i32: false };
520+
}
521+
517522
pub fn cast_to_and_pad_i32<T: Into<CastTarget>>(&mut self, target: T, pad_i32: bool) {
518523
self.mode = PassMode::Cast { cast: Box::new(target.into()), pad_i32 };
519524
}
@@ -801,7 +806,12 @@ impl<'a, Ty> FnAbi<'a, Ty> {
801806
// We want to pass small aggregates as immediates, but using
802807
// an LLVM aggregate type for this leads to bad optimizations,
803808
// so we pick an appropriately sized integer type instead.
804-
arg.cast_to(Reg { kind: RegKind::Integer, size });
809+
let attr = if layout_is_noundef(arg.layout, cx) {
810+
ArgAttribute::NoUndef
811+
} else {
812+
ArgAttribute::default()
813+
};
814+
arg.cast_to_with_attrs(Reg { kind: RegKind::Integer, size }, attr.into());
805815
}
806816
}
807817

@@ -836,6 +846,66 @@ impl<'a, Ty> FnAbi<'a, Ty> {
836846
}
837847
}
838848

849+
/// Determines whether `layout` contains no uninit bytes (no padding, no unions),
850+
/// using only the computed layout.
851+
///
852+
/// Conservative: returns `false` for anything it cannot prove fully initialized,
853+
/// including multi-variant enums and SIMD vectors.
854+
// FIXME: extend to multi-variant enums (per-variant padding analysis needed).
855+
fn layout_is_noundef<'a, Ty, C>(layout: TyAndLayout<'a, Ty>, cx: &C) -> bool
856+
where
857+
Ty: TyAbiInterface<'a, C> + Copy,
858+
C: HasDataLayout,
859+
{
860+
match layout.backend_repr {
861+
BackendRepr::Scalar(scalar) => !scalar.is_uninit_valid(),
862+
BackendRepr::ScalarPair(s1, s2) => {
863+
!s1.is_uninit_valid()
864+
&& !s2.is_uninit_valid()
865+
// Ensure there is no padding.
866+
&& s1.size(cx) + s2.size(cx) == layout.size
867+
}
868+
BackendRepr::Memory { .. } => match layout.fields {
869+
FieldsShape::Primitive | FieldsShape::Union(_) => false,
870+
// Array elements are at stride offsets with no inter-element gaps.
871+
FieldsShape::Array { stride: _, count } => {
872+
count == 0 || layout_is_noundef(layout.field(cx, 0), cx)
873+
}
874+
FieldsShape::Arbitrary { .. } => {
875+
// With `Variants::Multiple`, `layout.fields` only covers shared
876+
// bytes (niche/discriminant); per-variant data is absent, so
877+
// full coverage cannot be proven.
878+
matches!(layout.variants, Variants::Single { .. }) && fields_are_noundef(layout, cx)
879+
}
880+
},
881+
BackendRepr::SimdVector { .. } | BackendRepr::ScalableVector { .. } => false,
882+
}
883+
}
884+
885+
/// Returns `true` if the fields of `layout` contiguously cover bytes `0..layout.size`
886+
/// with no padding gaps and each field is recursively `layout_is_noundef`.
887+
fn fields_are_noundef<'a, Ty, C>(layout: TyAndLayout<'a, Ty>, cx: &C) -> bool
888+
where
889+
Ty: TyAbiInterface<'a, C> + Copy,
890+
C: HasDataLayout,
891+
{
892+
let mut cursor = Size::ZERO;
893+
for i in layout.fields.index_by_increasing_offset() {
894+
let field = layout.field(cx, i);
895+
if field.size == Size::ZERO {
896+
continue;
897+
}
898+
if layout.fields.offset(i) != cursor {
899+
return false;
900+
}
901+
if !layout_is_noundef(field, cx) {
902+
return false;
903+
}
904+
cursor += field.size;
905+
}
906+
cursor == layout.size
907+
}
908+
839909
// Some types are used a lot. Make sure they don't unintentionally get bigger.
840910
#[cfg(target_pointer_width = "64")]
841911
mod size_asserts {

compiler/rustc_ty_utils/src/abi.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,7 @@ fn fn_abi_adjust_for_abi<'tcx>(
627627

628628
if abi.is_rustic_abi() {
629629
fn_abi.adjust_for_rust_abi(cx);
630+
630631
// Look up the deduced parameter attributes for this function, if we have its def ID and
631632
// we're optimizing in non-incremental mode. We'll tag its parameters with those attributes
632633
// as appropriate.
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
// Verify that `PassMode::Cast` arguments/returns in the Rust ABI carry `noundef`
2+
// when the original layout provably contains no uninit bytes, and correctly omit
3+
// it when uninit bytes or padding may be present.
4+
//
5+
// See <https://github.com/rust-lang/rust/issues/123183>.
6+
7+
//@ compile-flags: -Copt-level=3 -C no-prepopulate-passes
8+
//@ only-64bit
9+
10+
#![crate_type = "lib"]
11+
12+
use std::mem::MaybeUninit;
13+
14+
// CHECK-LABEL: @arg_array_u32x2(
15+
// CHECK-SAME: i64 noundef
16+
#[no_mangle]
17+
pub fn arg_array_u32x2(v: [u32; 2]) -> u32 {
18+
v[0]
19+
}
20+
21+
// CHECK-LABEL: @arg_array_u8x4(
22+
// CHECK-SAME: i32 noundef
23+
#[no_mangle]
24+
pub fn arg_array_u8x4(v: [u8; 4]) -> u8 {
25+
v[0]
26+
}
27+
28+
// CHECK-LABEL: @arg_nested_array(
29+
// CHECK-SAME: i64 noundef
30+
#[no_mangle]
31+
pub fn arg_nested_array(v: [[u8; 2]; 4]) -> u8 {
32+
v[0][0]
33+
}
34+
35+
// CHECK-LABEL: @arg_array_bool(
36+
// CHECK-SAME: i64 noundef
37+
#[no_mangle]
38+
pub fn arg_array_bool(v: [bool; 8]) -> bool {
39+
v[0]
40+
}
41+
42+
struct FourU8 {
43+
a: u8,
44+
b: u8,
45+
c: u8,
46+
d: u8,
47+
}
48+
49+
// CHECK-LABEL: @arg_four_u8(
50+
// CHECK-SAME: i32 noundef
51+
#[no_mangle]
52+
pub fn arg_four_u8(v: FourU8) -> u8 {
53+
v.a
54+
}
55+
56+
struct Wrapper([u32; 2]);
57+
58+
// CHECK-LABEL: @arg_newtype_wrapper(
59+
// CHECK-SAME: i64 noundef
60+
#[no_mangle]
61+
pub fn arg_newtype_wrapper(v: Wrapper) -> u32 {
62+
(v.0)[0]
63+
}
64+
65+
enum SingleVariant {
66+
Only([u32; 2]),
67+
}
68+
69+
// CHECK-LABEL: @arg_single_variant_enum(
70+
// CHECK-SAME: i64 noundef
71+
#[no_mangle]
72+
pub fn arg_single_variant_enum(v: SingleVariant) -> u32 {
73+
match v {
74+
SingleVariant::Only(a) => a[0],
75+
}
76+
}
77+
78+
struct ContainsScalarPair {
79+
a: (u16, u16),
80+
b: u32,
81+
}
82+
83+
// CHECK-LABEL: @arg_contains_scalar_pair(
84+
// CHECK-SAME: i64 noundef
85+
#[no_mangle]
86+
pub fn arg_contains_scalar_pair(v: ContainsScalarPair) -> u32 {
87+
v.b
88+
}
89+
90+
// CHECK: define noundef i64 @ret_array_u32x2(
91+
#[no_mangle]
92+
pub fn ret_array_u32x2(x: u32, y: u32) -> [u32; 2] {
93+
[x, y]
94+
}
95+
96+
// CHECK-LABEL: @arg_maybeuninit_u8x8(
97+
// CHECK-SAME: i64 %
98+
#[no_mangle]
99+
pub fn arg_maybeuninit_u8x8(v: [MaybeUninit<u8>; 8]) -> MaybeUninit<u8> {
100+
v[0]
101+
}
102+
103+
enum MultiVariant {
104+
A(u8),
105+
B(u16),
106+
C,
107+
}
108+
109+
// CHECK-LABEL: @arg_multi_variant_enum(
110+
// CHECK-SAME: i32 %
111+
#[no_mangle]
112+
pub fn arg_multi_variant_enum(v: MultiVariant) -> u8 {
113+
match v {
114+
MultiVariant::A(x) => x,
115+
MultiVariant::B(_) | MultiVariant::C => 0,
116+
}
117+
}
118+
119+
#[repr(C)]
120+
struct HasFieldGap {
121+
a: u8,
122+
b: u16,
123+
c: u8,
124+
}
125+
126+
// CHECK-LABEL: @arg_struct_field_gap(
127+
// CHECK-SAME: i48 %
128+
#[no_mangle]
129+
pub fn arg_struct_field_gap(v: HasFieldGap) -> u8 {
130+
v.a
131+
}
132+
133+
#[repr(C)]
134+
struct HasPaddedPairField {
135+
a: (u8, u16),
136+
b: u8,
137+
}
138+
139+
// CHECK-LABEL: @arg_struct_padded_pair_field(
140+
// CHECK-SAME: i48 %
141+
#[no_mangle]
142+
pub fn arg_struct_padded_pair_field(v: HasPaddedPairField) -> u8 {
143+
v.b
144+
}
145+
146+
#[repr(C)]
147+
struct HasUndefPairField {
148+
a: (MaybeUninit<u16>, u16),
149+
b: u32,
150+
}
151+
152+
// CHECK-LABEL: @arg_struct_undef_pair_field(
153+
// CHECK-SAME: i64 %
154+
#[no_mangle]
155+
pub fn arg_struct_undef_pair_field(v: HasUndefPairField) -> u32 {
156+
v.b
157+
}
158+
159+
// CHECK-LABEL: @arg_triple_maybeuninit_u8(
160+
// CHECK-SAME: i24 %
161+
#[no_mangle]
162+
pub fn arg_triple_maybeuninit_u8(
163+
v: (MaybeUninit<u8>, MaybeUninit<u8>, MaybeUninit<u8>),
164+
) -> MaybeUninit<u8> {
165+
v.0
166+
}
167+
168+
#[repr(C)]
169+
struct HasTrailingPadding {
170+
x: u32,
171+
y: u16,
172+
z: u8,
173+
}
174+
175+
// CHECK-LABEL: @arg_struct_trailing_pad(
176+
// CHECK-SAME: i64 %
177+
#[no_mangle]
178+
pub fn arg_struct_trailing_pad(v: HasTrailingPadding) -> u32 {
179+
v.x
180+
}
181+
182+
// CHECK-LABEL: @arg_tuple_i8_i16(
183+
// CHECK-SAME: i8 noundef
184+
// CHECK-SAME: i16 noundef
185+
#[no_mangle]
186+
pub fn arg_tuple_i8_i16(v: (i8, i16)) -> i8 {
187+
v.0
188+
}
189+
190+
// CHECK-LABEL: @arg_tuple_i16_maybeuninit(
191+
// CHECK-SAME: i16 noundef
192+
// CHECK-SAME: i16 %
193+
#[no_mangle]
194+
pub fn arg_tuple_i16_maybeuninit(v: (i16, MaybeUninit<i16>)) -> i16 {
195+
v.0
196+
}
197+
198+
// CHECK-LABEL: @arg_result_i32(
199+
// CHECK-SAME: i32 noundef
200+
// CHECK-SAME: i32 noundef
201+
#[no_mangle]
202+
pub fn arg_result_i32(v: Result<i32, i32>) -> i32 {
203+
match v {
204+
Ok(x) | Err(x) => x,
205+
}
206+
}

tests/codegen-llvm/debuginfo-dse.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ fn direct(
148148
// Arguments are passed through registers, the final values are poison.
149149
#[unsafe(no_mangle)]
150150
fn cast(aggregate_4xi8: Aggregate_4xi8) {
151-
// CHECK-LABEL: define{{( dso_local)?}} void @cast(i32 %0)
151+
// CHECK-LABEL: define{{( dso_local)?}} void @cast(i32 noundef %0)
152152
// CHECK: call void @opaque_fn()
153153
opaque_fn();
154154
// The temporary allocated variable is eliminated.

tests/ui/abi/pass-indirectly-attr.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ error: fn_abi_of(extern_rust) = FnAbi {
143143
is_consecutive: false,
144144
},
145145
attrs: ArgAttributes {
146-
regular: ,
146+
regular: NoUndef,
147147
arg_ext: None,
148148
pointee_size: Size(0 bytes),
149149
pointee_align: None,

0 commit comments

Comments
 (0)