From 6bfecd41cc29a5faec964f2dc252443d5fda4293 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= <bsteinbr@gmail.com>
Date: Thu, 14 Sep 2017 22:09:09 +0200
Subject: [PATCH] Avoid unnecessary allocas for indirect function arguments

The extra alloca was only necessary because it made LLVM implicitly
handle the necessary deref to get to the actual value. The same happens
for indirect arguments that have the byval attribute. But the Rust ABI
does not use the byval attribute and so we need to manually add the
deref operation to the debuginfo.
---
 src/librustc_trans/abi.rs     |  4 ++++
 src/librustc_trans/mir/mod.rs | 29 ++++++++++++++++++++++-------
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs
index c3b6ede24b0..6df40c34ec5 100644
--- a/src/librustc_trans/abi.rs
+++ b/src/librustc_trans/abi.rs
@@ -113,6 +113,10 @@ impl ArgAttributes {
         self
     }
 
+    pub fn contains(&self, attr: ArgAttribute) -> bool {
+        self.regular.contains(attr)
+    }
+
     pub fn apply_llfn(&self, idx: AttributePlace, llfn: ValueRef) {
         unsafe {
             self.regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn));
diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs
index 5206ad74e20..d5d44bfa7ba 100644
--- a/src/librustc_trans/mir/mod.rs
+++ b/src/librustc_trans/mir/mod.rs
@@ -23,7 +23,7 @@ use builder::Builder;
 use common::{self, CrateContext, Funclet};
 use debuginfo::{self, declare_local, VariableAccess, VariableKind, FunctionDebugContext};
 use monomorphize::Instance;
-use abi::FnType;
+use abi::{ArgAttribute, FnType};
 use type_of;
 
 use syntax_pos::{DUMMY_SP, NO_EXPANSION, BytePos, Span};
@@ -378,6 +378,10 @@ fn arg_local_refs<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
         None
     };
 
+    let deref_op = unsafe {
+        [llvm::LLVMRustDIBuilderCreateOpDeref()]
+    };
+
     mir.args_iter().enumerate().map(|(arg_index, local)| {
         let arg_decl = &mir.local_decls[local];
         let arg_ty = mircx.monomorphize(&arg_decl.ty);
@@ -432,10 +436,9 @@ fn arg_local_refs<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
 
         let arg = &mircx.fn_ty.args[idx];
         idx += 1;
-        let llval = if arg.is_indirect() && bcx.sess().opts.debuginfo != FullDebugInfo {
+        let llval = if arg.is_indirect() {
             // Don't copy an indirect argument to an alloca, the caller
-            // already put it in a temporary alloca and gave it up, unless
-            // we emit extra-debug-info, which requires local allocas :(.
+            // already put it in a temporary alloca and gave it up
             // FIXME: lifetimes
             if arg.pad.is_some() {
                 llarg_idx += 1;
@@ -444,8 +447,7 @@ fn arg_local_refs<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
             llarg_idx += 1;
             llarg
         } else if !lvalue_locals.contains(local.index()) &&
-                  !arg.is_indirect() && arg.cast.is_none() &&
-                  arg_scope.is_none() {
+                  arg.cast.is_none() && arg_scope.is_none() {
             if arg.is_ignore() {
                 return LocalRef::new_operand(bcx.ccx, arg_ty);
             }
@@ -510,13 +512,26 @@ fn arg_local_refs<'a, 'tcx>(bcx: &Builder<'a, 'tcx>,
         arg_scope.map(|scope| {
             // Is this a regular argument?
             if arg_index > 0 || mir.upvar_decls.is_empty() {
+                // The Rust ABI passes indirect variables using a pointer and a manual copy, so we
+                // need to insert a deref here, but the C ABI uses a pointer and a copy using the
+                // byval attribute, for which LLVM does the deref itself, so we must not add it.
+                let variable_access = if arg.is_indirect() &&
+                    !arg.attrs.contains(ArgAttribute::ByVal) {
+                    VariableAccess::IndirectVariable {
+                        alloca: llval,
+                        address_operations: &deref_op,
+                    }
+                } else {
+                    VariableAccess::DirectVariable { alloca: llval }
+                };
+
                 declare_local(
                     bcx,
                     &mircx.debug_context,
                     arg_decl.name.unwrap_or(keywords::Invalid.name()),
                     arg_ty,
                     scope,
-                    VariableAccess::DirectVariable { alloca: llval },
+                    variable_access,
                     VariableKind::ArgumentVariable(arg_index + 1),
                     DUMMY_SP
                 );