Fix sret stack usage

Cargo.lock

@@ -56,18 +56,18 @@ dependencies = [
 
 [[package]]
 name = "gccjit"
-version = "7.1.0"
+version = "7.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "21312c2cc52f15e394551018c6ee878b84b039f47ad00ab027cdd13fe8b5fb08"
+checksum = "27cd340f3c265988de3379689cbbd103c25b1f8a3d0cc73f91a49083e899d3e1"
 dependencies = [
  "gccjit_sys",
 ]
 
 [[package]]
 name = "gccjit_sys"
-version = "4.1.0"
+version = "4.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3b85bc720f5c4194dcea10f95d8eb82b30b74d63255ee01285b990424f5a0a46"
+checksum = "da2d9d74572f004908f17ec1bd0af071e5db63a12c3335c9651a6125593951c6"
 dependencies = [
  "libc",
 ]

Cargo.toml

@@ -20,7 +20,7 @@ default = ["master"]
 [dependencies]
 object = { version = "0.39.0", default-features = false, features = ["std", "read"] }
 tempfile = "3.20"
-gccjit = { version = "7.1.0", features = ["dlopen"] }
+gccjit = { version = "7.2.0", features = ["dlopen"] }
 #gccjit = { git = "https://github.com/rust-lang/gccjit.rs", branch = "error-dlopen", features = ["dlopen"] }
 
 # Local copy.

libgccjit.version

@@ -1 +1 @@
-c86ad8032df732309fe949ea68da1874fbb76f5c
+37ea7cd851f04ee6a313c5067c9fde9328f876aa

src/builder.rs

@@ -202,46 +202,6 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
         self.llbb().add_assignment(self.location, lvalue, value);
     }
 
-    fn check_call<'b>(
-        &mut self,
-        _typ: &str,
-        func: Function<'gcc>,
-        args: &'b [RValue<'gcc>],
-    ) -> Cow<'b, [RValue<'gcc>]> {
-        let mut all_args_match = true;
-        let mut param_types = vec![];
-        let param_count = func.get_param_count();
-        for (index, arg) in args.iter().enumerate().take(param_count) {
-            let param = func.get_param(index as i32);
-            let param = param.to_rvalue().get_type();
-            if param != arg.get_type() {
-                all_args_match = false;
-            }
-            param_types.push(param);
-        }
-
-        if all_args_match {
-            return Cow::Borrowed(args);
-        }
-
-        let casted_args: Vec<_> = param_types
-            .into_iter()
-            .zip(args.iter())
-            .map(|(expected_ty, &actual_val)| {
-                let actual_ty = actual_val.get_type();
-                if expected_ty != actual_ty {
-                    self.bitcast(actual_val, expected_ty)
-                } else {
-                    actual_val
-                }
-            })
-            .collect();
-
-        debug_assert_eq!(casted_args.len(), args.len());
-
-        Cow::Owned(casted_args)
-    }
-
     fn check_ptr_call<'b>(
         &mut self,
         _typ: &str,
@@ -371,51 +331,13 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
         };
         #[cfg(not(feature = "master"))]
         let args: &[RValue<'gcc>] = &args;
-        // FIXME(antoyo): remove when having a proper API.
-        let gcc_func = unsafe { std::mem::transmute::<RValue<'gcc>, Function<'gcc>>(func) };
-        let call = if self.functions.borrow().values().any(|value| *value == gcc_func) {
-            // FIXME(antoyo): remove when the API supports a different type for functions.
-            let func: Function<'gcc> = self.cx.rvalue_as_function(func);
-            self.function_call(func, return_slot, args, funclet, must_tail)
-        } else {
-            // If it's a not function that was defined, it's a function pointer.
-            self.function_ptr_call(typ, fn_abi, func, return_slot, args, funclet, must_tail)
-        };
+        let call = self.function_ptr_call(typ, fn_abi, func, return_slot, args, funclet, must_tail);
         if let Some(_fn_abi) = fn_abi {
             // FIXME(bjorn3): Apply function attributes
         }
         call
     }
 
-    pub fn function_call(
-        &mut self,
-        func: Function<'gcc>,
-        return_slot: ReturnSlot<RValue<'gcc>>,
-        args: &[RValue<'gcc>],
-        _funclet: Option<&Funclet>,
-        must_tail: bool,
-    ) -> RValue<'gcc> {
-        let args = self.check_call("call", func, args);
-
-        let call = self.cx.context.new_call(self.location, func, &args);
-        if must_tail {
-            // Return the bare tail call, don't assign or `add_eval` it yet.
-            return call;
-        }
-
-        // gccjit requires to use the result of functions, even when it's not used.
-        // That's why we assign the result to a local or call add_eval().
-        let return_type = func.get_return_type();
-        let void_type = self.context.new_type::<()>();
-        if return_type != void_type {
-            self.store_call_result(return_slot, call)
-        } else {
-            self.block.add_eval(self.location, call);
-            // Return dummy value when not having return value.
-            self.context.new_rvalue_zero(self.isize_type)
-        }
-    }
-
     #[allow(clippy::too_many_arguments)]
     fn function_ptr_call(
         &mut self,
@@ -473,6 +395,7 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
 
         if return_type != void_type {
             let return_value = self.cx.context.new_call_through_ptr(self.location, func_ptr, &args);
+            set_return_slot_optimization(return_value, &return_slot);
             let return_value = llvm::adjust_intrinsic_return_value(
                 self,
                 return_value,
@@ -2583,6 +2506,17 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> {
     }
 }
 
+/// Let a call returning in memory build its result directly in cg_ssa's return slot: the return
+/// pointer is `noalias` in the Rust ABI, so the callee cannot observe the slot through another path.
+fn set_return_slot_optimization<'gcc>(call: RValue<'gcc>, return_slot: &ReturnSlot<RValue<'gcc>>) {
+    #[cfg(feature = "master")]
+    if let ReturnSlot::Indirect(_) = *return_slot {
+        call.set_return_slot_optimization(true);
+    }
+    #[cfg(not(feature = "master"))]
+    let _ = (call, return_slot);
+}
+
 fn difference_or_zero<'gcc>(
     loc: Option<Location<'gcc>>,
     a: RValue<'gcc>,

src/context.rs

@@ -141,7 +141,7 @@ pub struct CodegenCx<'gcc, 'tcx> {
 
     eh_personality: Cell<Option<Function<'gcc>>>,
     #[cfg(feature = "master")]
-    pub rust_try_fn: Cell<Option<(Type<'gcc>, Function<'gcc>)>>,
+    pub rust_try_fn: Cell<Option<Function<'gcc>>>,
 
     pub pointee_infos: RefCell<FxHashMap<(Ty<'tcx>, Size), Option<PointeeInfo>>>,
 
@@ -366,17 +366,6 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
         self.landing_pads.borrow_mut().clear();
     }
 
-    pub fn rvalue_as_function(&self, value: RValue<'gcc>) -> Function<'gcc> {
-        let function: Function<'gcc> = unsafe { std::mem::transmute(value) };
-        debug_assert!(
-            self.functions.borrow().values().any(|value| *value == function),
-            "{:?} ({:?}) is not a function",
-            value,
-            value.get_type()
-        );
-        function
-    }
-
     pub fn is_native_int_type(&self, typ: Type<'gcc>) -> bool {
         let types = [
             self.u8_type,

src/declare.rs

@@ -136,10 +136,14 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
             // Return in memory even where the target ABI would use registers, as cg_llvm does for
             // `PassMode::Indirect`.
             func.set_indirect_return();
-            // `get_param(0)` hands out this local's address and `ret_void` returns it.
+            // `get_param(0)` hands out this local's address and `ret_void` returns it. As the named
+            // return value, it lives in the caller's return slot instead of being copied there.
             if self.linkage.get() != FunctionType::Extern {
                 self.functions_with_indirect_return.borrow_mut().entry(func).or_insert_with(|| {
-                    func.new_local(None, fn_abi_gcc.return_type, "indirectReturn")
+                    let return_value =
+                        func.new_local(None, fn_abi_gcc.return_type, "indirectReturn");
+                    func.set_named_return_value(return_value);
+                    return_value
                 });
             }
         }

src/intrinsic/llvm.rs

@@ -1091,27 +1091,17 @@ pub fn intrinsic<'gcc, 'tcx>(name: &str, cx: &CodegenCx<'gcc, 'tcx>) -> Function
         "llvm.x86.xgetbv" => "__builtin_trap",
         _ => unimplemented!("unsupported LLVM intrinsic {}", name),
     };
-    let func = cx.context.get_builtin_function(gcc_name);
-    cx.functions.borrow_mut().insert(gcc_name.to_string(), func);
-    func
+    cx.context.get_builtin_function(gcc_name)
 }
 
 #[cfg(feature = "master")]
 pub fn intrinsic<'gcc, 'tcx>(name: &str, cx: &CodegenCx<'gcc, 'tcx>) -> Function<'gcc> {
     let gcc_name = match name {
-        "llvm.prefetch.p0" => {
-            let gcc_name = "__builtin_prefetch";
-            let func = cx.context.get_builtin_function(gcc_name);
-            cx.functions.borrow_mut().insert(gcc_name.to_string(), func);
-            return func;
-        }
+        "llvm.prefetch.p0" => return cx.context.get_builtin_function("__builtin_prefetch"),
 
         "llvm.aarch64.isb" => {
             // FIXME: GCC doesn't support __builtin_arm_isb yet, check if this builtin is OK.
-            let gcc_name = "__atomic_thread_fence";
-            let func = cx.context.get_builtin_function(gcc_name);
-            cx.functions.borrow_mut().insert(gcc_name.to_string(), func);
-            return func;
+            return cx.context.get_builtin_function("__atomic_thread_fence");
         }
 
         "llvm.x86.xgetbv" => "__builtin_ia32_xgetbv",
@@ -1122,12 +1112,7 @@ pub fn intrinsic<'gcc, 'tcx>(name: &str, cx: &CodegenCx<'gcc, 'tcx>) -> Function
         "llvm.sqrt.v2f64" => "__builtin_ia32_sqrtpd",
         // FIXME: Should handle other targets than `ia32`.
         "llvm.sqrt.v4f32" => "__builtin_ia32_sqrtps",
-        "llvm.sqrt.f32" => {
-            let gcc_name = "__builtin_sqrtf";
-            let func = cx.context.get_builtin_function(gcc_name);
-            cx.functions.borrow_mut().insert(gcc_name.to_string(), func);
-            return func;
-        }
+        "llvm.sqrt.f32" => return cx.context.get_builtin_function("__builtin_sqrtf"),
         // FIXME: Should handle other targets than `ia32`.
         "llvm.smax.v4i32" => "__builtin_ia32_pmaxsd128",
         "llvm.x86.avx512.pmul.dq.512" => "__builtin_ia32_pmuldq512_mask",
@@ -1741,9 +1726,7 @@ pub fn intrinsic<'gcc, 'tcx>(name: &str, cx: &CodegenCx<'gcc, 'tcx>) -> Function
         _ => map_arch_intrinsic(name),
     };
 
-    let func = cx.context.get_target_builtin_function(gcc_name);
-    cx.functions.borrow_mut().insert(gcc_name.to_string(), func);
-    func
+    cx.context.get_target_builtin_function(gcc_name)
 }
 
 #[cfg(feature = "master")]

src/intrinsic/mod.rs

@@ -28,8 +28,6 @@ use rustc_session::config::OptLevel;
 use rustc_span::{Span, Symbol, bug, span_bug, sym};
 use rustc_target::callconv::{ArgAbi, PassMode};
 
-#[cfg(feature = "master")]
-use crate::abi::FnAbiGccExt;
 use crate::abi::GccType;
 use crate::builder::Builder;
 use crate::common::{SignType, TypeReflection};
@@ -1403,7 +1401,7 @@ fn codegen_gnu_try<'gcc, 'tcx>(
     dest: PlaceRef<'tcx, RValue<'gcc>>,
 ) {
     let cx: &CodegenCx<'gcc, '_> = bx.cx;
-    let (llty, func) = get_rust_try_fn(cx, &mut |mut bx| {
+    let func = get_rust_try_fn(cx, &mut |mut bx| {
         // Codegens the shims described above:
         //
         //   bx:
@@ -1462,12 +1460,12 @@ fn codegen_gnu_try<'gcc, 'tcx>(
         );
     });
 
-    let func = unsafe { std::mem::transmute::<Function<'gcc>, RValue<'gcc>>(func) };
+    let func = func.get_address(None);
 
     // Note that no invoke is used here because by definition this function
     // can't panic (that's what it's catching).
     let ret = bx.call(
-        llty,
+        func.get_type(),
         None,
         None,
         func,
@@ -1487,7 +1485,7 @@ fn codegen_gnu_try<'gcc, 'tcx>(
 fn get_rust_try_fn<'a, 'gcc, 'tcx>(
     cx: &'a CodegenCx<'gcc, 'tcx>,
     codegen: &mut dyn FnMut(Builder<'a, 'gcc, 'tcx>),
-) -> (Type<'gcc>, Function<'gcc>) {
+) -> Function<'gcc> {
     if let Some(llfn) = cx.rust_try_fn.get() {
         return llfn;
     }
@@ -1532,9 +1530,8 @@ fn gen_fn<'a, 'gcc, 'tcx>(
     name: &str,
     rust_fn_sig: ty::PolyFnSig<'tcx>,
     codegen: &mut dyn FnMut(Builder<'a, 'gcc, 'tcx>),
-) -> (Type<'gcc>, Function<'gcc>) {
+) -> Function<'gcc> {
     let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty());
-    let return_type = fn_abi.gcc_type(cx).return_type;
     // FIXME(eddyb) find a nicer way to do this.
     cx.linkage.set(FunctionType::Internal);
     let func = cx.declare_fn(name, fn_abi);
@@ -1543,5 +1540,5 @@ fn gen_fn<'a, 'gcc, 'tcx>(
     let block = Builder::append_block(cx, func, "entry-block");
     let bx = Builder::build(cx, block);
     codegen(bx);
-    (return_type, func)
+    func
 }

tests/run/sret_frame_size.rs

@@ -0,0 +1,67 @@
+// Compiler:
+//
+// Run-time:
+//   status: 0
+
+// A call returning in memory must get its destination as the return slot. A backend that returns
+// into a temporary and then copies it to the destination needs two copies of the value in the
+// caller's frame, which fails these assertions, for direct calls and for calls through pointers.
+
+use std::hint::black_box;
+
+const LEN: usize = 8192;
+const BIG_SIZE: usize = LEN * size_of::<u64>();
+
+type Big = [u64; LEN];
+
+#[inline(never)]
+fn make(seed: u64) -> Big {
+    let mut array = [seed; LEN];
+    array[1] = seed + 1;
+    array
+}
+
+/// The size of this function's frame: the distance between `value` in two nested calls.
+#[inline(never)]
+fn direct_call_frame_size(depth: u32, outer_address: usize) -> usize {
+    let value = make(depth.into());
+    let address = black_box(&value) as *const Big as usize;
+    let size = if depth == 0 {
+        outer_address.abs_diff(address)
+    } else {
+        direct_call_frame_size(depth - 1, address)
+    };
+    // Keeps `value` alive across the recursive call.
+    black_box(&value);
+    size
+}
+
+/// The size of this function's frame: the distance between `value` in two nested calls.
+#[inline(never)]
+fn pointer_call_frame_size(depth: u32, outer_address: usize, make: fn(u64) -> Big) -> usize {
+    let value = make(depth.into());
+    let address = black_box(&value) as *const Big as usize;
+    let size = if depth == 0 {
+        outer_address.abs_diff(address)
+    } else {
+        pointer_call_frame_size(depth - 1, address, make)
+    };
+    // Keeps `value` alive across the recursive call.
+    black_box(&value);
+    size
+}
+
+fn main() {
+    let limit = BIG_SIZE * 3 / 2;
+
+    let size = direct_call_frame_size(black_box(1), 0);
+    assert!(size < limit, "direct call: {}-byte frame for one {}-byte value", size, BIG_SIZE);
+
+    let size = pointer_call_frame_size(black_box(1), 0, black_box(make));
+    assert!(
+        size < limit,
+        "call through a pointer: {}-byte frame for one {}-byte value",
+        size,
+        BIG_SIZE
+    );
+}