Skip to content

Commit 89cc852

Browse files
committed
auto merge of #9332 : eugals/rust/master, r=alexcrichton
It is intended to optimize/beautify the code generated in a few trivial trait operations. Let's take the following code as an example: ``` trait Stuff { fn bar(&self); } fn callBar(s: &Stuff) { s.bar(); } struct Foo; impl Stuff for Foo { fn bar(&self) { } } pub fn main() { let o = Foo; callBar(&o as &Stuff); } ``` At present it is translated into something like: ``` define void @_ZN7callBar_UUID.0E({ i32, %tydesc*, i8*, i8*, i8 }*, { %tydesc*, i8* }*) #4 { "function top level": %__trait_callee = alloca { %tydesc*, i8* } %__auto_borrow_obj = alloca { %tydesc*, i8* } %2 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 0 %3 = load %tydesc** %2 %4 = getelementptr inbounds { %tydesc*, i8* }* %__auto_borrow_obj, i32 0, i32 0 store %tydesc* %3, %tydesc** %4 %5 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 1 %6 = load i8** %5 %7 = getelementptr inbounds { %tydesc*, i8* }* %__auto_borrow_obj, i32 0, i32 1 store i8* %6, i8** %7 %8 = bitcast { %tydesc*, i8* }* %__auto_borrow_obj to i8* %9 = bitcast { %tydesc*, i8* }* %__trait_callee to i8* call void @llvm.memcpy.p0i8.p0i8.i32(i8* %9, i8* %8, i32 8, i32 4, i1 false) %10 = getelementptr inbounds { %tydesc*, i8* }* %__trait_callee, i32 0, i32 1 %11 = load i8** %10 %12 = bitcast i8* %11 to { i32, %tydesc*, i8*, i8*, i8 }* %13 = getelementptr inbounds { %tydesc*, i8* }* %__trait_callee, i32 0, i32 0 %14 = bitcast %tydesc** %13 to [1 x i8*]** %15 = load [1 x i8*]** %14 %16 = getelementptr inbounds [1 x i8*]* %15, i32 0, i32 1 %17 = load i8** %16 %18 = bitcast i8* %17 to void ({ i32, %tydesc*, i8*, i8*, i8 }*)* call void %18({ i32, %tydesc*, i8*, i8*, i8 }* %12) ret void } ... define void @_ZN4main_UUID.0E({ i32, %tydesc*, i8*, i8*, i8 }*) #4 { "function top level": %o = alloca %struct.Foo %1 = alloca { %tydesc*, i8* } %__auto_borrow_obj = alloca { %tydesc*, i8* } %2 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 1 %3 = bitcast i8** %2 to %struct.Foo** store %struct.Foo* %o, %struct.Foo** %3 %4 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 0 %5 = bitcast %tydesc** %4 to { %tydesc*, void ({ i32, %tydesc*, i8*, i8*, i8 }*)* }** store { %tydesc*, void ({ i32, %tydesc*, i8*, i8*, i8 }*)* }* @vtable1081, { %tydesc*, void ({ i32, %tydesc*, i8*, i8*, i8 }*)* }** %5 %6 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 0 %7 = load %tydesc** %6 %8 = getelementptr inbounds { %tydesc*, i8* }* %__auto_borrow_obj, i32 0, i32 0 store %tydesc* %7, %tydesc** %8 %9 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 1 %10 = load i8** %9 %11 = getelementptr inbounds { %tydesc*, i8* }* %__auto_borrow_obj, i32 0, i32 1 store i8* %10, i8** %11 call void @_ZN7callBar_UUID.0E({ i32, %tydesc*, i8*, i8*, i8 }* undef, { %tydesc*, i8* }* %__auto_borrow_obj) ret void } ``` If you apply my patch, it would become way shorter and cleaner: ``` define void @_ZN7callBar_UUID.0E({ i32, %tydesc*, i8*, i8*, i8 }*, { %tydesc*, i8* }*) #4 { "function top level": %2 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 1 %3 = load i8** %2 %4 = bitcast i8* %3 to { i32, %tydesc*, i8*, i8*, i8 }* %5 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 0 %6 = bitcast %tydesc** %5 to [1 x i8*]** %7 = load [1 x i8*]** %6 %8 = getelementptr inbounds [1 x i8*]* %7, i32 0, i32 1 %9 = load i8** %8 %10 = bitcast i8* %9 to void ({ i32, %tydesc*, i8*, i8*, i8 }*)* call void %10({ i32, %tydesc*, i8*, i8*, i8 }* %4) ret void } ... define void @_ZN4main_UUID.0E({ i32, %tydesc*, i8*, i8*, i8 }*) #4 { "function top level": %o = alloca %struct.Foo %1 = alloca { %tydesc*, i8* } %2 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 1 %3 = bitcast i8** %2 to %struct.Foo** store %struct.Foo* %o, %struct.Foo** %3 %4 = getelementptr inbounds { %tydesc*, i8* }* %1, i32 0, i32 0 %5 = bitcast %tydesc** %4 to { %tydesc*, void ({ i32, %tydesc*, i8*, i8*, i8 }*)* }** store { %tydesc*, void ({ i32, %tydesc*, i8*, i8*, i8 }*)* }* @vtable1081, { %tydesc*, void ({ i32, %tydesc*, i8*, i8*, i8 }*)* }** %5 call void @_ZN7callBar_UUID.0E({ i32, %tydesc*, i8*, i8*, i8 }* undef, { %tydesc*, i8* }* %1) ret void } ``` Although this change doesn't increase the compilation speed much (I mentioned only about 1-2% boost on "rustc -O -Z time-passes syntax.rs"), but I still think it's a good thing to do as it greatly simplifies/clarifies LL generated in some cases which would definitely help in the future code generation investigations. I don't provide any new test cases in this patch as it is merely an optimization. Sorry guys, I somehow messed my previous PR and I don't see any better way to fix as to recreate it here.
2 parents 44997a1 + fadc6cc commit 89cc852

File tree

4 files changed

+51
-31
lines changed

4 files changed

+51
-31
lines changed

Diff for: src/librustc/middle/trans/expr.rs

+30-9
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,36 @@ pub fn trans_to_datum(bcx: @mut Block, expr: @ast::Expr) -> DatumBlock {
313313
let target_obj_ty = expr_ty_adjusted(bcx, expr);
314314
debug!("auto_borrow_obj(target=%s)",
315315
target_obj_ty.repr(tcx));
316+
317+
// Extract source store information
318+
let (source_store, source_mutbl) = match ty::get(source_datum.ty).sty {
319+
ty::ty_trait(_, _, s, m, _) => (s, m),
320+
_ => {
321+
bcx.sess().span_bug(
322+
expr.span,
323+
fmt!("auto_borrow_trait_obj expected a trait, found %s",
324+
source_datum.ty.repr(bcx.tcx())));
325+
}
326+
};
327+
328+
// check if any borrowing is really needed or we could reuse the source_datum instead
329+
match ty::get(target_obj_ty).sty {
330+
ty::ty_trait(_, _, ty::RegionTraitStore(target_scope), target_mutbl, _) => {
331+
if target_mutbl == ast::MutImmutable && target_mutbl == source_mutbl {
332+
match source_store {
333+
ty::RegionTraitStore(source_scope) => {
334+
if tcx.region_maps.is_subregion_of(target_scope, source_scope) {
335+
return DatumBlock { bcx: bcx, datum: source_datum };
336+
}
337+
},
338+
_ => {}
339+
340+
};
341+
}
342+
},
343+
_ => {}
344+
}
345+
316346
let scratch = scratch_datum(bcx, target_obj_ty,
317347
"__auto_borrow_obj", false);
318348

@@ -331,15 +361,6 @@ pub fn trans_to_datum(bcx: @mut Block, expr: @ast::Expr) -> DatumBlock {
331361
// ~T, or &T, depending on source_obj_ty.
332362
let source_data_ptr = GEPi(bcx, source_llval, [0u, abi::trt_field_box]);
333363
let source_data = Load(bcx, source_data_ptr); // always a ptr
334-
let (source_store, source_mutbl) = match ty::get(source_datum.ty).sty {
335-
ty::ty_trait(_, _, s, m, _) => (s, m),
336-
_ => {
337-
bcx.sess().span_bug(
338-
expr.span,
339-
fmt!("auto_borrow_trait_obj expected a trait, found %s",
340-
source_datum.ty.repr(bcx.tcx())));
341-
}
342-
};
343364
let target_data = match source_store {
344365
ty::BoxTraitStore(*) => {
345366
// For deref of @T or @mut T, create a dummy datum and

Diff for: src/librustc/middle/trans/meth.rs

+14-5
Original file line numberDiff line numberDiff line change
@@ -434,13 +434,22 @@ pub fn trans_trait_callee(bcx: @mut Block,
434434
let _icx = push_ctxt("impl::trans_trait_callee");
435435
let mut bcx = bcx;
436436

437+
// make a local copy for trait if needed
437438
let self_ty = expr_ty_adjusted(bcx, self_expr);
438-
let self_scratch = scratch_datum(bcx, self_ty, "__trait_callee", false);
439-
bcx = expr::trans_into(bcx, self_expr, expr::SaveIn(self_scratch.val));
439+
let self_scratch = match ty::get(self_ty).sty {
440+
ty::ty_trait(_, _, ty::RegionTraitStore(*), _, _) => {
441+
unpack_datum!(bcx, expr::trans_to_datum(bcx, self_expr))
442+
}
443+
_ => {
444+
let d = scratch_datum(bcx, self_ty, "__trait_callee", false);
445+
bcx = expr::trans_into(bcx, self_expr, expr::SaveIn(d.val));
446+
// Arrange a temporary cleanup for the object in case something
447+
// should go wrong before the method is actually *invoked*.
448+
d.add_clean(bcx);
449+
d
450+
}
451+
};
440452

441-
// Arrange a temporary cleanup for the object in case something
442-
// should go wrong before the method is actually *invoked*.
443-
self_scratch.add_clean(bcx);
444453

445454
let callee_ty = node_id_type(bcx, callee_id);
446455
trans_trait_callee_from_llval(bcx,

Diff for: src/librustc/middle/trans/type_.rs

+6-17
Original file line numberDiff line numberDiff line change
@@ -278,23 +278,12 @@ impl Type {
278278

279279
pub fn opaque_trait(ctx: &CrateContext, store: ty::TraitStore) -> Type {
280280
let tydesc_ptr = ctx.tydesc_type.ptr_to();
281-
match store {
282-
ty::BoxTraitStore => {
283-
Type::struct_(
284-
[ tydesc_ptr, Type::opaque_box(ctx).ptr_to() ],
285-
false)
286-
}
287-
ty::UniqTraitStore => {
288-
Type::struct_(
289-
[ tydesc_ptr, Type::unique(ctx, &Type::i8()).ptr_to()],
290-
false)
291-
}
292-
ty::RegionTraitStore(*) => {
293-
Type::struct_(
294-
[ tydesc_ptr, Type::i8().ptr_to() ],
295-
false)
296-
}
297-
}
281+
let box_ty = match store {
282+
ty::BoxTraitStore => Type::opaque_box(ctx),
283+
ty::UniqTraitStore => Type::unique(ctx, &Type::i8()),
284+
ty::RegionTraitStore(*) => Type::i8()
285+
};
286+
Type::struct_([tydesc_ptr, box_ty.ptr_to()], false)
298287
}
299288

300289
pub fn kind(&self) -> TypeKind {

Diff for: src/test/run-pass/core-run-destroy.rs

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ fn test_destroy_actually_kills(force: bool) {
5555

5656
#[cfg(windows)]
5757
fn process_exists(pid: libc::pid_t) -> bool {
58+
#[fixed_stack_segment];
5859

5960
use std::libc::types::os::arch::extra::DWORD;
6061
use std::libc::funcs::extra::kernel32::{CloseHandle, GetExitCodeProcess, OpenProcess};

0 commit comments

Comments
 (0)