Skip to content
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

Unify Rvalue::Aggregate paths in cg_ssa #124999

Merged
merged 2 commits into from
May 13, 2024
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
9 changes: 7 additions & 2 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ impl<FieldIdx: Idx> FieldsShape<FieldIdx> {

/// Gets source indices of the fields by increasing offsets.
#[inline]
pub fn index_by_increasing_offset(&self) -> impl Iterator<Item = usize> + '_ {
pub fn index_by_increasing_offset(&self) -> impl ExactSizeIterator<Item = usize> + '_ {
let mut inverse_small = [0u8; 64];
let mut inverse_big = IndexVec::new();
let use_small = self.count() <= inverse_small.len();
Expand All @@ -1271,7 +1271,12 @@ impl<FieldIdx: Idx> FieldsShape<FieldIdx> {
}
}

(0..self.count()).map(move |i| match *self {
// Primitives don't really have fields in the way that structs do,
// but having this return an empty iterator for them is unhelpful
// since that makes them look kinda like ZSTs, which they're not.
let pseudofield_count = if let FieldsShape::Primitive = self { 1 } else { self.count() };

(0..pseudofield_count).map(move |i| match *self {
FieldsShape::Primitive | FieldsShape::Union(_) | FieldsShape::Array { .. } => i,
FieldsShape::Arbitrary { .. } => {
if use_small {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,19 @@ impl<V: CodegenObject> OperandValue<V> {
let (llval, llextra) = self.pointer_parts();
PlaceValue { llval, llextra, align }
}

pub(crate) fn is_expected_variant_for_type<'tcx, Cx: LayoutTypeMethods<'tcx>>(
&self,
cx: &Cx,
ty: TyAndLayout<'tcx>,
) -> bool {
match self {
OperandValue::ZeroSized => ty.is_zst(),
OperandValue::Immediate(_) => cx.is_backend_immediate(ty),
OperandValue::Pair(_, _) => cx.is_backend_scalar_pair(ty),
OperandValue::Ref(_) => cx.is_backend_ref(ty),
}
}
}

/// An `OperandRef` is an "SSA" reference to a Rust value, along with
Expand Down
24 changes: 5 additions & 19 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,24 +696,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
OperandRef { val: OperandValue::Immediate(static_), layout }
}
mir::Rvalue::Use(ref operand) => self.codegen_operand(bx, operand),
mir::Rvalue::Aggregate(box mir::AggregateKind::RawPtr(..), ref fields) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let layout = self.cx.layout_of(self.monomorphize(ty));
let [data, meta] = &*fields.raw else {
bug!("RawPtr fields: {fields:?}");
};
let data = self.codegen_operand(bx, data);
let meta = self.codegen_operand(bx, meta);
match (data.val, meta.val) {
(p @ OperandValue::Immediate(_), OperandValue::ZeroSized) => {
OperandRef { val: p, layout }
}
(OperandValue::Immediate(p), OperandValue::Immediate(m)) => {
OperandRef { val: OperandValue::Pair(p, m), layout }
}
_ => bug!("RawPtr operands {data:?} {meta:?}"),
}
}
mir::Rvalue::Repeat(..) => bug!("{rvalue:?} in codegen_rvalue_operand"),
mir::Rvalue::Aggregate(_, ref fields) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
Expand Down Expand Up @@ -748,6 +730,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
);

let val = OperandValue::from_immediates(inputs);
debug_assert!(
val.is_expected_variant_for_type(self.cx, layout),
"Made wrong variant {val:?} for type {layout:?}",
);
Comment on lines +733 to +736
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this to help make the ICE clearer in debug, since otherwise it hit this less-helpful one:

_ => bug!("not immediate: {:?}", self),

OperandRef { val, layout }
}
mir::Rvalue::ShallowInitBox(ref operand, content_ty) => {
Expand Down Expand Up @@ -792,7 +778,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
debug_assert!(
if bx.cx().type_has_metadata(ty) {
matches!(val, OperandValue::Pair(..))
} else {
} else {
matches!(val, OperandValue::Immediate(..))
},
"Address of place was unexpectedly {val:?} for pointee type {ty:?}",
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/mir-aggregate-no-alloca.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub fn make_cell_of_bool(b: bool) -> std::cell::Cell<bool> {
std::cell::Cell::new(b)
}

// CHECK-LABLE: { i8, i16 } @make_cell_of_bool_and_short(i1 noundef zeroext %b, i16 noundef %s)
// CHECK-LABEL: { i8, i16 } @make_cell_of_bool_and_short(i1 noundef zeroext %b, i16 noundef %s)
#[no_mangle]
pub fn make_cell_of_bool_and_short(b: bool, s: u16) -> std::cell::Cell<(bool, u16)> {
// CHECK-NOT: alloca
Expand Down
Loading