rustc_codegen_ssa: move all set_var_name calls to mir::debuginfo.

eddyb · eddyb · commit 5f4ee36e0330 · 2019-10-31T20:25:42.000+02:00
diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs
@@ -7,10 +7,12 @@ use rustc::ty::layout::HasTyCtxt;
 use rustc_target::abi::{Variants, VariantIdx};
 use crate::traits::*;
 
+use std::fmt;
 use syntax_pos::{DUMMY_SP, BytePos, Span};
 use syntax::symbol::kw;
 
 use super::{FunctionCx, LocalRef};
+use super::OperandValue;
 
 pub enum FunctionDebugContext<D> {
     RegularContext(FunctionDebugContextData<D>),
@@ -90,6 +92,29 @@ impl<D> DebugScope<D> {
     }
 }
 
+// HACK(eddyb) helpers for `set_var_name` calls, move elsewhere?
+enum Either<T, U> {
+    Left(T),
+    Right(U),
+}
+
+impl<T: fmt::Display, U: fmt::Display> fmt::Display for Either<T, U> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Either::Left(x) => x.fmt(f),
+            Either::Right(x) => x.fmt(f),
+        }
+    }
+}
+
+struct DisplayViaDebug<T>(T);
+
+impl<T: fmt::Debug> fmt::Display for DisplayViaDebug<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
 impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     pub fn set_debug_loc(
         &mut self,
@@ -149,54 +174,107 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         }
     }
 
-    pub fn debug_declare_locals(&self, bx: &mut Bx) {
-        let tcx = self.cx.tcx();
+    /// Apply debuginfo and/or name, after creating the `alloca` for a local,
+    /// or initializing the local with an operand (whichever applies).
+    // FIXME(eddyb) use `llvm.dbg.value` (which would work for operands),
+    // not just `llvm.dbg.declare` (which requires `alloca`).
+    pub fn debug_introduce_local(&self, bx: &mut Bx, local: mir::Local) {
         let upvar_debuginfo = &self.mir.__upvar_debuginfo_codegen_only_do_not_use;
 
-        if bx.sess().opts.debuginfo != DebugInfo::Full {
+        // FIXME(eddyb) maybe name the return place as `_0` or `return`?
+        if local == mir::RETURN_PLACE {
             return;
         }
 
-        for (local, local_ref) in self.locals.iter_enumerated() {
-            if local == mir::RETURN_PLACE {
-                continue;
+        let decl = &self.mir.local_decls[local];
+        let (name, kind) = if self.mir.local_kind(local) == mir::LocalKind::Arg {
+            let arg_index = local.index() - 1;
+
+            // Add debuginfo even to unnamed arguments.
+            // FIXME(eddyb) is this really needed?
+            let name = if arg_index == 0 && !upvar_debuginfo.is_empty() {
+                // Hide closure environments from debuginfo.
+                // FIXME(eddyb) shouldn't `ArgumentVariable` indices
+                // be offset to account for the hidden environment?
+                None
+            } else {
+                Some(decl.name.unwrap_or(kw::Invalid))
+            };
+            (name, VariableKind::ArgumentVariable(arg_index + 1))
+        } else {
+            (decl.name, VariableKind::LocalVariable)
+        };
+
+        let local_ref = &self.locals[local];
+
+        {
+            let name = match name {
+                Some(name) if name != kw::Invalid => Either::Left(name),
+                _ => Either::Right(DisplayViaDebug(local)),
+            };
+            match local_ref {
+                LocalRef::Place(place) |
+                LocalRef::UnsizedPlace(place) => {
+                    bx.set_var_name(place.llval, name);
+                }
+                LocalRef::Operand(Some(operand)) => match operand.val {
+                    OperandValue::Ref(x, ..) |
+                    OperandValue::Immediate(x) => {
+                        bx.set_var_name(x, name);
+                    }
+                    OperandValue::Pair(a, b) => {
+                        // FIXME(eddyb) these are scalar components,
+                        // maybe extract the high-level fields?
+                        bx.set_var_name(a, format_args!("{}.0", name));
+                        bx.set_var_name(b, format_args!("{}.1", name));
+                    }
+                }
+                LocalRef::Operand(None) => {}
+            }
+        }
+
+        if let Some(name) = name {
+            if bx.sess().opts.debuginfo != DebugInfo::Full {
+                return;
             }
 
             // FIXME(eddyb) add debuginfo for unsized places too.
             let place = match local_ref {
                 LocalRef::Place(place) => place,
-                _ => continue,
+                _ => return,
             };
 
-            let decl = &self.mir.local_decls[local];
-            let (name, kind) = if self.mir.local_kind(local) == mir::LocalKind::Arg {
-                let arg_index = local.index() - 1;
-
-                // Add debuginfo even to unnamed arguments.
-                // FIXME(eddyb) is this really needed?
-                let name = if arg_index == 0 && !upvar_debuginfo.is_empty() {
-                    // Hide closure environments from debuginfo.
-                    // FIXME(eddyb) shouldn't `ArgumentVariable` indices
-                    // be offset to account for the hidden environment?
-                    None
-                } else {
-                    Some(decl.name.unwrap_or(kw::Invalid))
-                };
-                (name, VariableKind::ArgumentVariable(arg_index + 1))
-            } else {
-                (decl.name, VariableKind::LocalVariable)
-            };
-            if let Some(name) = name {
-                let (scope, span) = self.debug_loc(mir::SourceInfo {
-                    span: decl.source_info.span,
-                    scope: decl.visibility_scope,
-                });
-                if let Some(scope) = scope {
-                    bx.declare_local(&self.debug_context, name, place.layout.ty, scope,
-                        VariableAccess::DirectVariable { alloca: place.llval },
-                        kind, span);
+            let (scope, span) = self.debug_loc(mir::SourceInfo {
+                span: decl.source_info.span,
+                scope: decl.visibility_scope,
+            });
+            if let Some(scope) = scope {
+                bx.declare_local(&self.debug_context, name, place.layout.ty, scope,
+                    VariableAccess::DirectVariable { alloca: place.llval },
+                    kind, span);
+            }
+        }
+    }
+
+    pub fn debug_introduce_locals(&self, bx: &mut Bx) {
+        let tcx = self.cx.tcx();
+        let upvar_debuginfo = &self.mir.__upvar_debuginfo_codegen_only_do_not_use;
+
+        if bx.sess().opts.debuginfo != DebugInfo::Full {
+            // HACK(eddyb) figure out a way to perhaps disentangle
+            // the use of `declare_local` and `set_var_name`.
+            // Or maybe just running this loop always is not that expensive?
+            if !bx.sess().fewer_names() {
+                for local in self.locals.indices() {
+                    self.debug_introduce_local(bx, local);
                 }
             }
+
+            return;
+        }
+
+        for local in self.locals.indices() {
+            self.debug_introduce_local(bx, local);
         }
 
         // Declare closure captures as if they were local variables.
diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs
@@ -190,31 +190,15 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 return LocalRef::Place(PlaceRef::new_sized(llretptr, layout));
             }
 
-            let decl_name = decl.name.map(|name| name.as_str());
-            let decl_name = decl_name.as_ref().map(|name| &name[..]);
-            let name;
-            let name = if let Some(name) = decl_name {
-                name
-            } else {
-                // FIXME(eddyb) compute something else for the name so no work is done
-                // unless LLVM IR names are turned on (e.g. for `--emit=llvm-ir`).
-                name = format!("{:?}", local);
-                &name
-            };
-
             if memory_locals.contains(local) {
-                debug!("alloc: {:?} ({}) -> place", local, name);
+                debug!("alloc: {:?} -> place", local);
                 if layout.is_unsized() {
-                    let indirect_place = PlaceRef::alloca_unsized_indirect(&mut bx, layout);
-                    bx.set_var_name(indirect_place.llval, name);
-                    LocalRef::UnsizedPlace(indirect_place)
+                    LocalRef::UnsizedPlace(PlaceRef::alloca_unsized_indirect(&mut bx, layout))
                 } else {
-                    let place = PlaceRef::alloca(&mut bx, layout);
-                    bx.set_var_name(place.llval, name);
-                    LocalRef::Place(place)
+                    LocalRef::Place(PlaceRef::alloca(&mut bx, layout))
                 }
             } else {
-                debug!("alloc: {:?} ({}) -> operand", local, name);
+                debug!("alloc: {:?} -> operand", local);
                 LocalRef::new_operand(&mut bx, layout)
             }
         };
@@ -227,7 +211,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     };
 
     // Apply debuginfo to the newly allocated locals.
-    fx.debug_declare_locals(&mut bx);
+    fx.debug_introduce_locals(&mut bx);
 
     // Branch to the START block, if it's not the entry block.
     if reentrant_start_block {
@@ -343,13 +327,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     mir.args_iter().enumerate().map(|(arg_index, local)| {
         let arg_decl = &mir.local_decls[local];
 
-        // FIXME(eddyb) don't allocate a `String` unless it gets used.
-        let name = if let Some(name) = arg_decl.name {
-            name.as_str().to_string()
-        } else {
-            format!("{:?}", local)
-        };
-
         if Some(local) == mir.spread_arg {
             // This argument (e.g., the last argument in the "rust-call" ABI)
             // is a tuple that was spread at the ABI level and now we have
@@ -363,7 +340,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             };
 
             let place = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
-            bx.set_var_name(place.llval, name);
             for i in 0..tupled_arg_tys.len() {
                 let arg = &fx.fn_ty.args[idx];
                 idx += 1;
@@ -381,7 +357,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             let arg_ty = fx.monomorphize(&arg_decl.ty);
 
             let va_list = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
-            bx.set_var_name(va_list.llval, name);
             bx.va_start(va_list.llval);
 
             return LocalRef::Place(va_list);
@@ -404,7 +379,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 }
                 PassMode::Direct(_) => {
                     let llarg = bx.get_param(llarg_idx);
-                    bx.set_var_name(llarg, &name);
                     llarg_idx += 1;
                     return local(
                         OperandRef::from_immediate_or_packed_pair(bx, llarg, arg.layout));
@@ -413,11 +387,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                     let (a, b) = (bx.get_param(llarg_idx), bx.get_param(llarg_idx + 1));
                     llarg_idx += 2;
 
-                    // FIXME(eddyb) these are scalar components,
-                    // maybe extract the high-level fields?
-                    bx.set_var_name(a, format_args!("{}.0", name));
-                    bx.set_var_name(b, format_args!("{}.1", name));
-
                     return local(OperandRef {
                         val: OperandValue::Pair(a, b),
                         layout: arg.layout
@@ -432,7 +401,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             // already put it in a temporary alloca and gave it up.
             // FIXME: lifetimes
             let llarg = bx.get_param(llarg_idx);
-            bx.set_var_name(llarg, &name);
             llarg_idx += 1;
             LocalRef::Place(PlaceRef::new_sized(llarg, arg.layout))
         } else if arg.is_unsized_indirect() {
@@ -445,12 +413,10 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             let indirect_operand = OperandValue::Pair(llarg, llextra);
 
             let tmp = PlaceRef::alloca_unsized_indirect(bx, arg.layout);
-            bx.set_var_name(tmp.llval, name);
             indirect_operand.store(bx, tmp);
             LocalRef::UnsizedPlace(tmp)
         } else {
             let tmp = PlaceRef::alloca(bx, arg.layout);
-            bx.set_var_name(tmp.llval, name);
             bx.store_fn_arg(arg, &mut llarg_idx, tmp);
             LocalRef::Place(tmp)
         }
diff --git a/src/librustc_codegen_ssa/mir/statement.rs b/src/librustc_codegen_ssa/mir/statement.rs
@@ -27,21 +27,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         }
                         LocalRef::Operand(None) => {
                             let (mut bx, operand) = self.codegen_rvalue_operand(bx, rvalue);
-                            if let Some(name) = self.mir.local_decls[index].name {
-                                match operand.val {
-                                    OperandValue::Ref(x, ..) |
-                                    OperandValue::Immediate(x) => {
-                                        bx.set_var_name(x, name);
-                                    }
-                                    OperandValue::Pair(a, b) => {
-                                        // FIXME(eddyb) these are scalar components,
-                                        // maybe extract the high-level fields?
-                                        bx.set_var_name(a, format_args!("{}.0", name));
-                                        bx.set_var_name(b, format_args!("{}.1", name));
-                                    }
-                                }
-                            }
                             self.locals[index] = LocalRef::Operand(Some(operand));
+                            self.debug_introduce_local(&mut bx, index);
                             bx
                         }
                         LocalRef::Operand(Some(op)) => {
diff --git a/src/test/codegen/optimize-attr-1.rs b/src/test/codegen/optimize-attr-1.rs
@@ -8,7 +8,7 @@
 
 // CHECK-LABEL: define i32 @nothing
 // CHECK-SAME: [[NOTHING_ATTRS:#[0-9]+]]
-// NO-OPT: ret i32 %1
+// NO-OPT: ret i32 %_1.0
 // SIZE-OPT: ret i32 4
 // SPEEC-OPT: ret i32 4
 #[no_mangle]
@@ -18,7 +18,7 @@ pub fn nothing() -> i32 {
 
 // CHECK-LABEL: define i32 @size
 // CHECK-SAME: [[SIZE_ATTRS:#[0-9]+]]
-// NO-OPT: ret i32 %1
+// NO-OPT: ret i32 %_1.0
 // SIZE-OPT: ret i32 6
 // SPEED-OPT: ret i32 6
 #[optimize(size)]
@@ -31,7 +31,7 @@ pub fn size() -> i32 {
 // NO-OPT-SAME: [[NOTHING_ATTRS]]
 // SPEED-OPT-SAME: [[NOTHING_ATTRS]]
 // SIZE-OPT-SAME: [[SPEED_ATTRS:#[0-9]+]]
-// NO-OPT: ret i32 %1
+// NO-OPT: ret i32 %_1.0
 // SIZE-OPT: ret i32 8
 // SPEED-OPT: ret i32 8
 #[optimize(speed)]
diff --git a/src/test/codegen/simd-intrinsic/simd-intrinsic-generic-arithmetic-saturating.rs b/src/test/codegen/simd-intrinsic/simd-intrinsic-generic-arithmetic-saturating.rs
diff --git a/src/test/codegen/simd-intrinsic/simd-intrinsic-generic-bitmask.rs b/src/test/codegen/simd-intrinsic/simd-intrinsic-generic-bitmask.rs
