Skip to content

Commit ba26efb

Browse files
committed
Implement filling drop in MIR
Hopefully the author caught all the cases. For the mir_dynamic_drops_3 test case the ratio of memsets to other instructions is 12%. On the other hand we actually do not double drop for at least the test cases provided anymore in MIR.
1 parent be7196a commit ba26efb

File tree

11 files changed

+147
-83
lines changed

11 files changed

+147
-83
lines changed

src/librustc_llvm/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2165,6 +2165,8 @@ extern {
21652165
NumInputs: c_uint)
21662166
-> OperandBundleDefRef;
21672167
pub fn LLVMRustFreeOperandBundleDef(Bundle: OperandBundleDefRef);
2168+
2169+
pub fn LLVMRustPositionBuilderAtStart(B: BuilderRef, BB: BasicBlockRef);
21682170
}
21692171

21702172
// LLVM requires symbols from this library, but apparently they're not printed

src/librustc_trans/trans/base.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,22 +1298,29 @@ pub fn init_zero_mem<'blk, 'tcx>(cx: Block<'blk, 'tcx>, llptr: ValueRef, t: Ty<'
12981298
fn memfill<'a, 'tcx>(b: &Builder<'a, 'tcx>, llptr: ValueRef, ty: Ty<'tcx>, byte: u8) {
12991299
let _icx = push_ctxt("memfill");
13001300
let ccx = b.ccx;
1301-
13021301
let llty = type_of::type_of(ccx, ty);
1303-
let ptr_width = &ccx.sess().target.target.target_pointer_width[..];
1304-
let intrinsic_key = format!("llvm.memset.p0i8.i{}", ptr_width);
1305-
1306-
let llintrinsicfn = ccx.get_intrinsic(&intrinsic_key);
13071302
let llptr = b.pointercast(llptr, Type::i8(ccx).ptr_to());
13081303
let llzeroval = C_u8(ccx, byte);
13091304
let size = machine::llsize_of(ccx, llty);
13101305
let align = C_i32(ccx, type_of::align_of(ccx, ty) as i32);
1311-
let volatile = C_bool(ccx, false);
1312-
b.call(llintrinsicfn,
1313-
&[llptr, llzeroval, size, align, volatile],
1314-
None, None);
1306+
call_memset(b, llptr, llzeroval, size, align, false);
13151307
}
13161308

1309+
pub fn call_memset<'bcx, 'tcx>(b: &Builder<'bcx, 'tcx>,
1310+
ptr: ValueRef,
1311+
fill_byte: ValueRef,
1312+
size: ValueRef,
1313+
align: ValueRef,
1314+
volatile: bool) {
1315+
let ccx = b.ccx;
1316+
let ptr_width = &ccx.sess().target.target.target_pointer_width[..];
1317+
let intrinsic_key = format!("llvm.memset.p0i8.i{}", ptr_width);
1318+
let llintrinsicfn = ccx.get_intrinsic(&intrinsic_key);
1319+
let volatile = C_bool(ccx, volatile);
1320+
b.call(llintrinsicfn, &[ptr, fill_byte, size, align, volatile], None, None);
1321+
}
1322+
1323+
13171324
/// In general, when we create an scratch value in an alloca, the
13181325
/// creator may not know if the block (that initializes the scratch
13191326
/// with the desired value) actually dominates the cleanup associated

src/librustc_trans/trans/builder.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
104104
}
105105
}
106106

107+
pub fn position_at_start(&self, llbb: BasicBlockRef) {
108+
unsafe {
109+
llvm::LLVMRustPositionBuilderAtStart(self.llbuilder, llbb);
110+
}
111+
}
112+
107113
pub fn ret_void(&self) {
108114
self.count_insn("retvoid");
109115
unsafe {

src/librustc_trans/trans/common.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,15 @@ impl<'blk, 'tcx> BlockAndBuilder<'blk, 'tcx> {
735735
BlockAndBuilder::new(bcx, owned_builder)
736736
}
737737

738+
pub fn at_start<F, R>(&self, f: F) -> R
739+
where F: FnOnce(&BlockAndBuilder<'blk, 'tcx>) -> R
740+
{
741+
self.position_at_start(self.bcx.llbb);
742+
let r = f(self);
743+
self.position_at_end(self.bcx.llbb);
744+
r
745+
}
746+
738747
// Methods delegated to bcx
739748

740749
pub fn ccx(&self) -> &'blk CrateContext<'blk, 'tcx> {

src/librustc_trans/trans/mir/block.rs

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ use trans::common::{self, Block, BlockAndBuilder};
2020
use trans::debuginfo::DebugLoc;
2121
use trans::Disr;
2222
use trans::foreign;
23-
use trans::glue;
2423
use trans::type_of;
24+
use trans::glue;
2525
use trans::type_::Type;
2626

27-
use super::MirContext;
27+
use super::{MirContext, drop};
2828
use super::operand::OperandValue::{FatPtr, Immediate, Ref};
2929
use super::operand::OperandRef;
3030

@@ -188,8 +188,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
188188
unwind.llbb(),
189189
cleanup_bundle.as_ref(),
190190
None);
191+
self.bcx(target).at_start(|bcx| drop::drop_fill(bcx, lvalue.llval, ty));
191192
} else {
192193
bcx.call(drop_fn, &[llvalue], cleanup_bundle.as_ref(), None);
194+
drop::drop_fill(&bcx, lvalue.llval, ty);
193195
funclet_br(bcx, self.llblock(target));
194196
}
195197
}
@@ -250,59 +252,41 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
250252
landingpad.llbb(),
251253
cleanup_bundle.as_ref(),
252254
Some(attrs));
255+
landingpad.at_start(|bcx| for op in args {
256+
self.set_operand_dropped(bcx, op);
257+
});
253258
},
254259
(false, &Some(cleanup), &Some((_, success))) => {
255260
let cleanup = self.bcx(cleanup);
256261
let landingpad = self.make_landing_pad(cleanup);
257-
let (target, postinvoke) = if must_copy_dest {
258-
(self.fcx.new_block("", None).build(), Some(self.bcx(success)))
259-
} else {
260-
(self.bcx(success), None)
261-
};
262262
let invokeret = bcx.invoke(callee.immediate(),
263263
&llargs[..],
264-
target.llbb(),
264+
self.llblock(success),
265265
landingpad.llbb(),
266266
cleanup_bundle.as_ref(),
267267
Some(attrs));
268-
if let Some(postinvoketarget) = postinvoke {
269-
// We translate the copy into a temporary block. The temporary block is
270-
// necessary because the current block has already been terminated (by
271-
// `invoke`) and we cannot really translate into the target block
272-
// because:
273-
// * The target block may have more than a single precedesor;
274-
// * Some LLVM insns cannot have a preceeding store insn (phi,
275-
// cleanuppad), and adding/prepending the store now may render
276-
// those other instructions invalid.
277-
//
278-
// NB: This approach still may break some LLVM code. For example if the
279-
// target block starts with a `phi` (which may only match on immediate
280-
// precedesors), it cannot know about this temporary block thus
281-
// resulting in an invalid code:
282-
//
283-
// this:
284-
// …
285-
// %0 = …
286-
// %1 = invoke to label %temp …
287-
// temp:
288-
// store ty %1, ty* %dest
289-
// br label %actualtargetblock
290-
// actualtargetblock: ; preds: %temp, …
291-
// phi … [%this, …], [%0, …] ; ERROR: phi requires to match only on
292-
// ; immediate precedesors
268+
if must_copy_dest {
293269
let (ret_dest, ret_ty) = ret_dest_ty
294270
.expect("return destination and type not set");
295-
target.with_block(|target| {
296-
base::store_ty(target, invokeret, ret_dest.llval, ret_ty);
297-
});
298-
target.br(postinvoketarget.llbb());
271+
// We translate the copy straight into the beginning of the target
272+
// block.
273+
self.bcx(success).at_start(|bcx| bcx.with_block( |bcx| {
274+
base::store_ty(bcx, invokeret, ret_dest.llval, ret_ty);
275+
}));
299276
}
277+
self.bcx(success).at_start(|bcx| for op in args {
278+
self.set_operand_dropped(bcx, op);
279+
});
280+
landingpad.at_start(|bcx| for op in args {
281+
self.set_operand_dropped(bcx, op);
282+
});
300283
},
301284
(false, _, &None) => {
302285
bcx.call(callee.immediate(),
303286
&llargs[..],
304287
cleanup_bundle.as_ref(),
305288
Some(attrs));
289+
// no need to drop args, because the call never returns
306290
bcx.unreachable();
307291
}
308292
(false, _, &Some((_, target))) => {
@@ -317,6 +301,9 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
317301
base::store_ty(bcx, llret, ret_dest.llval, ret_ty);
318302
});
319303
}
304+
for op in args {
305+
self.set_operand_dropped(&bcx, op);
306+
}
320307
funclet_br(bcx, self.llblock(target));
321308
}
322309
// Foreign functions
@@ -333,6 +320,9 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
333320
debugloc)
334321
});
335322
if let Some((_, target)) = *destination {
323+
for op in args {
324+
self.set_operand_dropped(&bcx, op);
325+
}
336326
funclet_br(bcx, self.llblock(target));
337327
}
338328
},
@@ -388,7 +378,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
388378
let use_funclets = base::wants_msvc_seh(bcx.sess()) && data.is_cleanup;
389379
let cleanup_pad = if use_funclets {
390380
bcx.set_personality_fn(self.fcx.eh_personality());
391-
Some(bcx.cleanup_pad(None, &[]))
381+
bcx.at_start(|bcx| Some(bcx.cleanup_pad(None, &[])))
392382
} else {
393383
None
394384
};
@@ -416,7 +406,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
416406
self.blocks[bb.index()].build()
417407
}
418408

419-
fn llblock(&self, bb: mir::BasicBlock) -> BasicBlockRef {
409+
pub fn llblock(&self, bb: mir::BasicBlock) -> BasicBlockRef {
420410
self.blocks[bb.index()].llbb
421411
}
422412
}

src/librustc_trans/trans/mir/drop.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use llvm::ValueRef;
12+
use rustc::middle::ty::Ty;
13+
use trans::adt;
14+
use trans::base;
15+
use trans::common::{self, BlockAndBuilder};
16+
use trans::machine;
17+
use trans::type_of;
18+
use trans::type_::Type;
19+
20+
pub fn drop_fill<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>, value: ValueRef, ty: Ty<'tcx>) {
21+
let llty = type_of::type_of(bcx.ccx(), ty);
22+
let llptr = bcx.pointercast(value, Type::i8(bcx.ccx()).ptr_to());
23+
let filling = common::C_u8(bcx.ccx(), adt::DTOR_DONE);
24+
let size = machine::llsize_of(bcx.ccx(), llty);
25+
let align = common::C_u32(bcx.ccx(), machine::llalign_of_min(bcx.ccx(), llty));
26+
base::call_memset(&bcx, llptr, filling, size, align, false);
27+
}

src/librustc_trans/trans/mir/lvalue.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use trans::base;
1717
use trans::common::{self, BlockAndBuilder};
1818
use trans::machine;
1919
use trans::type_of;
20+
use trans::mir::drop;
2021
use llvm;
2122
use trans::Disr;
2223

@@ -48,6 +49,7 @@ impl<'tcx> LvalueRef<'tcx> {
4849
{
4950
assert!(!ty.has_erasable_regions());
5051
let lltemp = bcx.with_block(|bcx| base::alloc_ty(bcx, ty, name));
52+
drop::drop_fill(bcx, lltemp, ty);
5153
LvalueRef::new_sized(lltemp, LvalueTy::from_ty(ty))
5254
}
5355
}

src/librustc_trans/trans/mir/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ mod analyze;
197197
mod block;
198198
mod constant;
199199
mod did;
200+
mod drop;
200201
mod lvalue;
201202
mod operand;
202203
mod rvalue;

src/librustc_trans/trans/mir/operand.rs

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ use trans::base;
1616
use trans::common::{self, Block, BlockAndBuilder};
1717
use trans::datum;
1818
use trans::Disr;
19+
use trans::glue;
1920

20-
use super::{MirContext, TempRef};
21+
use super::{MirContext, TempRef, drop};
2122
use super::lvalue::LvalueRef;
2223

2324
/// The representation of a Rust value. The enum variant is in fact
@@ -158,31 +159,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
158159
}
159160
}
160161

161-
pub fn trans_operand_into(&mut self,
162-
bcx: &BlockAndBuilder<'bcx, 'tcx>,
163-
lldest: ValueRef,
164-
operand: &mir::Operand<'tcx>)
165-
{
166-
debug!("trans_operand_into(lldest={}, operand={:?})",
167-
bcx.val_to_string(lldest),
168-
operand);
169-
170-
// FIXME: consider not copying constants through the
171-
// stack.
172-
173-
let o = self.trans_operand(bcx, operand);
174-
self.store_operand(bcx, lldest, o);
175-
}
176-
177162
pub fn store_operand(&mut self,
178163
bcx: &BlockAndBuilder<'bcx, 'tcx>,
179164
lldest: ValueRef,
180165
operand: OperandRef<'tcx>)
181166
{
182167
debug!("store_operand: operand={}", operand.repr(bcx));
183-
bcx.with_block(|bcx| {
184-
self.store_operand_direct(bcx, lldest, operand)
185-
})
168+
bcx.with_block(|bcx| self.store_operand_direct(bcx, lldest, operand))
186169
}
187170

188171
pub fn store_operand_direct(&mut self,
@@ -245,4 +228,30 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
245228
}), ty)
246229
}).collect()
247230
}
231+
232+
pub fn set_operand_dropped(&mut self,
233+
bcx: &BlockAndBuilder<'bcx, 'tcx>,
234+
operand: &mir::Operand<'tcx>) {
235+
match *operand {
236+
mir::Operand::Constant(_) => return,
237+
mir::Operand::Consume(ref lvalue) => {
238+
if let mir::Lvalue::Temp(idx) = *lvalue {
239+
if let TempRef::Operand(..) = self.temps[idx as usize] {
240+
// All lvalues which have an associated drop are promoted to an alloca
241+
// beforehand. If this is an operand, it is safe to say this is never
242+
// dropped and there’s no reason for us to zero this out at all.
243+
return
244+
}
245+
}
246+
let lvalue = self.trans_lvalue(bcx, lvalue);
247+
let ty = lvalue.ty.to_ty(bcx.tcx());
248+
if !glue::type_needs_drop(bcx.tcx(), ty) ||
249+
common::type_is_fat_ptr(bcx.tcx(), ty) {
250+
return
251+
} else {
252+
drop::drop_fill(bcx, lvalue.llval, ty);
253+
}
254+
}
255+
}
256+
}
248257
}

0 commit comments

Comments
 (0)