Skip to content

Commit

Permalink
Rollup merge of #109582 - scottmcm:local-ref-pending, r=oli-obk
Browse files Browse the repository at this point in the history
Refactor: Separate `LocalRef` variant for not-evaluated-yet operands

As I was reading through this, I noticed that almost every place that was using this needed to distinguish between Some vs None in the match arm anyway, so thought that separating the cases at the variant level might be clearer instead.

I like how it ended up; let me know what you think!
  • Loading branch information
matthiaskrgr committed Mar 27, 2023
2 parents 9c73bf9 + 4979860 commit 7f6b406
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 20 deletions.
12 changes: 6 additions & 6 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

PassMode::Cast(cast_ty, _) => {
let op = match self.locals[mir::RETURN_PLACE] {
LocalRef::Operand(Some(op)) => op,
LocalRef::Operand(None) => bug!("use of return before def"),
LocalRef::Operand(op) => op,
LocalRef::PendingOperand => bug!("use of return before def"),
LocalRef::Place(cg_place) => OperandRef {
val: Ref(cg_place.llval, None, cg_place.align),
layout: cg_place.layout,
Expand Down Expand Up @@ -1673,7 +1673,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
match self.locals[index] {
LocalRef::Place(dest) => dest,
LocalRef::UnsizedPlace(_) => bug!("return type must be sized"),
LocalRef::Operand(None) => {
LocalRef::PendingOperand => {
// Handle temporary places, specifically `Operand` ones, as
// they don't have `alloca`s.
return if fn_ret.is_indirect() {
Expand All @@ -1694,7 +1694,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
ReturnDest::DirectOperand(index)
};
}
LocalRef::Operand(Some(_)) => {
LocalRef::Operand(_) => {
bug!("place local already assigned to");
}
}
Expand Down Expand Up @@ -1737,7 +1737,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
IndirectOperand(tmp, index) => {
let op = bx.load_operand(tmp);
tmp.storage_dead(bx);
self.locals[index] = LocalRef::Operand(Some(op));
self.locals[index] = LocalRef::Operand(op);
self.debug_introduce_local(bx, index);
}
DirectOperand(index) => {
Expand All @@ -1752,7 +1752,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
} else {
OperandRef::from_immediate_or_packed_pair(bx, llval, ret_abi.layout)
};
self.locals[index] = LocalRef::Operand(Some(op));
self.locals[index] = LocalRef::Operand(op);
self.debug_introduce_local(bx, index);
}
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
LocalRef::Place(place) | LocalRef::UnsizedPlace(place) => {
bx.set_var_name(place.llval, name);
}
LocalRef::Operand(Some(operand)) => match operand.val {
LocalRef::Operand(operand) => match operand.val {
OperandValue::Ref(x, ..) | OperandValue::Immediate(x) => {
bx.set_var_name(x, name);
}
Expand All @@ -323,7 +323,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.set_var_name(b, &(name.clone() + ".1"));
}
},
LocalRef::Operand(None) => {}
LocalRef::PendingOperand => {}
}
}

Expand All @@ -332,9 +332,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

let base = match local_ref {
LocalRef::Operand(None) => return,
LocalRef::PendingOperand => return,

LocalRef::Operand(Some(operand)) => {
LocalRef::Operand(operand) => {
// Don't spill operands onto the stack in naked functions.
// See: https://github.com/rust-lang/rust/issues/42779
let attrs = bx.tcx().codegen_fn_attrs(self.instance.def_id());
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ enum LocalRef<'tcx, V> {
/// Every time it is initialized, we have to reallocate the place
/// and update the fat pointer. That's the reason why it is indirect.
UnsizedPlace(PlaceRef<'tcx, V>),
Operand(Option<OperandRef<'tcx, V>>),
/// The backend [`OperandValue`] has already been generated.
Operand(OperandRef<'tcx, V>),
/// Will be a `Self::Operand` once we get to its definition.
PendingOperand,
}

impl<'a, 'tcx, V: CodegenObject> LocalRef<'tcx, V> {
Expand All @@ -135,9 +138,9 @@ impl<'a, 'tcx, V: CodegenObject> LocalRef<'tcx, V> {
// Zero-size temporaries aren't always initialized, which
// doesn't matter because they don't contain data, but
// we need something in the operand.
LocalRef::Operand(Some(OperandRef::new_zst(bx, layout)))
LocalRef::Operand(OperandRef::new_zst(bx, layout))
} else {
LocalRef::Operand(None)
LocalRef::PendingOperand
}
}
}
Expand Down Expand Up @@ -337,7 +340,7 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// We don't have to cast or keep the argument in the alloca.
// FIXME(eddyb): We should figure out how to use llvm.dbg.value instead
// of putting everything in allocas just so we can use llvm.dbg.declare.
let local = |op| LocalRef::Operand(Some(op));
let local = |op| LocalRef::Operand(op);
match arg.mode {
PassMode::Ignore => {
return local(OperandRef::new_zst(bx, arg.layout));
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
debug!("maybe_codegen_consume_direct(place_ref={:?})", place_ref);

match self.locals[place_ref.local] {
LocalRef::Operand(Some(mut o)) => {
LocalRef::Operand(mut o) => {
// Moves out of scalar and scalar pair fields are trivial.
for elem in place_ref.projection.iter() {
match elem {
Expand All @@ -395,7 +395,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

Some(o)
}
LocalRef::Operand(None) => {
LocalRef::PendingOperand => {
bug!("use of {:?} before def", place_ref);
}
LocalRef::Place(..) | LocalRef::UnsizedPlace(..) => {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bug!("using operand local {:?} as place", place_ref);
}
}
LocalRef::PendingOperand => {
bug!("using still-pending operand local {:?} as place", place_ref);
}
};
for elem in place_ref.projection[base..].iter() {
cg_base = match *elem {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// ZST are passed as operands and require special handling
// because codegen_place() panics if Local is operand.
if let Some(index) = place.as_local() {
if let LocalRef::Operand(Some(op)) = self.locals[index] {
if let LocalRef::Operand(op) = self.locals[index] {
if let ty::Array(_, n) = op.layout.ty.kind() {
let n = n.eval_target_usize(bx.cx().tcx(), ty::ParamEnv::reveal_all());
return bx.cx().const_usize(n);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
LocalRef::UnsizedPlace(cg_indirect_dest) => {
self.codegen_rvalue_unsized(bx, cg_indirect_dest, rvalue)
}
LocalRef::Operand(None) => {
LocalRef::PendingOperand => {
let operand = self.codegen_rvalue_operand(bx, rvalue);
self.locals[index] = LocalRef::Operand(Some(operand));
self.locals[index] = LocalRef::Operand(operand);
self.debug_introduce_local(bx, index);
}
LocalRef::Operand(Some(op)) => {
LocalRef::Operand(op) => {
if !op.layout.is_zst() {
span_bug!(
statement.source_info.span,
Expand Down

0 comments on commit 7f6b406

Please sign in to comment.