From c4275c2ac376c866c93982e5513f5c6cb6d4dd98 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 8 Mar 2017 20:03:04 +0200 Subject: [PATCH 1/2] rustc_trans: don't emit ZST allocas that are only assigned to. --- src/librustc_trans/mir/analyze.rs | 3 +- src/librustc_trans/mir/mod.rs | 21 +++---------- src/librustc_trans/mir/operand.rs | 18 +++++++++++- src/librustc_trans/mir/rvalue.rs | 49 +++++++++++++++++-------------- 4 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs index 2c3b479c7dd0f..6f0f9cabef345 100644 --- a/src/librustc_trans/mir/analyze.rs +++ b/src/librustc_trans/mir/analyze.rs @@ -18,7 +18,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; @@ -92,7 +91,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 { diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index 6419f41f86b6d..eaedabc1f54c9 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/src/librustc_trans/mir/mod.rs @@ -11,20 +11,18 @@ use libc::c_uint; use llvm::{self, ValueRef, BasicBlockRef}; use llvm::debuginfo::DIScope; -use rustc::ty::{self, layout}; +use rustc::ty::{self, layout, Ty, TypeFoldable}; 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; -use type_of; use syntax_pos::{DUMMY_SP, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos, Span}; use syntax::symbol::keywords; @@ -176,23 +174,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) } diff --git a/src/librustc_trans/mir/operand.rs b/src/librustc_trans/mir/operand.rs index cb77fcbbff85d..df6667afd442e 100644 --- a/src/librustc_trans/mir/operand.rs +++ b/src/librustc_trans/mir/operand.rs @@ -15,7 +15,7 @@ use rustc::mir; 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; @@ -77,6 +77,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 { diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index b6af4e52e820b..4328f2516970e 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -157,7 +157,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 @@ -170,7 +170,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) => { @@ -478,8 +478,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))) } } } @@ -662,26 +664,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)] From df60044df5b531a8fc144024535d0dea43ca8b52 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 8 Mar 2017 20:08:09 +0200 Subject: [PATCH 2/2] rustc_trans: avoid a separate entry BB if START_BLOCK has no backedges. --- src/librustc_trans/mir/mod.rs | 17 ++++++----- src/test/codegen/naked-functions.rs | 47 +++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/librustc_trans/mir/mod.rs b/src/librustc_trans/mir/mod.rs index eaedabc1f54c9..3bcb6e78dce9f 100644 --- a/src/librustc_trans/mir/mod.rs +++ b/src/librustc_trans/mir/mod.rs @@ -199,15 +199,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.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() } @@ -294,9 +296,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 diff --git a/src/test/codegen/naked-functions.rs b/src/test/codegen/naked-functions.rs index 9de74f72005e3..9883ca6b35d04 100644 --- a/src/test/codegen/naked-functions.rs +++ b/src/test/codegen/naked-functions.rs @@ -20,7 +20,8 @@ #[no_mangle] #[naked] fn naked_empty() { - // CHECK: ret void + // CHECK-NEXT: {{.+}}: + // CHECK-NEXT: ret void } // CHECK: Function Attrs: naked uwtable @@ -28,9 +29,10 @@ fn naked_empty() { #[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 @@ -38,7 +40,8 @@ fn naked_with_args(a: isize) { #[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 }