Skip to content

Commit 72fc6d4

Browse files
author
Ariel Ben-Yehuda
authored
Rollup merge of rust-lang#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
2 parents d042c51 + df60044 commit 72fc6d4

File tree

5 files changed

+96
-59
lines changed

5 files changed

+96
-59
lines changed

‎src/librustc_trans/mir/analyze.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use rustc::mir::visit::{Visitor, LvalueContext};
1818
use rustc::mir::traversal;
1919
use common;
2020
use super::MirContext;
21-
use super::rvalue;
2221

2322
pub fn lvalue_locals<'a, 'tcx>(mircx: &MirContext<'a, 'tcx>) -> BitVector {
2423
let mir = mircx.mir;
@@ -92,7 +91,7 @@ impl<'mir, 'a, 'tcx> Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'tcx> {
9291

9392
if let mir::Lvalue::Local(index) = *lvalue {
9493
self.mark_assigned(index);
95-
if !rvalue::rvalue_creates_operand(rvalue) {
94+
if !self.cx.rvalue_creates_operand(rvalue) {
9695
self.mark_as_lvalue(index);
9796
}
9897
} else {

‎src/librustc_trans/mir/mod.rs

+14-24
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,18 @@
1111
use libc::c_uint;
1212
use llvm::{self, ValueRef, BasicBlockRef};
1313
use llvm::debuginfo::DIScope;
14-
use rustc::ty::{self, layout};
14+
use rustc::ty::{self, layout, Ty, TypeFoldable};
1515
use rustc::mir::{self, Mir};
1616
use rustc::mir::tcx::LvalueTy;
1717
use rustc::ty::subst::Substs;
1818
use rustc::infer::TransNormalize;
19-
use rustc::ty::TypeFoldable;
2019
use session::config::FullDebugInfo;
2120
use base;
2221
use builder::Builder;
23-
use common::{self, CrateContext, C_null, Funclet};
22+
use common::{self, CrateContext, Funclet};
2423
use debuginfo::{self, declare_local, VariableAccess, VariableKind, FunctionDebugContext};
2524
use monomorphize::{self, Instance};
2625
use abi::FnType;
27-
use type_of;
2826

2927
use syntax_pos::{DUMMY_SP, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos, Span};
3028
use syntax::symbol::keywords;
@@ -176,23 +174,12 @@ enum LocalRef<'tcx> {
176174

177175
impl<'tcx> LocalRef<'tcx> {
178176
fn new_operand<'a>(ccx: &CrateContext<'a, 'tcx>,
179-
ty: ty::Ty<'tcx>) -> LocalRef<'tcx> {
177+
ty: Ty<'tcx>) -> LocalRef<'tcx> {
180178
if common::type_is_zero_size(ccx, ty) {
181179
// Zero-size temporaries aren't always initialized, which
182180
// doesn't matter because they don't contain data, but
183181
// we need something in the operand.
184-
let llty = type_of::type_of(ccx, ty);
185-
let val = if common::type_is_imm_pair(ccx, ty) {
186-
let fields = llty.field_types();
187-
OperandValue::Pair(C_null(fields[0]), C_null(fields[1]))
188-
} else {
189-
OperandValue::Immediate(C_null(llty))
190-
};
191-
let op = OperandRef {
192-
val: val,
193-
ty: ty
194-
};
195-
LocalRef::Operand(Some(op))
182+
LocalRef::Operand(Some(OperandRef::new_zst(ccx, ty)))
196183
} else {
197184
LocalRef::Operand(None)
198185
}
@@ -212,15 +199,17 @@ pub fn trans_mir<'a, 'tcx: 'a>(
212199
debug!("fn_ty: {:?}", fn_ty);
213200
let debug_context =
214201
debuginfo::create_function_debug_context(ccx, instance, sig, llfn, mir);
215-
let bcx = Builder::new_block(ccx, llfn, "entry-block");
202+
let bcx = Builder::new_block(ccx, llfn, "start");
216203

217204
let cleanup_kinds = analyze::cleanup_kinds(&mir);
218205

219-
// Allocate a `Block` for every basic block
206+
// Allocate a `Block` for every basic block, except
207+
// the start block, if nothing loops back to it.
208+
let reentrant_start_block = !mir.predecessors_for(mir::START_BLOCK).is_empty();
220209
let block_bcxs: IndexVec<mir::BasicBlock, BasicBlockRef> =
221210
mir.basic_blocks().indices().map(|bb| {
222-
if bb == mir::START_BLOCK {
223-
bcx.build_sibling_block("start").llbb()
211+
if bb == mir::START_BLOCK && !reentrant_start_block {
212+
bcx.llbb()
224213
} else {
225214
bcx.build_sibling_block(&format!("{:?}", bb)).llbb()
226215
}
@@ -307,9 +296,10 @@ pub fn trans_mir<'a, 'tcx: 'a>(
307296
.collect()
308297
};
309298

310-
// Branch to the START block
311-
let start_bcx = mircx.blocks[mir::START_BLOCK];
312-
bcx.br(start_bcx);
299+
// Branch to the START block, if it's not the entry block.
300+
if reentrant_start_block {
301+
bcx.br(mircx.blocks[mir::START_BLOCK]);
302+
}
313303

314304
// Up until here, IR instructions for this function have explicitly not been annotated with
315305
// source code location, so we don't step into call setup code. From here on, source location

‎src/librustc_trans/mir/operand.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc::mir;
1515
use rustc_data_structures::indexed_vec::Idx;
1616

1717
use base;
18-
use common;
18+
use common::{self, CrateContext, C_null};
1919
use builder::Builder;
2020
use value::Value;
2121
use type_of;
@@ -77,6 +77,22 @@ impl<'tcx> fmt::Debug for OperandRef<'tcx> {
7777
}
7878

7979
impl<'a, 'tcx> OperandRef<'tcx> {
80+
pub fn new_zst(ccx: &CrateContext<'a, 'tcx>,
81+
ty: Ty<'tcx>) -> OperandRef<'tcx> {
82+
assert!(common::type_is_zero_size(ccx, ty));
83+
let llty = type_of::type_of(ccx, ty);
84+
let val = if common::type_is_imm_pair(ccx, ty) {
85+
let fields = llty.field_types();
86+
OperandValue::Pair(C_null(fields[0]), C_null(fields[1]))
87+
} else {
88+
OperandValue::Immediate(C_null(llty))
89+
};
90+
OperandRef {
91+
val: val,
92+
ty: ty
93+
}
94+
}
95+
8096
/// Asserts that this operand refers to a scalar and returns
8197
/// a reference to its value.
8298
pub fn immediate(self) -> ValueRef {

‎src/librustc_trans/mir/rvalue.rs

+27-22
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
157157
}
158158

159159
_ => {
160-
assert!(rvalue_creates_operand(rvalue));
160+
assert!(self.rvalue_creates_operand(rvalue));
161161
let (bcx, temp) = self.trans_rvalue_operand(bcx, rvalue);
162162
self.store_operand(&bcx, dest.llval, dest.alignment.to_align(), temp);
163163
bcx
@@ -170,7 +170,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
170170
rvalue: &mir::Rvalue<'tcx>)
171171
-> (Builder<'a, 'tcx>, OperandRef<'tcx>)
172172
{
173-
assert!(rvalue_creates_operand(rvalue), "cannot trans {:?} to operand", rvalue);
173+
assert!(self.rvalue_creates_operand(rvalue), "cannot trans {:?} to operand", rvalue);
174174

175175
match *rvalue {
176176
mir::Rvalue::Cast(ref kind, ref source, cast_ty) => {
@@ -478,8 +478,10 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
478478
}
479479
mir::Rvalue::Repeat(..) |
480480
mir::Rvalue::Aggregate(..) => {
481-
bug!("cannot generate operand from rvalue {:?}", rvalue);
482-
481+
// According to `rvalue_creates_operand`, only ZST
482+
// aggregate rvalues are allowed to be operands.
483+
let ty = rvalue.ty(self.mir, self.ccx.tcx());
484+
(bcx, OperandRef::new_zst(self.ccx, self.monomorphize(&ty)))
483485
}
484486
}
485487
}
@@ -662,26 +664,29 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
662664

663665
OperandValue::Pair(val, of)
664666
}
665-
}
666667

667-
pub fn rvalue_creates_operand(rvalue: &mir::Rvalue) -> bool {
668-
match *rvalue {
669-
mir::Rvalue::Ref(..) |
670-
mir::Rvalue::Len(..) |
671-
mir::Rvalue::Cast(..) | // (*)
672-
mir::Rvalue::BinaryOp(..) |
673-
mir::Rvalue::CheckedBinaryOp(..) |
674-
mir::Rvalue::UnaryOp(..) |
675-
mir::Rvalue::Discriminant(..) |
676-
mir::Rvalue::Box(..) |
677-
mir::Rvalue::Use(..) =>
678-
true,
679-
mir::Rvalue::Repeat(..) |
680-
mir::Rvalue::Aggregate(..) =>
681-
false,
682-
}
668+
pub fn rvalue_creates_operand(&self, rvalue: &mir::Rvalue<'tcx>) -> bool {
669+
match *rvalue {
670+
mir::Rvalue::Ref(..) |
671+
mir::Rvalue::Len(..) |
672+
mir::Rvalue::Cast(..) | // (*)
673+
mir::Rvalue::BinaryOp(..) |
674+
mir::Rvalue::CheckedBinaryOp(..) |
675+
mir::Rvalue::UnaryOp(..) |
676+
mir::Rvalue::Discriminant(..) |
677+
mir::Rvalue::Box(..) |
678+
mir::Rvalue::Use(..) =>
679+
true,
680+
mir::Rvalue::Repeat(..) |
681+
mir::Rvalue::Aggregate(..) => {
682+
let ty = rvalue.ty(self.mir, self.ccx.tcx());
683+
let ty = self.monomorphize(&ty);
684+
common::type_is_zero_size(self.ccx, ty)
685+
}
686+
}
683687

684-
// (*) this is only true if the type is suitable
688+
// (*) this is only true if the type is suitable
689+
}
685690
}
686691

687692
#[derive(Copy, Clone)]

‎src/test/codegen/naked-functions.rs

+37-10
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,28 @@
2020
#[no_mangle]
2121
#[naked]
2222
fn naked_empty() {
23-
// CHECK: ret void
23+
// CHECK-NEXT: {{.+}}:
24+
// CHECK-NEXT: ret void
2425
}
2526

2627
// CHECK: Function Attrs: naked uwtable
2728
#[no_mangle]
2829
#[naked]
2930
// CHECK-NEXT: define internal void @naked_with_args(i{{[0-9]+}})
3031
fn naked_with_args(a: isize) {
31-
// CHECK: %a = alloca i{{[0-9]+}}
32-
// CHECK: ret void
32+
// CHECK-NEXT: {{.+}}:
33+
// CHECK-NEXT: %a = alloca i{{[0-9]+}}
3334
&a; // keep variable in an alloca
35+
// CHECK: ret void
3436
}
3537

3638
// CHECK: Function Attrs: naked uwtable
3739
// CHECK-NEXT: define internal i{{[0-9]+}} @naked_with_return()
3840
#[no_mangle]
3941
#[naked]
4042
fn naked_with_return() -> isize {
41-
// CHECK: ret i{{[0-9]+}} 0
43+
// CHECK-NEXT: {{.+}}:
44+
// CHECK-NEXT: ret i{{[0-9]+}} 0
4245
0
4346
}
4447

@@ -47,9 +50,10 @@ fn naked_with_return() -> isize {
4750
#[no_mangle]
4851
#[naked]
4952
fn naked_with_args_and_return(a: isize) -> isize {
50-
// CHECK: %a = alloca i{{[0-9]+}}
51-
// CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
53+
// CHECK-NEXT: {{.+}}:
54+
// CHECK-NEXT: %a = alloca i{{[0-9]+}}
5255
&a; // keep variable in an alloca
56+
// CHECK: ret i{{[0-9]+}} %{{[0-9]+}}
5357
a
5458
}
5559

@@ -58,14 +62,37 @@ fn naked_with_args_and_return(a: isize) -> isize {
5862
#[no_mangle]
5963
#[naked]
6064
fn naked_recursive() {
61-
// CHECK: call void @naked_empty()
65+
// CHECK-NEXT: {{.+}}:
66+
// CHECK-NEXT: call void @naked_empty()
67+
68+
// FIXME(#39685) Avoid one block per call.
69+
// CHECK-NEXT: br label %bb1
70+
// CHECK: bb1:
71+
6272
naked_empty();
63-
// CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return()
73+
74+
// CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return()
75+
76+
// FIXME(#39685) Avoid one block per call.
77+
// CHECK-NEXT: br label %bb2
78+
// CHECK: bb2:
79+
80+
// CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}})
81+
82+
// FIXME(#39685) Avoid one block per call.
83+
// CHECK-NEXT: br label %bb3
84+
// CHECK: bb3:
85+
86+
// CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}})
87+
88+
// FIXME(#39685) Avoid one block per call.
89+
// CHECK-NEXT: br label %bb4
90+
// CHECK: bb4:
91+
6492
naked_with_args(
65-
// CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}})
6693
naked_with_args_and_return(
67-
// CHECK: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}})
6894
naked_with_return()
6995
)
7096
);
97+
// CHECK-NEXT: ret void
7198
}

0 commit comments

Comments
 (0)