Auto merge of #40367 - eddyb:naked-cruft, r=nagisa

Improve the LLVM IR we generate for trivial functions, especially #[naked] ones.

These two small changes fix edef1c/libfringe#68:
* Don't emit ZST allocas, such as when returning `()`
* Don't emit a branch from LLVM's entry block to MIR's `START_BLOCK` unless needed
  * That is, if a loop branches back to it, although I'm not sure that's even valid MIR

Author: bors

src/librustc_trans/debuginfo/metadata.rs

@@ -784,7 +784,8 @@ pub fn compile_unit_metadata(scc: &SharedCrateContext,
     };
 
     debug!("compile_unit_metadata: {:?}", compile_unit_name);
-    let producer = format!("rustc version {}",
+    // FIXME(#41252) Remove "clang LLVM" if we can get GDB and LLVM to play nice.
+    let producer = format!("clang LLVM (rustc version {})",
                            (option_env!("CFG_VERSION")).expect("CFG_VERSION"));
 
     let compile_unit_name = compile_unit_name.as_ptr();

src/librustc_trans/mir/analyze.rs

@@ -19,7 +19,6 @@ use rustc::mir::visit::{Visitor, LvalueContext};
 use rustc::mir::traversal;
 use common;
 use super::MirContext;
-use super::rvalue;
 
 pub fn lvalue_locals<'a, 'tcx>(mircx: &MirContext<'a, 'tcx>) -> BitVector {
     let mir = mircx.mir;
@@ -93,7 +92,7 @@ impl<'mir, 'a, 'tcx> Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'tcx> {
 
         if let mir::Lvalue::Local(index) = *lvalue {
             self.mark_assigned(index);
-            if !rvalue::rvalue_creates_operand(rvalue) {
+            if !self.cx.rvalue_creates_operand(rvalue) {
                 self.mark_as_lvalue(index);
             }
         } else {

src/librustc_trans/mir/mod.rs

@@ -11,17 +11,16 @@
 use libc::c_uint;
 use llvm::{self, ValueRef, BasicBlockRef};
 use llvm::debuginfo::DIScope;
-use rustc::ty;
+use rustc::ty::{self, Ty, TypeFoldable};
 use rustc::ty::layout::{self, LayoutTyper};
 use rustc::mir::{self, Mir};
 use rustc::mir::tcx::LvalueTy;
 use rustc::ty::subst::Substs;
 use rustc::infer::TransNormalize;
-use rustc::ty::TypeFoldable;
 use session::config::FullDebugInfo;
 use base;
 use builder::Builder;
-use common::{self, CrateContext, C_null, Funclet};
+use common::{self, CrateContext, Funclet};
 use debuginfo::{self, declare_local, VariableAccess, VariableKind, FunctionDebugContext};
 use monomorphize::{self, Instance};
 use abi::FnType;
@@ -171,23 +170,12 @@ enum LocalRef<'tcx> {
 
 impl<'tcx> LocalRef<'tcx> {
     fn new_operand<'a>(ccx: &CrateContext<'a, 'tcx>,
-                         ty: ty::Ty<'tcx>) -> LocalRef<'tcx> {
+                       ty: Ty<'tcx>) -> LocalRef<'tcx> {
         if common::type_is_zero_size(ccx, ty) {
             // Zero-size temporaries aren't always initialized, which
             // doesn't matter because they don't contain data, but
             // we need something in the operand.
-            let llty = type_of::type_of(ccx, ty);
-            let val = if common::type_is_imm_pair(ccx, ty) {
-                let fields = llty.field_types();
-                OperandValue::Pair(C_null(fields[0]), C_null(fields[1]))
-            } else {
-                OperandValue::Immediate(C_null(llty))
-            };
-            let op = OperandRef {
-                val: val,
-                ty: ty
-            };
-            LocalRef::Operand(Some(op))
+            LocalRef::Operand(Some(OperandRef::new_zst(ccx, ty)))
         } else {
             LocalRef::Operand(None)
         }
@@ -207,15 +195,17 @@ pub fn trans_mir<'a, 'tcx: 'a>(
     debug!("fn_ty: {:?}", fn_ty);
     let debug_context =
         debuginfo::create_function_debug_context(ccx, instance, sig, llfn, mir);
-    let bcx = Builder::new_block(ccx, llfn, "entry-block");
+    let bcx = Builder::new_block(ccx, llfn, "start");
 
     let cleanup_kinds = analyze::cleanup_kinds(&mir);
 
-    // Allocate a `Block` for every basic block
+    // Allocate a `Block` for every basic block, except
+    // the start block, if nothing loops back to it.
+    let reentrant_start_block = !mir.predecessors_for(mir::START_BLOCK).is_empty();
     let block_bcxs: IndexVec<mir::BasicBlock, BasicBlockRef> =
         mir.basic_blocks().indices().map(|bb| {
-            if bb == mir::START_BLOCK {
-                bcx.build_sibling_block("start").llbb()
+            if bb == mir::START_BLOCK && !reentrant_start_block {
+                bcx.llbb()
             } else {
                 bcx.build_sibling_block(&format!("{:?}", bb)).llbb()
             }
@@ -301,9 +291,10 @@ pub fn trans_mir<'a, 'tcx: 'a>(
             .collect()
     };
 
-    // Branch to the START block
-    let start_bcx = mircx.blocks[mir::START_BLOCK];
-    bcx.br(start_bcx);
+    // Branch to the START block, if it's not the entry block.
+    if reentrant_start_block {
+        bcx.br(mircx.blocks[mir::START_BLOCK]);
+    }
 
     // Up until here, IR instructions for this function have explicitly not been annotated with
     // source code location, so we don't step into call setup code. From here on, source location

src/librustc_trans/mir/operand.rs

@@ -16,7 +16,7 @@ use rustc::mir::tcx::LvalueTy;
 use rustc_data_structures::indexed_vec::Idx;
 
 use base;
-use common;
+use common::{self, CrateContext, C_null};
 use builder::Builder;
 use value::Value;
 use type_of;
@@ -79,6 +79,22 @@ impl<'tcx> fmt::Debug for OperandRef<'tcx> {
 }
 
 impl<'a, 'tcx> OperandRef<'tcx> {
+    pub fn new_zst(ccx: &CrateContext<'a, 'tcx>,
+                   ty: Ty<'tcx>) -> OperandRef<'tcx> {
+        assert!(common::type_is_zero_size(ccx, ty));
+        let llty = type_of::type_of(ccx, ty);
+        let val = if common::type_is_imm_pair(ccx, ty) {
+            let fields = llty.field_types();
+            OperandValue::Pair(C_null(fields[0]), C_null(fields[1]))
+        } else {
+            OperandValue::Immediate(C_null(llty))
+        };
+        OperandRef {
+            val: val,
+            ty: ty
+        }
+    }
+
     /// Asserts that this operand refers to a scalar and returns
     /// a reference to its value.
     pub fn immediate(self) -> ValueRef {

src/librustc_trans/mir/rvalue.rs

@@ -158,7 +158,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
             }
 
             _ => {
-                assert!(rvalue_creates_operand(rvalue));
+                assert!(self.rvalue_creates_operand(rvalue));
                 let (bcx, temp) = self.trans_rvalue_operand(bcx, rvalue);
                 self.store_operand(&bcx, dest.llval, dest.alignment.to_align(), temp);
                 bcx
@@ -171,7 +171,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
                                 rvalue: &mir::Rvalue<'tcx>)
                                 -> (Builder<'a, 'tcx>, OperandRef<'tcx>)
     {
-        assert!(rvalue_creates_operand(rvalue), "cannot trans {:?} to operand", rvalue);
+        assert!(self.rvalue_creates_operand(rvalue), "cannot trans {:?} to operand", rvalue);
 
         match *rvalue {
             mir::Rvalue::Cast(ref kind, ref source, cast_ty) => {
@@ -466,8 +466,10 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
             }
             mir::Rvalue::Repeat(..) |
             mir::Rvalue::Aggregate(..) => {
-                bug!("cannot generate operand from rvalue {:?}", rvalue);
-
+                // According to `rvalue_creates_operand`, only ZST
+                // aggregate rvalues are allowed to be operands.
+                let ty = rvalue.ty(self.mir, self.ccx.tcx());
+                (bcx, OperandRef::new_zst(self.ccx, self.monomorphize(&ty)))
             }
         }
     }
@@ -650,26 +652,29 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
 
         OperandValue::Pair(val, of)
     }
-}
 
-pub fn rvalue_creates_operand(rvalue: &mir::Rvalue) -> bool {
-    match *rvalue {
-        mir::Rvalue::Ref(..) |
-        mir::Rvalue::Len(..) |
-        mir::Rvalue::Cast(..) | // (*)
-        mir::Rvalue::BinaryOp(..) |
-        mir::Rvalue::CheckedBinaryOp(..) |
-        mir::Rvalue::UnaryOp(..) |
-        mir::Rvalue::Discriminant(..) |
-        mir::Rvalue::Box(..) |
-        mir::Rvalue::Use(..) => // (*)
-            true,
-        mir::Rvalue::Repeat(..) |
-        mir::Rvalue::Aggregate(..) =>
-            false,
-    }
+    pub fn rvalue_creates_operand(&self, rvalue: &mir::Rvalue<'tcx>) -> bool {
+        match *rvalue {
+            mir::Rvalue::Ref(..) |
+            mir::Rvalue::Len(..) |
+            mir::Rvalue::Cast(..) | // (*)
+            mir::Rvalue::BinaryOp(..) |
+            mir::Rvalue::CheckedBinaryOp(..) |
+            mir::Rvalue::UnaryOp(..) |
+            mir::Rvalue::Discriminant(..) |
+            mir::Rvalue::Box(..) |
+            mir::Rvalue::Use(..) => // (*)
+                true,
+            mir::Rvalue::Repeat(..) |
+            mir::Rvalue::Aggregate(..) => {
+                let ty = rvalue.ty(self.mir, self.ccx.tcx());
+                let ty = self.monomorphize(&ty);
+                common::type_is_zero_size(self.ccx, ty)
+            }
+        }
 
-    // (*) this is only true if the type is suitable
+        // (*) this is only true if the type is suitable
+    }
 }
 
 #[derive(Copy, Clone)]

src/test/codegen/naked-functions.rs

@@ -20,25 +20,28 @@
 #[no_mangle]
 #[naked]
 fn naked_empty() {
-    // CHECK: ret void
+    // CHECK-NEXT: {{.+}}:
+    // CHECK-NEXT: ret void
 }
 
 // CHECK: Function Attrs: naked uwtable
 #[no_mangle]
 #[naked]
 // CHECK-NEXT: define internal void @naked_with_args(i{{[0-9]+}})
 fn naked_with_args(a: isize) {
-    // CHECK: %a = alloca i{{[0-9]+}}
-    // CHECK: ret void
+    // CHECK-NEXT: {{.+}}:
+    // CHECK-NEXT: %a = alloca i{{[0-9]+}}
     &a; // keep variable in an alloca
+    // CHECK: ret void
 }
 
 // CHECK: Function Attrs: naked uwtable
 // CHECK-NEXT: define internal i{{[0-9]+}} @naked_with_return()
 #[no_mangle]
 #[naked]
 fn naked_with_return() -> isize {
-    // CHECK: ret i{{[0-9]+}} 0
+    // CHECK-NEXT: {{.+}}:
+    // CHECK-NEXT: ret i{{[0-9]+}} 0
     0
 }
 
@@ -47,9 +50,10 @@ fn naked_with_return() -> isize {
 #[no_mangle]
 #[naked]
 fn naked_with_args_and_return(a: isize) -> isize {
-    // CHECK: %a = alloca i{{[0-9]+}}
-    // CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
+    // CHECK-NEXT: {{.+}}:
+    // CHECK-NEXT: %a = alloca i{{[0-9]+}}
     &a; // keep variable in an alloca
+    // CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
     a
 }
 
@@ -58,14 +62,37 @@ fn naked_with_args_and_return(a: isize) -> isize {
 #[no_mangle]
 #[naked]
 fn naked_recursive() {
-    // CHECK: call void @naked_empty()
+    // CHECK-NEXT: {{.+}}:
+    // CHECK-NEXT: call void @naked_empty()
+
+    // FIXME(#39685) Avoid one block per call.
+    // CHECK-NEXT: br label %bb1
+    // CHECK: bb1:
+
     naked_empty();
-    // CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return()
+
+    // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return()
+
+    // FIXME(#39685) Avoid one block per call.
+    // CHECK-NEXT: br label %bb2
+    // CHECK: bb2:
+
+    // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}})
+
+    // FIXME(#39685) Avoid one block per call.
+    // CHECK-NEXT: br label %bb3
+    // CHECK: bb3:
+
+    // CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}})
+
+    // FIXME(#39685) Avoid one block per call.
+    // CHECK-NEXT: br label %bb4
+    // CHECK: bb4:
+
     naked_with_args(
-        // CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}})
         naked_with_args_and_return(
-            // CHECK: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}})
             naked_with_return()
         )
     );
+    // CHECK-NEXT: ret void
 }