Skip to content

Commit

Permalink
Rollup merge of rust-lang#40367 - eddyb:naked-cruft, r=nagisa
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton authored Mar 10, 2017
2 parents af9bc3e + df60044 commit d63bbef
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 59 deletions.
3 changes: 1 addition & 2 deletions src/librustc_trans/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
38 changes: 14 additions & 24 deletions src/librustc_trans/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand All @@ -212,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::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()
}
Expand Down Expand Up @@ -307,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
Expand Down
18 changes: 17 additions & 1 deletion src/librustc_trans/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
49 changes: 27 additions & 22 deletions src/librustc_trans/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) => {
Expand Down Expand Up @@ -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)))
}
}
}
Expand Down Expand Up @@ -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)]
Expand Down
47 changes: 37 additions & 10 deletions src/test/codegen/naked-functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}

0 comments on commit d63bbef

Please sign in to comment.