Rollup merge of #109582 - scottmcm:local-ref-pending, r=oli-obk

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!
This commit is contained in:
Matthias Krüger 2023-03-27 15:32:42 +02:00 committed by GitHub
commit 7f6b406fa1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 26 additions and 20 deletions

View File

@ -397,8 +397,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
PassMode::Cast(cast_ty, _) => { PassMode::Cast(cast_ty, _) => {
let op = match self.locals[mir::RETURN_PLACE] { let op = match self.locals[mir::RETURN_PLACE] {
LocalRef::Operand(Some(op)) => op, LocalRef::Operand(op) => op,
LocalRef::Operand(None) => bug!("use of return before def"), LocalRef::PendingOperand => bug!("use of return before def"),
LocalRef::Place(cg_place) => OperandRef { LocalRef::Place(cg_place) => OperandRef {
val: Ref(cg_place.llval, None, cg_place.align), val: Ref(cg_place.llval, None, cg_place.align),
layout: cg_place.layout, layout: cg_place.layout,
@ -1673,7 +1673,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
match self.locals[index] { match self.locals[index] {
LocalRef::Place(dest) => dest, LocalRef::Place(dest) => dest,
LocalRef::UnsizedPlace(_) => bug!("return type must be sized"), LocalRef::UnsizedPlace(_) => bug!("return type must be sized"),
LocalRef::Operand(None) => { LocalRef::PendingOperand => {
// Handle temporary places, specifically `Operand` ones, as // Handle temporary places, specifically `Operand` ones, as
// they don't have `alloca`s. // they don't have `alloca`s.
return if fn_ret.is_indirect() { return if fn_ret.is_indirect() {
@ -1694,7 +1694,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
ReturnDest::DirectOperand(index) ReturnDest::DirectOperand(index)
}; };
} }
LocalRef::Operand(Some(_)) => { LocalRef::Operand(_) => {
bug!("place local already assigned to"); bug!("place local already assigned to");
} }
} }
@ -1737,7 +1737,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
IndirectOperand(tmp, index) => { IndirectOperand(tmp, index) => {
let op = bx.load_operand(tmp); let op = bx.load_operand(tmp);
tmp.storage_dead(bx); tmp.storage_dead(bx);
self.locals[index] = LocalRef::Operand(Some(op)); self.locals[index] = LocalRef::Operand(op);
self.debug_introduce_local(bx, index); self.debug_introduce_local(bx, index);
} }
DirectOperand(index) => { DirectOperand(index) => {
@ -1752,7 +1752,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
} else { } else {
OperandRef::from_immediate_or_packed_pair(bx, llval, ret_abi.layout) 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); self.debug_introduce_local(bx, index);
} }
} }

View File

@ -312,7 +312,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
LocalRef::Place(place) | LocalRef::UnsizedPlace(place) => { LocalRef::Place(place) | LocalRef::UnsizedPlace(place) => {
bx.set_var_name(place.llval, name); 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) => { OperandValue::Ref(x, ..) | OperandValue::Immediate(x) => {
bx.set_var_name(x, name); bx.set_var_name(x, name);
} }
@ -323,7 +323,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.set_var_name(b, &(name.clone() + ".1")); bx.set_var_name(b, &(name.clone() + ".1"));
} }
}, },
LocalRef::Operand(None) => {} LocalRef::PendingOperand => {}
} }
} }
@ -332,9 +332,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
} }
let base = match local_ref { 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. // Don't spill operands onto the stack in naked functions.
// See: https://github.com/rust-lang/rust/issues/42779 // See: https://github.com/rust-lang/rust/issues/42779
let attrs = bx.tcx().codegen_fn_attrs(self.instance.def_id()); let attrs = bx.tcx().codegen_fn_attrs(self.instance.def_id());

View File

@ -123,7 +123,10 @@ enum LocalRef<'tcx, V> {
/// Every time it is initialized, we have to reallocate the place /// Every time it is initialized, we have to reallocate the place
/// and update the fat pointer. That's the reason why it is indirect. /// and update the fat pointer. That's the reason why it is indirect.
UnsizedPlace(PlaceRef<'tcx, V>), 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> { impl<'a, 'tcx, V: CodegenObject> LocalRef<'tcx, V> {
@ -135,9 +138,9 @@ impl<'a, 'tcx, V: CodegenObject> LocalRef<'tcx, V> {
// Zero-size temporaries aren't always initialized, which // Zero-size temporaries aren't always initialized, which
// doesn't matter because they don't contain data, but // doesn't matter because they don't contain data, but
// we need something in the operand. // we need something in the operand.
LocalRef::Operand(Some(OperandRef::new_zst(bx, layout))) LocalRef::Operand(OperandRef::new_zst(bx, layout))
} else { } else {
LocalRef::Operand(None) LocalRef::PendingOperand
} }
} }
} }
@ -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. // 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 // 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. // 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 { match arg.mode {
PassMode::Ignore => { PassMode::Ignore => {
return local(OperandRef::new_zst(bx, arg.layout)); return local(OperandRef::new_zst(bx, arg.layout));

View File

@ -370,7 +370,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
debug!("maybe_codegen_consume_direct(place_ref={:?})", place_ref); debug!("maybe_codegen_consume_direct(place_ref={:?})", place_ref);
match self.locals[place_ref.local] { 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. // Moves out of scalar and scalar pair fields are trivial.
for elem in place_ref.projection.iter() { for elem in place_ref.projection.iter() {
match elem { match elem {
@ -395,7 +395,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Some(o) Some(o)
} }
LocalRef::Operand(None) => { LocalRef::PendingOperand => {
bug!("use of {:?} before def", place_ref); bug!("use of {:?} before def", place_ref);
} }
LocalRef::Place(..) | LocalRef::UnsizedPlace(..) => { LocalRef::Place(..) | LocalRef::UnsizedPlace(..) => {

View File

@ -558,6 +558,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bug!("using operand local {:?} as place", place_ref); 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() { for elem in place_ref.projection[base..].iter() {
cg_base = match *elem { cg_base = match *elem {

View File

@ -545,7 +545,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// ZST are passed as operands and require special handling // ZST are passed as operands and require special handling
// because codegen_place() panics if Local is operand. // because codegen_place() panics if Local is operand.
if let Some(index) = place.as_local() { 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() { if let ty::Array(_, n) = op.layout.ty.kind() {
let n = n.eval_target_usize(bx.cx().tcx(), ty::ParamEnv::reveal_all()); let n = n.eval_target_usize(bx.cx().tcx(), ty::ParamEnv::reveal_all());
return bx.cx().const_usize(n); return bx.cx().const_usize(n);

View File

@ -18,12 +18,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
LocalRef::UnsizedPlace(cg_indirect_dest) => { LocalRef::UnsizedPlace(cg_indirect_dest) => {
self.codegen_rvalue_unsized(bx, cg_indirect_dest, rvalue) self.codegen_rvalue_unsized(bx, cg_indirect_dest, rvalue)
} }
LocalRef::Operand(None) => { LocalRef::PendingOperand => {
let operand = self.codegen_rvalue_operand(bx, rvalue); 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); self.debug_introduce_local(bx, index);
} }
LocalRef::Operand(Some(op)) => { LocalRef::Operand(op) => {
if !op.layout.is_zst() { if !op.layout.is_zst() {
span_bug!( span_bug!(
statement.source_info.span, statement.source_info.span,