Refactor call terminator to always hold a destination place

Author: JakobDegen

src/abi/mod.rs

@@ -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() {
@@ -333,7 +334,8 @@ pub(crate) fn codegen_terminator_call<'tcx>(
                 &fx.tcx.symbol_name(instance).name,
                 substs,
                 args,
-                destination,
+                ret_place,
+                target,
             );
             return;
         }
@@ -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;
@@ -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));
         }
     }
@@ -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()
@@ -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 {

src/abi/returning.rs

@@ -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")
         }
@@ -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);
             }

src/base.rs

@@ -393,6 +393,7 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) {
                 func,
                 args,
                 destination,
+                target,
                 fn_span,
                 cleanup: _,
                 from_hir_call: _,
@@ -404,6 +405,7 @@ fn codegen_fn_content(fx: &mut FunctionCx<'_, '_, '_>) {
                         func,
                         args,
                         *destination,
+                        *target,
                     )
                 });
             }

src/constant.rs

@@ -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;
                     }

src/intrinsics/llvm.rs

@@ -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,
         _ => {
@@ -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, &[]);
 }

src/intrinsics/mod.rs

@@ -217,35 +217,42 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
     fx: &mut FunctionCx<'_, '_, 'tcx>,
     instance: Instance<'tcx>,
     args: &[mir::Operand<'tcx>],
-    destination: Option<(CPlace<'tcx>, BasicBlock)>,
+    destination: CPlace<'tcx>,
+    target: Option<BasicBlock>,
     source_info: mir::SourceInfo,
 ) {
     let intrinsic = fx.tcx.item_name(instance.def_id());
     let substs = instance.substs;
 
-    let ret = match destination {
-        Some((place, _)) => place,
-        None => {
-            // Insert non returning intrinsics here
-            match intrinsic {
-                sym::abort => {
-                    fx.bcx.ins().trap(TrapCode::User(0));
-                }
-                sym::transmute => {
-                    crate::base::codegen_panic(fx, "Transmuting to uninhabited type.", source_info);
-                }
-                _ => unimplemented!("unsupported instrinsic {}", intrinsic),
+    let target = if let Some(target) = target {
+        target
+    } else {
+        // Insert non returning intrinsics here
+        match intrinsic {
+            sym::abort => {
+                fx.bcx.ins().trap(TrapCode::User(0));
             }
-            return;
+            sym::transmute => {
+                crate::base::codegen_panic(fx, "Transmuting to uninhabited type.", source_info);
+            }
+            _ => unimplemented!("unsupported instrinsic {}", intrinsic),
         }
+        return;
     };
 
     if intrinsic.as_str().starts_with("simd_") {
-        self::simd::codegen_simd_intrinsic_call(fx, intrinsic, substs, args, ret, source_info.span);
-        let ret_block = fx.get_block(destination.expect("SIMD intrinsics don't diverge").1);
+        self::simd::codegen_simd_intrinsic_call(
+            fx,
+            intrinsic,
+            substs,
+            args,
+            destination,
+            source_info.span,
+        );
+        let ret_block = fx.get_block(target);
         fx.bcx.ins().jump(ret_block, &[]);
-    } else if codegen_float_intrinsic_call(fx, intrinsic, args, ret) {
-        let ret_block = fx.get_block(destination.expect("Float intrinsics don't diverge").1);
+    } else if codegen_float_intrinsic_call(fx, intrinsic, args, destination) {
+        let ret_block = fx.get_block(target);
         fx.bcx.ins().jump(ret_block, &[]);
     } else {
         codegen_regular_intrinsic_call(
@@ -254,9 +261,9 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
             intrinsic,
             substs,
             args,
-            ret,
-            source_info,
             destination,
+            Some(target),
+            source_info,
         );
     }
 }
@@ -339,8 +346,8 @@ fn codegen_regular_intrinsic_call<'tcx>(
     substs: SubstsRef<'tcx>,
     args: &[mir::Operand<'tcx>],
     ret: CPlace<'tcx>,
+    destination: Option<BasicBlock>,
     source_info: mir::SourceInfo,
-    destination: Option<(CPlace<'tcx>, BasicBlock)>,
 ) {
     let usize_layout = fx.layout_of(fx.tcx.types.usize);
 
@@ -761,7 +768,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
                     if fx.tcx.is_compiler_builtins(LOCAL_CRATE) {
                         // special case for compiler-builtins to avoid having to patch it
                         crate::trap::trap_unimplemented(fx, "128bit atomics not yet supported");
-                        let ret_block = fx.get_block(destination.unwrap().1);
+                        let ret_block = fx.get_block(destination.unwrap());
                         fx.bcx.ins().jump(ret_block, &[]);
                         return;
                     } else {
@@ -789,7 +796,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
                     if fx.tcx.is_compiler_builtins(LOCAL_CRATE) {
                         // special case for compiler-builtins to avoid having to patch it
                         crate::trap::trap_unimplemented(fx, "128bit atomics not yet supported");
-                        let ret_block = fx.get_block(destination.unwrap().1);
+                        let ret_block = fx.get_block(destination.unwrap());
                         fx.bcx.ins().jump(ret_block, &[]);
                         return;
                     } else {
@@ -1130,6 +1137,6 @@ fn codegen_regular_intrinsic_call<'tcx>(
         };
     }
 
-    let ret_block = fx.get_block(destination.unwrap().1);
+    let ret_block = fx.get_block(destination.unwrap());
     fx.bcx.ins().jump(ret_block, &[]);
 }