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

Refactor call terminator to always include destination place #96098

Merged
merged 1 commit into from
May 24, 2022
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
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