Skip to content

Commit 56671cb

Browse files
committed
Prefer GEP instructions over weird pointer casting
There are two places left where we used to only know the byte size of/offset into an array and had to cast to i8 and back to get the right addresses. But by now, we always know the sizes in terms of the number of elements in the array. In fact we have to add an extra Mul instruction so we can use the weird cast-to-u8 code. So we should really just embrace our new knowledge and use simple GEPs to do the address calculations. Additionally, the pointer calculations in bind_subslice_pat don't handle zero-sized types correctly, producing slices that point outside the array that is being matched against. Using GEP fixes that as well. Fixes #3729
1 parent e94a9f0 commit 56671cb

File tree

6 files changed

+25
-44
lines changed

6 files changed

+25
-44
lines changed

src/librustc_trans/trans/_match.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ use middle::pat_util::*;
201201
use trans::adt;
202202
use trans::base::*;
203203
use trans::build::{AddCase, And, BitCast, Br, CondBr, GEPi, InBoundsGEP, Load};
204-
use trans::build::{Mul, Not, Store, Sub, add_comment};
204+
use trans::build::{Not, Store, Sub, add_comment};
205205
use trans::build;
206206
use trans::callee;
207207
use trans::cleanup::{self, CleanupMethods};
@@ -630,8 +630,7 @@ fn bind_subslice_pat(bcx: Block,
630630
let vec_datum = match_datum(val, vec_ty);
631631
let (base, len) = vec_datum.get_vec_base_and_len(bcx);
632632

633-
let slice_byte_offset = Mul(bcx, vt.llunit_size, C_uint(bcx.ccx(), offset_left));
634-
let slice_begin = tvec::pointer_add_byte(bcx, base, slice_byte_offset);
633+
let slice_begin = InBoundsGEP(bcx, base, &[C_uint(bcx.ccx(), offset_left)]);
635634
let slice_len_offset = C_uint(bcx.ccx(), offset_left + offset_right);
636635
let slice_len = Sub(bcx, len, slice_len_offset);
637636
let slice_ty = ty::mk_slice(bcx.tcx(),

src/librustc_trans/trans/base.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -546,15 +546,6 @@ pub fn get_res_dtor<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
546546
}
547547
}
548548

549-
// Structural comparison: a rather involved form of glue.
550-
pub fn maybe_name_value(cx: &CrateContext, v: ValueRef, s: &str) {
551-
if cx.sess().opts.cg.save_temps {
552-
let buf = CString::from_slice(s.as_bytes());
553-
unsafe { llvm::LLVMSetValueName(v, buf.as_ptr()) }
554-
}
555-
}
556-
557-
558549
// Used only for creating scalar comparison glue.
559550
#[derive(Copy)]
560551
pub enum scalar_type { nil_type, signed_int, unsigned_int, floating_point, }

src/librustc_trans/trans/expr.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,6 @@ fn trans_index<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
768768
tvec::vec_types(bcx,
769769
ty::sequence_element_type(bcx.tcx(),
770770
base_datum.ty));
771-
base::maybe_name_value(bcx.ccx(), vt.llunit_size, "unit_sz");
772771

773772
let (base, len) = base_datum.get_vec_base_and_len(bcx);
774773

src/librustc_trans/trans/machine.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,6 @@ pub fn llsize_of(cx: &CrateContext, ty: Type) -> ValueRef {
8282
return C_uint(cx, llsize_of_alloc(cx, ty));
8383
}
8484

85-
// Returns the "default" size of t (see above), or 1 if the size would
86-
// be zero. This is important for things like vectors that expect
87-
// space to be consumed.
88-
pub fn nonzero_llsize_of(cx: &CrateContext, ty: Type) -> ValueRef {
89-
if llbitsize_of_real(cx, ty) == 0 {
90-
unsafe { llvm::LLVMConstInt(cx.int_type().to_ref(), 1, False) }
91-
} else {
92-
llsize_of(cx, ty)
93-
}
94-
}
95-
9685
// Returns the preferred alignment of the given type for the current target.
9786
// The preferred alignment may be larger than the alignment used when
9887
// packing the type into structs. This will be used for things like

src/librustc_trans/trans/tvec.rs

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use trans::expr::{Dest, Ignore, SaveIn};
2525
use trans::expr;
2626
use trans::glue;
2727
use trans::machine;
28-
use trans::machine::{nonzero_llsize_of, llsize_of_alloc};
28+
use trans::machine::llsize_of_alloc;
2929
use trans::type_::Type;
3030
use trans::type_of;
3131
use middle::ty::{self, Ty};
@@ -44,13 +44,6 @@ fn get_dataptr(bcx: Block, vptr: ValueRef) -> ValueRef {
4444
Load(bcx, expr::get_dataptr(bcx, vptr))
4545
}
4646

47-
pub fn pointer_add_byte(bcx: Block, ptr: ValueRef, bytes: ValueRef) -> ValueRef {
48-
let _icx = push_ctxt("tvec::pointer_add_byte");
49-
let old_ty = val_ty(ptr);
50-
let bptr = PointerCast(bcx, ptr, Type::i8p(bcx.ccx()));
51-
return PointerCast(bcx, InBoundsGEP(bcx, bptr, &[bytes]), old_ty);
52-
}
53-
5447
pub fn make_drop_glue_unboxed<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
5548
vptr: ValueRef,
5649
unit_ty: Ty<'tcx>,
@@ -94,17 +87,14 @@ pub fn make_drop_glue_unboxed<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
9487
pub struct VecTypes<'tcx> {
9588
pub unit_ty: Ty<'tcx>,
9689
pub llunit_ty: Type,
97-
pub llunit_size: ValueRef,
9890
pub llunit_alloc_size: u64
9991
}
10092

10193
impl<'tcx> VecTypes<'tcx> {
10294
pub fn to_string<'a>(&self, ccx: &CrateContext<'a, 'tcx>) -> String {
103-
format!("VecTypes {{unit_ty={}, llunit_ty={}, \
104-
llunit_size={}, llunit_alloc_size={}}}",
95+
format!("VecTypes {{unit_ty={}, llunit_ty={}, llunit_alloc_size={}}}",
10596
ty_to_string(ccx.tcx(), self.unit_ty),
10697
ccx.tn().type_to_string(self.llunit_ty),
107-
ccx.tn().val_to_string(self.llunit_size),
10898
self.llunit_alloc_size)
10999
}
110100
}
@@ -333,13 +323,11 @@ pub fn vec_types<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
333323
-> VecTypes<'tcx> {
334324
let ccx = bcx.ccx();
335325
let llunit_ty = type_of::type_of(ccx, unit_ty);
336-
let llunit_size = nonzero_llsize_of(ccx, llunit_ty);
337326
let llunit_alloc_size = llsize_of_alloc(ccx, llunit_ty);
338327

339328
VecTypes {
340329
unit_ty: unit_ty,
341330
llunit_ty: llunit_ty,
342-
llunit_size: llunit_size,
343331
llunit_alloc_size: llunit_alloc_size
344332
}
345333
}
@@ -486,17 +474,13 @@ pub fn iter_vec_raw<'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>,
486474
let fcx = bcx.fcx;
487475

488476
let vt = vec_types(bcx, unit_ty);
489-
let fill = Mul(bcx, len, vt.llunit_size);
490477

491478
if vt.llunit_alloc_size == 0 {
492479
// Special-case vectors with elements of size 0 so they don't go out of bounds (#9890)
493-
iter_vec_loop(bcx, data_ptr, &vt, fill, f)
480+
iter_vec_loop(bcx, data_ptr, &vt, len, f)
494481
} else {
495482
// Calculate the last pointer address we want to handle.
496-
// FIXME (#3729): Optimize this when the size of the unit type is
497-
// statically known to not use pointer casts, which tend to confuse
498-
// LLVM.
499-
let data_end_ptr = pointer_add_byte(bcx, data_ptr, fill);
483+
let data_end_ptr = InBoundsGEP(bcx, data_ptr, &[len]);
500484

501485
// Now perform the iteration.
502486
let header_bcx = fcx.new_temp_block("iter_vec_loop_header");
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright 2015 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+
fn main() {
12+
let x = [(), ()];
13+
14+
// The subslice used to go out of bounds for zero-sized array items, check that this doesn't
15+
// happen anymore
16+
match x {
17+
[_, y..] => assert_eq!(&x[1] as *const _, &y[0] as *const _)
18+
}
19+
}

0 commit comments

Comments
 (0)