From dd7a81d025802021d49e9a69016a890f421d1833 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Fri, 8 Feb 2013 01:52:47 -0800 Subject: [PATCH 1/2] Let llsize_of be a ConstantInt --- src/librustc/middle/trans/machine.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustc/middle/trans/machine.rs b/src/librustc/middle/trans/machine.rs index 828b561939055..dfbc75376c7cd 100644 --- a/src/librustc/middle/trans/machine.rs +++ b/src/librustc/middle/trans/machine.rs @@ -126,17 +126,16 @@ pub fn llbitsize_of_real(cx: @crate_ctxt, t: TypeRef) -> uint { } } -// Returns the "default" size of t, which is calculated by casting null to a -// *T and then doing gep(1) on it and measuring the result. Really, look in -// the LLVM sources. It does that. So this is likely similar to the ABI size -// (i.e. including alignment-padding), but goodness knows which alignment it -// winds up using. Probably the ABI one? Not recommended. +/// Returns the size of the type as an LLVM constant integer value. pub fn llsize_of(cx: @crate_ctxt, t: TypeRef) -> ValueRef { - unsafe { - return llvm::LLVMConstIntCast(lib::llvm::llvm::LLVMSizeOf(t), - cx.int_type, - False); - } + // Once upon a time, this called LLVMSizeOf, which does a + // getelementptr(1) on a null pointer and casts to an int, in + // order to obtain the type size as a value without requiring the + // target data layout. But we have the target data layout, so + // there's no need for that contrivance. The instruction + // selection DAG generator would flatten that GEP(1) node into a + // constant of the type's alloc size, so let's save it some work. + return C_uint(cx, llsize_of_alloc(cx, t)); } // Returns the "default" size of t (see above), or 1 if the size would From 9318babf6cf5c2d053d78dfb788575905226215e Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Fri, 8 Feb 2013 01:52:55 -0800 Subject: [PATCH 2/2] Fix const array index limit calculation. The number of operands of the LLVM node initializing the array underlying a const vector isn't always the array length -- if the array is of a sufficiently primitive type and all the elements' values are known (or something like that), LLVM uses a specialized Constant subclass that stores the data packed, and thus has no operands. Oops. But, because llsize_of now gives us a ConstantInt, we can just fix mozilla/rust#3169 and this all goes away. --- src/librustc/middle/trans/consts.rs | 33 ++--------------------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/src/librustc/middle/trans/consts.rs b/src/librustc/middle/trans/consts.rs index 5af62eda1fe4f..515239883e2d8 100644 --- a/src/librustc/middle/trans/consts.rs +++ b/src/librustc/middle/trans/consts.rs @@ -254,7 +254,7 @@ pub fn const_expr(cx: @crate_ctxt, e: @ast::expr) -> ValueRef { ~"index is not an integer-constant \ expression") }; - let (arr, _len) = match ty::get(bt).sty { + let (arr, len) = match ty::get(bt).sty { ty::ty_evec(_, vstore) | ty::ty_estr(vstore) => match vstore { ty::vstore_fixed(u) => @@ -278,36 +278,7 @@ pub fn const_expr(cx: @crate_ctxt, e: @ast::expr) -> ValueRef { a vector or string type") }; - // FIXME #3169: This is a little odd but it arises due to a - // weird wrinkle in LLVM: it doesn't appear willing to let us - // call LLVMConstIntGetZExtValue on the size element of the - // slice, or seemingly any integer-const involving a sizeof() - // call. Despite that being "a const", it's not the kind of - // const you can ask for the integer-value of, evidently. This - // might be an LLVM bug, not sure. In any case, to work around - // this we obtain the initializer and count how many elements it - // has, ignoring the length we pulled out of the slice. (Note - // that the initializer might be a struct rather than an array, - // if enums are involved.) This only works because we picked out - // the original globalvar via const_deref and so can recover the - // array-size of the underlying array (or the element count of - // the underlying struct), and all this will hold together - // exactly as long as we _don't_ support const sub-slices (that - // is, slices that represent something other than a whole - // array). At that point we'll have more and uglier work to do - // here, but for now this should work. - // - // In the future, what we should be doing here is the - // moral equivalent of: - // - // let len = llvm::LLVMConstIntGetZExtValue(len) as u64; - // - // but we might have to do substantially more magic to - // make it work. Or figure out what is causing LLVM to - // not want to consider sizeof() a constant expression - // we can get the value (as a number) out of. - - let len = llvm::LLVMGetNumOperands(arr) as u64; + let len = llvm::LLVMConstIntGetZExtValue(len) as u64; let len = match ty::get(bt).sty { ty::ty_estr(*) => {assert len > 0; len - 1}, _ => len