Skip to content

Commit

Permalink
Refactor call terminator to always hold a destination place
Browse files Browse the repository at this point in the history
  • Loading branch information
JakobDegen committed May 23, 2022
1 parent 222c572 commit 09b0936
Show file tree
Hide file tree
Showing 67 changed files with 422 additions and 412 deletions.
4 changes: 1 addition & 3 deletions compiler/rustc_borrowck/src/constraint_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ impl<'cg, 'cx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'cx, 'tcx> {
// A `Call` terminator's return value can be a local which has borrows,
// so we need to record those as `killed` as well.
if let TerminatorKind::Call { destination, .. } = terminator.kind {
if let Some((place, _)) = destination {
self.record_killed_borrows_for_place(place, location);
}
self.record_killed_borrows_for_place(destination, location);
}

self.super_terminator(terminator, location);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2198,10 +2198,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
"annotate_argument_and_return_for_borrow: target={:?} terminator={:?}",
target, terminator
);
if let TerminatorKind::Call { destination: Some((place, _)), args, .. } =
if let TerminatorKind::Call { destination, target: Some(_), args, .. } =
&terminator.kind
{
if let Some(assigned_to) = place.as_local() {
if let Some(assigned_to) = destination.as_local() {
debug!(
"annotate_argument_and_return_for_borrow: assigned_to={:?} args={:?}",
assigned_to, args
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,10 +705,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let terminator = block.terminator();
debug!("was_captured_by_trait_object: terminator={:?}", terminator);

if let TerminatorKind::Call { destination: Some((place, block)), args, .. } =
if let TerminatorKind::Call { destination, target: Some(block), args, .. } =
&terminator.kind
{
if let Some(dest) = place.as_local() {
if let Some(dest) = destination.as_local() {
debug!(
"was_captured_by_trait_object: target={:?} dest={:?} args={:?}",
target, dest, args
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_borrowck/src/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
ref func,
ref args,
destination,
target: _,
cleanup: _,
from_hir_call: _,
fn_span: _,
Expand All @@ -132,9 +133,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
for arg in args {
self.consume_operand(location, arg);
}
if let Some((dest, _ /*bb*/)) = destination {
self.mutate_place(location, *dest, Deep);
}
self.mutate_place(location, *destination, Deep);
}
TerminatorKind::Assert { ref cond, expected: _, ref msg, target: _, cleanup: _ } => {
self.consume_operand(location, cond);
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,8 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
TerminatorKind::Call {
ref func,
ref args,
ref destination,
destination,
target: _,
cleanup: _,
from_hir_call: _,
fn_span: _,
Expand All @@ -670,9 +671,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
for arg in args {
self.consume_operand(loc, (arg, span), flow_state);
}
if let Some((dest, _ /*bb*/)) = *destination {
self.mutate_place(loc, (dest, span), Deep, flow_state);
}
self.mutate_place(loc, (destination, span), Deep, flow_state);
}
TerminatorKind::Assert { ref cond, expected: _, ref msg, target: _, cleanup: _ } => {
self.consume_operand(loc, (cond, span), flow_state);
Expand Down
21 changes: 12 additions & 9 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}
// FIXME: check the values
}
TerminatorKind::Call { ref func, ref args, ref destination, from_hir_call, .. } => {
TerminatorKind::Call {
ref func, ref args, destination, target, from_hir_call, ..
} => {
self.check_operand(func, term_location);
for arg in args {
self.check_operand(arg, term_location);
Expand All @@ -1424,7 +1426,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
sig,
);
let sig = self.normalize(sig, term_location);
self.check_call_dest(body, term, &sig, destination, term_location);
self.check_call_dest(body, term, &sig, destination, target, term_location);

self.prove_predicates(
sig.inputs_and_output
Expand Down Expand Up @@ -1502,15 +1504,16 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
body: &Body<'tcx>,
term: &Terminator<'tcx>,
sig: &ty::FnSig<'tcx>,
destination: &Option<(Place<'tcx>, BasicBlock)>,
destination: Place<'tcx>,
target: Option<BasicBlock>,
term_location: Location,
) {
let tcx = self.tcx();
match *destination {
Some((ref dest, _target_block)) => {
let dest_ty = dest.ty(body, tcx).ty;
match target {
Some(_) => {
let dest_ty = destination.ty(body, tcx).ty;
let dest_ty = self.normalize(dest_ty, term_location);
let category = match dest.as_local() {
let category = match destination.as_local() {
Some(RETURN_PLACE) => {
if let BorrowCheckContext {
universal_regions:
Expand Down Expand Up @@ -1659,8 +1662,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.assert_iscleanup(body, block_data, unwind, true);
}
}
TerminatorKind::Call { ref destination, cleanup, .. } => {
if let &Some((_, target)) = destination {
TerminatorKind::Call { ref target, cleanup, .. } => {
if let &Some(target) = target {
self.assert_iscleanup(body, block_data, target, is_cleanup);
}
if let Some(cleanup) = cleanup {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/used_muts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ impl<'visit, 'cx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'tc
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
debug!("visit_terminator: terminator={:?}", terminator);
match &terminator.kind {
TerminatorKind::Call { destination: Some((into, _)), .. } => {
self.remove_never_initialized_mut_locals(*into);
TerminatorKind::Call { destination, .. } => {
self.remove_never_initialized_mut_locals(*destination);
}
TerminatorKind::DropAndReplace { place, .. } => {
self.remove_never_initialized_mut_locals(*place);
Expand Down
18 changes: 10 additions & 8 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,14 @@ pub(crate) fn codegen_terminator_call<'tcx>(
source_info: mir::SourceInfo,
func: &Operand<'tcx>,
args: &[Operand<'tcx>],
mir_dest: Option<(Place<'tcx>, BasicBlock)>,
destination: Place<'tcx>,
target: Option<BasicBlock>,
) {
let fn_ty = fx.monomorphize(func.ty(fx.mir, fx.tcx));
let fn_sig =
fx.tcx.normalize_erasing_late_bound_regions(ParamEnv::reveal_all(), fn_ty.fn_sig(fx.tcx));

let destination = mir_dest.map(|(place, bb)| (codegen_place(fx, place), bb));
let ret_place = codegen_place(fx, destination);

// Handle special calls like instrinsics and empty drop glue.
let instance = if let ty::FnDef(def_id, substs) = *fn_ty.kind() {
Expand All @@ -333,7 +334,8 @@ pub(crate) fn codegen_terminator_call<'tcx>(
&fx.tcx.symbol_name(instance).name,
substs,
args,
destination,
ret_place,
target,
);
return;
}
Expand All @@ -344,14 +346,15 @@ pub(crate) fn codegen_terminator_call<'tcx>(
fx,
instance,
args,
destination,
ret_place,
target,
source_info,
);
return;
}
InstanceDef::DropGlue(_, None) => {
// empty drop glue - a nop.
let (_, dest) = destination.expect("Non terminating drop_in_place_real???");
let dest = target.expect("Non terminating drop_in_place_real???");
let ret_block = fx.get_block(dest);
fx.bcx.ins().jump(ret_block, &[]);
return;
Expand All @@ -377,7 +380,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
.unwrap_or(false);
if is_cold {
fx.bcx.set_cold_block(fx.bcx.current_block().unwrap());
if let Some((_place, destination_block)) = destination {
if let Some(destination_block) = target {
fx.bcx.set_cold_block(fx.get_block(destination_block));
}
}
Expand Down Expand Up @@ -459,7 +462,6 @@ pub(crate) fn codegen_terminator_call<'tcx>(
}
};

let ret_place = destination.map(|(place, _)| place);
self::returning::codegen_with_call_return_arg(fx, &fn_abi.ret, ret_place, |fx, return_ptr| {
let call_args = return_ptr
.into_iter()
Expand Down Expand Up @@ -511,7 +513,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
call_inst
});

if let Some((_, dest)) = destination {
if let Some(dest) = target {
let ret_block = fx.get_block(dest);
fx.bcx.ins().jump(ret_block, &[]);
} else {
Expand Down
51 changes: 18 additions & 33 deletions compiler/rustc_codegen_cranelift/src/abi/returning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,22 @@ pub(super) fn codegen_return_param<'tcx>(
pub(super) fn codegen_with_call_return_arg<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
ret_arg_abi: &ArgAbi<'tcx, Ty<'tcx>>,
ret_place: Option<CPlace<'tcx>>,
ret_place: CPlace<'tcx>,
f: impl FnOnce(&mut FunctionCx<'_, '_, 'tcx>, Option<Value>) -> Inst,
) {
let (ret_temp_place, return_ptr) = match ret_arg_abi.mode {
PassMode::Ignore => (None, None),
PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => match ret_place {
Some(ret_place) if matches!(ret_place.inner(), CPlaceInner::Addr(_, None)) => {
PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => {
if matches!(ret_place.inner(), CPlaceInner::Addr(_, None)) {
// This is an optimization to prevent unnecessary copies of the return value when
// the return place is already a memory place as opposed to a register.
// This match arm can be safely removed.
(None, Some(ret_place.to_ptr().get_addr(fx)))
}
_ => {
} else {
let place = CPlace::new_stack_slot(fx, ret_arg_abi.layout);
(Some(place), Some(place.to_ptr().get_addr(fx)))
}
},
}
PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ } => {
unreachable!("unsized return value")
}
Expand All @@ -84,39 +83,25 @@ pub(super) fn codegen_with_call_return_arg<'tcx>(
match ret_arg_abi.mode {
PassMode::Ignore => {}
PassMode::Direct(_) => {
if let Some(ret_place) = ret_place {
let ret_val = fx.bcx.inst_results(call_inst)[0];
ret_place.write_cvalue(fx, CValue::by_val(ret_val, ret_arg_abi.layout));
}
let ret_val = fx.bcx.inst_results(call_inst)[0];
ret_place.write_cvalue(fx, CValue::by_val(ret_val, ret_arg_abi.layout));
}
PassMode::Pair(_, _) => {
if let Some(ret_place) = ret_place {
let ret_val_a = fx.bcx.inst_results(call_inst)[0];
let ret_val_b = fx.bcx.inst_results(call_inst)[1];
ret_place.write_cvalue(
fx,
CValue::by_val_pair(ret_val_a, ret_val_b, ret_arg_abi.layout),
);
}
let ret_val_a = fx.bcx.inst_results(call_inst)[0];
let ret_val_b = fx.bcx.inst_results(call_inst)[1];
ret_place
.write_cvalue(fx, CValue::by_val_pair(ret_val_a, ret_val_b, ret_arg_abi.layout));
}
PassMode::Cast(cast) => {
if let Some(ret_place) = ret_place {
let results = fx
.bcx
.inst_results(call_inst)
.iter()
.copied()
.collect::<SmallVec<[Value; 2]>>();
let result =
super::pass_mode::from_casted_value(fx, &results, ret_place.layout(), cast);
ret_place.write_cvalue(fx, result);
}
let results =
fx.bcx.inst_results(call_inst).iter().copied().collect::<SmallVec<[Value; 2]>>();
let result =
super::pass_mode::from_casted_value(fx, &results, ret_place.layout(), cast);
ret_place.write_cvalue(fx, result);
}
PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => {
if let (Some(ret_place), Some(ret_temp_place)) = (ret_place, ret_temp_place) {
// Both ret_place and ret_temp_place must be Some. If ret_place is None, this is
// a non-returning call. If ret_temp_place is None, it is not necessary to copy the
// return value.
if let Some(ret_temp_place) = ret_temp_place {
// If ret_temp_place is None, it is not necessary to copy the return value.
let ret_temp_value = ret_temp_place.to_cvalue(fx);
ret_place.write_cvalue(fx, ret_temp_value);
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) {
func,
args,
destination,
target,
fn_span,
cleanup: _,
from_hir_call: _,
Expand All @@ -404,6 +405,7 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) {
func,
args,
*destination,
*target,
)
});
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,8 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. } => unreachable!(),
TerminatorKind::InlineAsm { .. } => return None,
TerminatorKind::Call { destination: Some((call_place, _)), .. }
if call_place == place =>
TerminatorKind::Call { destination, target: Some(_), .. }
if destination == place =>
{
return None;
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_codegen_cranelift/src/intrinsics/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ pub(crate) fn codegen_llvm_intrinsic_call<'tcx>(
intrinsic: &str,
_substs: SubstsRef<'tcx>,
args: &[mir::Operand<'tcx>],
destination: Option<(CPlace<'tcx>, BasicBlock)>,
ret: CPlace<'tcx>,
target: Option<BasicBlock>,
) {
let ret = destination.unwrap().0;

intrinsic_match! {
fx, intrinsic, args,
_ => {
Expand Down Expand Up @@ -126,7 +125,7 @@ pub(crate) fn codegen_llvm_intrinsic_call<'tcx>(
};
}

let dest = destination.expect("all llvm intrinsics used by stdlib should return").1;
let dest = target.expect("all llvm intrinsics used by stdlib should return");
let ret_block = fx.get_block(dest);
fx.bcx.ins().jump(ret_block, &[]);
}
Expand Down
Loading

0 comments on commit 09b0936

Please sign in to comment.