Rollup merge of rust-lang#64149 - eddyb:llvm-var-names, r=rkruppe

rustc_codegen_llvm: give names to non-alloca variable values.

These names only matter when looking at LLVM IR, but they can help.

When one value is used for multiple variables, I decided to combine the names.
I chose `,` as a separator but maybe `=` or ` ` (space) are more appropriate.
(LLVM names can contain any characters - if necessary they end up having quotes)

As an example, this function:
```rust
#[no_mangle]
pub fn test(a: u32, b: u32) -> u32 {
    let c = a + b;
    let d = c;
    let e = d * a;
    e
}
```
Used to produce this LLVM IR:
```llvm
define i32 @test(i32 %a, i32 %b) unnamed_addr #0 {
start:
  %0 = add i32 %a, %b
  %1 = mul i32 %0, %a
  ret i32 %1
}
```
But after this PR you get this:
```llvm
define i32 @test(i32 %a, i32 %b) unnamed_addr #0 {
start:
  %"c,d" = add i32 %a, %b
  %e = mul i32 %"c,d", %a
  ret i32 %e
}
```

cc @nagisa @rkruppe

Author: Centril

Diff for: src/librustc_codegen_llvm/debuginfo/mod.rs

@@ -32,7 +32,7 @@ use rustc_codegen_ssa::debuginfo::{FunctionDebugContext, MirDebugScope, Variable
 
 use libc::c_uint;
 use std::cell::RefCell;
-use std::ffi::CString;
+use std::ffi::{CStr, CString};
 
 use syntax_pos::{self, Span, Pos};
 use syntax::ast;
@@ -224,8 +224,37 @@ impl DebugInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         gdb::insert_reference_to_gdb_debug_scripts_section_global(self)
     }
 
-    fn set_value_name(&mut self, value: &'ll Value, name: &str) {
-        let cname = SmallCStr::new(name);
+    fn set_var_name(&mut self, value: &'ll Value, name: impl ToString) {
+        // Avoid wasting time if LLVM value names aren't even enabled.
+        if self.sess().fewer_names() {
+            return;
+        }
+
+        // Only function parameters and instructions are local to a function,
+        // don't change the name of anything else (e.g. globals).
+        let param_or_inst = unsafe {
+            llvm::LLVMIsAArgument(value).is_some() ||
+            llvm::LLVMIsAInstruction(value).is_some()
+        };
+        if !param_or_inst {
+            return;
+        }
+
+        let old_name = unsafe {
+            CStr::from_ptr(llvm::LLVMGetValueName(value))
+        };
+        match old_name.to_str() {
+            Ok("") => {}
+            Ok(_) => {
+                // Avoid replacing the name if it already exists.
+                // While we could combine the names somehow, it'd
+                // get noisy quick, and the usefulness is dubious.
+                return;
+            }
+            Err(_) => return,
+        }
+
+        let cname = CString::new(name.to_string()).unwrap();
         unsafe {
             llvm::LLVMSetValueName(value, cname.as_ptr());
         }

Diff for: src/librustc_codegen_llvm/llvm/ffi.rs

@@ -806,6 +806,7 @@ extern "C" {
     pub fn LLVMRustRemoveFunctionAttributes(Fn: &Value, index: c_uint, attr: Attribute);
 
     // Operations on parameters
+    pub fn LLVMIsAArgument(Val: &Value) -> Option<&Value>;
     pub fn LLVMCountParams(Fn: &Value) -> c_uint;
     pub fn LLVMGetParam(Fn: &Value, Index: c_uint) -> &Value;
 
@@ -818,6 +819,7 @@ extern "C" {
     pub fn LLVMDeleteBasicBlock(BB: &BasicBlock);
 
     // Operations on instructions
+    pub fn LLVMIsAInstruction(Val: &Value) -> Option<&Value>;
     pub fn LLVMGetFirstBasicBlock(Fn: &Value) -> &BasicBlock;
 
     // Operations on call sites

Diff for: src/librustc_codegen_ssa/mir/mod.rs

@@ -518,19 +518,19 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 PassMode::Ignore(IgnoreMode::CVarArgs) => {}
                 PassMode::Direct(_) => {
                     let llarg = bx.get_param(llarg_idx);
-                    bx.set_value_name(llarg, &name);
+                    bx.set_var_name(llarg, &name);
                     llarg_idx += 1;
                     return local(
                         OperandRef::from_immediate_or_packed_pair(bx, llarg, arg.layout));
                 }
                 PassMode::Pair(..) => {
-                    let a = bx.get_param(llarg_idx);
-                    bx.set_value_name(a, &(name.clone() + ".0"));
-                    llarg_idx += 1;
+                    let (a, b) = (bx.get_param(llarg_idx), bx.get_param(llarg_idx + 1));
+                    llarg_idx += 2;
 
-                    let b = bx.get_param(llarg_idx);
-                    bx.set_value_name(b, &(name + ".1"));
-                    llarg_idx += 1;
+                    // 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),
@@ -546,7 +546,7 @@ 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_value_name(llarg, &name);
+            bx.set_var_name(llarg, &name);
             llarg_idx += 1;
             PlaceRef::new_sized(llarg, arg.layout)
         } else if arg.is_unsized_indirect() {

Diff for: src/librustc_codegen_ssa/mir/statement.rs

@@ -29,7 +29,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                             self.codegen_rvalue_unsized(bx, cg_indirect_dest, rvalue)
                         }
                         LocalRef::Operand(None) => {
-                            let (bx, operand) = self.codegen_rvalue_operand(bx, rvalue);
+                            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));
                             bx
                         }

Diff for: src/librustc_codegen_ssa/traits/debuginfo.rs

@@ -57,5 +57,5 @@ pub trait DebugInfoBuilderMethods<'tcx>: BackendTypes {
         span: Span,
     );
     fn insert_reference_to_gdb_debug_scripts_section_global(&mut self);
-    fn set_value_name(&mut self, value: Self::Value, name: &str);
+    fn set_var_name(&mut self, value: Self::Value, name: impl ToString);
 }

Diff for: src/test/codegen/var-names.rs

@@ -0,0 +1,15 @@
+// compile-flags: -O -C no-prepopulate-passes
+
+#![crate_type = "lib"]
+
+// CHECK-LABEL: define i32 @test(i32 %a, i32 %b)
+#[no_mangle]
+pub fn test(a: u32, b: u32) -> u32 {
+    let c = a + b;
+    // CHECK: %c = add i32 %a, %b
+    let d = c;
+    let e = d * a;
+    // CHECK-NEXT: %e = mul i32 %c, %a
+    e
+    // CHECK-NEXT: ret i32 %e
+}