From adeb9a78660ed76943e682f223184d93f8e8238b Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 28 Jul 2014 21:26:21 -0700 Subject: [PATCH 1/4] rustrt: Make begin_unwind take a single file/line pointer Smaller text size. --- src/librustrt/unwind.rs | 17 ++++++++++++++++- src/libstd/macros.rs | 33 +++++++++++++++++++++++++++++++++ src/libsyntax/ext/build.rs | 13 ++++++++----- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/librustrt/unwind.rs b/src/librustrt/unwind.rs index 5dfeb15afb84a..344d3a0f103d3 100644 --- a/src/librustrt/unwind.rs +++ b/src/librustrt/unwind.rs @@ -417,8 +417,8 @@ pub fn begin_unwind_fmt(msg: &fmt::Arguments, file_line: &(&'static str, uint)) begin_unwind_inner(box String::from_utf8(v).unwrap(), file_line) } -// FIXME: Need to change expr_fail in AstBuilder to change this to &(str, uint) /// This is the entry point of unwinding for fail!() and assert!(). +#[cfg(stage0)] #[inline(never)] #[cold] // avoid code bloat at the call sites as much as possible pub fn begin_unwind(msg: M, file: &'static str, line: uint) -> ! { // Note that this should be the only allocation performed in this code path. @@ -432,6 +432,21 @@ pub fn begin_unwind(msg: M, file: &'static str, line: uint) -> ! begin_unwind_inner(box msg, &(file, line)) } +/// This is the entry point of unwinding for fail!() and assert!(). +#[cfg(not(stage0))] +#[inline(never)] #[cold] // avoid code bloat at the call sites as much as possible +pub fn begin_unwind(msg: M, file_line: &(&'static str, uint)) -> ! { + // Note that this should be the only allocation performed in this code path. + // Currently this means that fail!() on OOM will invoke this code path, + // but then again we're not really ready for failing on OOM anyway. If + // we do start doing this, then we should propagate this allocation to + // be performed in the parent of this task instead of the task that's + // failing. + + // see below for why we do the `Any` coercion here. + begin_unwind_inner(box msg, file_line) +} + /// The core of the unwinding. /// /// This is non-generic to avoid instantiation bloat in other crates diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index f0732c7d508e8..e67329df7aec4 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -37,6 +37,39 @@ /// fail!("this is a {} {message}", "fancy", message = "message"); /// ``` #[macro_export] +#[cfg(not(stage0))] +macro_rules! fail( + () => ({ + fail!("explicit failure") + }); + ($msg:expr) => ({ + // static requires less code at runtime, more constant data + static FILE_LINE: (&'static str, uint) = (file!(), line!()); + ::std::rt::begin_unwind($msg, &FILE_LINE) + }); + ($fmt:expr, $($arg:tt)*) => ({ + // a closure can't have return type !, so we need a full + // function to pass to format_args!, *and* we need the + // file and line numbers right here; so an inner bare fn + // is our only choice. + // + // LLVM doesn't tend to inline this, presumably because begin_unwind_fmt + // is #[cold] and #[inline(never)] and because this is flagged as cold + // as returning !. We really do want this to be inlined, however, + // because it's just a tiny wrapper. Small wins (156K to 149K in size) + // were seen when forcing this to be inlined, and that number just goes + // up with the number of calls to fail!() + #[inline(always)] + fn run_fmt(fmt: &::std::fmt::Arguments) -> ! { + static FILE_LINE: (&'static str, uint) = (file!(), line!()); + ::std::rt::begin_unwind_fmt(fmt, &FILE_LINE) + } + format_args!(run_fmt, $fmt, $($arg)*) + }); +) + +#[macro_export] +#[cfg(stage0)] macro_rules! fail( () => ({ fail!("explicit failure") diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 7d683382589b9..1272db18063a0 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -682,6 +682,13 @@ impl<'a> AstBuilder for ExtCtxt<'a> { fn expr_fail(&self, span: Span, msg: InternedString) -> Gc { let loc = self.codemap().lookup_char_pos(span.lo); + let expr_file = self.expr_str(span, + token::intern_and_get_ident(loc.file + .name + .as_slice())); + let expr_line = self.expr_uint(span, loc.line); + let expr_file_line_tuple = self.expr_tuple(span, vec!(expr_file, expr_line)); + let expr_file_line_ptr = self.expr_addr_of(span, expr_file_line_tuple); self.expr_call_global( span, vec!( @@ -690,11 +697,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { self.ident_of("begin_unwind")), vec!( self.expr_str(span, msg), - self.expr_str(span, - token::intern_and_get_ident(loc.file - .name - .as_slice())), - self.expr_uint(span, loc.line))) + expr_file_line_ptr)) } fn expr_unreachable(&self, span: Span) -> Gc { From 55c14d293b23ed1250927e526416f15e5ff33725 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 29 Jul 2014 16:40:59 -0700 Subject: [PATCH 2/4] Modify failure lang items to take less pointers. Divide-by-zero before: ``` leaq "str\"str\"(1762)"(%rip), %rax movq %rax, 16(%rsp) movq $27, 24(%rsp) leaq "str\"str\"(1542)"(%rip), %rax movq %rax, (%rsp) movq $19, 8(%rsp) leaq 16(%rsp), %rdi leaq (%rsp), %rsi movl $32, %edx callq _ZN7failure5fail_20hc04408f955ce60aaqWjE@PLT ``` After: ``` leaq .Lconst(%rip), %rdi callq _ZN7failure5fail_20haf918a97c8f7f2bfqWjE@PLT ``` Bounds check before: ``` leaq "str\"str\"(1542)"(%rip), %rax movq %rax, 8(%rsp) movq $19, 16(%rsp) leaq 8(%rsp), %rdi movl $38, %esi movl $1, %edx movl $1, %ecx callq _ZN7failure17fail_bounds_check20hf4bc3c69e96caf41RXjE@PLT ``` Bounds check after: ``` leaq .Lconst2(%rip), %rdi movl $1, %esi movl $1, %edx callq _ZN7failure17fail_bounds_check20h5267276a537a7de22XjE@PLT ``` Size before: 21277995 librustc-4e7c5e5c.s ``` text data 12554881 6089335 ``` Size after: 21247617 librustc-4e7c5e5c.so ``` text data 12518497 6095748 ``` --- src/libcore/failure.rs | 26 ++++++++++++++++++++++ src/librustc/middle/trans/consts.rs | 2 +- src/librustc/middle/trans/controlflow.rs | 28 +++++++++++------------- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/libcore/failure.rs b/src/libcore/failure.rs index 5965603770ba7..6a638b5618ca6 100644 --- a/src/libcore/failure.rs +++ b/src/libcore/failure.rs @@ -33,6 +33,7 @@ use fmt; use intrinsics; +#[cfg(stage0)] #[cold] #[inline(never)] // this is the slow path, always #[lang="fail_"] fn fail_(expr: &'static str, file: &'static str, line: uint) -> ! { @@ -43,6 +44,7 @@ fn fail_(expr: &'static str, file: &'static str, line: uint) -> ! { unsafe { intrinsics::abort() } } +#[cfg(stage0)] #[cold] #[lang="fail_bounds_check"] fn fail_bounds_check(file: &'static str, line: uint, @@ -53,6 +55,30 @@ fn fail_bounds_check(file: &'static str, line: uint, unsafe { intrinsics::abort() } } +#[cfg(not(stage0))] +#[cold] #[inline(never)] // this is the slow path, always +#[lang="fail_"] +fn fail_(expr_file_line: &(&'static str, &'static str, uint)) -> ! { + let (expr, file, line) = *expr_file_line; + let ref file_line = (file, line); + format_args!(|args| -> () { + begin_unwind(args, file_line); + }, "{}", expr); + + unsafe { intrinsics::abort() } +} + +#[cfg(not(stage0))] +#[cold] +#[lang="fail_bounds_check"] +fn fail_bounds_check(file_line: &(&'static str, uint), + index: uint, len: uint) -> ! { + format_args!(|args| -> () { + begin_unwind(args, file_line); + }, "index out of bounds: the len is {} but the index is {}", len, index); + unsafe { intrinsics::abort() } +} + #[cold] pub fn begin_unwind(fmt: &fmt::Arguments, file_line: &(&'static str, uint)) -> ! { #[allow(ctypes)] diff --git a/src/librustc/middle/trans/consts.rs b/src/librustc/middle/trans/consts.rs index 2fd468d8fda8f..7d546aac0cbae 100644 --- a/src/librustc/middle/trans/consts.rs +++ b/src/librustc/middle/trans/consts.rs @@ -109,7 +109,7 @@ fn const_vec(cx: &CrateContext, e: &ast::Expr, (v, llunitty, inlineable.iter().fold(true, |a, &b| a && b)) } -fn const_addr_of(cx: &CrateContext, cv: ValueRef) -> ValueRef { +pub fn const_addr_of(cx: &CrateContext, cv: ValueRef) -> ValueRef { unsafe { let gv = "const".with_c_str(|name| { llvm::LLVMAddGlobal(cx.llmod, val_ty(cv).to_ref(), name) diff --git a/src/librustc/middle/trans/controlflow.rs b/src/librustc/middle/trans/controlflow.rs index d8a8cc1c561a9..f36c51f552623 100644 --- a/src/librustc/middle/trans/controlflow.rs +++ b/src/librustc/middle/trans/controlflow.rs @@ -20,6 +20,7 @@ use middle::trans::callee; use middle::trans::cleanup::CleanupMethods; use middle::trans::cleanup; use middle::trans::common::*; +use middle::trans::consts; use middle::trans::datum; use middle::trans::debuginfo; use middle::trans::expr; @@ -472,14 +473,6 @@ pub fn trans_ret<'a>(bcx: &'a Block<'a>, return bcx; } -fn str_slice_arg<'a>(bcx: &'a Block<'a>, s: InternedString) -> ValueRef { - let ccx = bcx.ccx(); - let s = C_str_slice(ccx, s); - let slot = alloca(bcx, val_ty(s), "__temp"); - Store(bcx, s, slot); - slot -} - pub fn trans_fail<'a>( bcx: &'a Block<'a>, sp: Span, @@ -488,12 +481,14 @@ pub fn trans_fail<'a>( let ccx = bcx.ccx(); let _icx = push_ctxt("trans_fail_value"); - let v_str = str_slice_arg(bcx, fail_str); + let v_str = C_str_slice(ccx, fail_str); let loc = bcx.sess().codemap().lookup_char_pos(sp.lo); let filename = token::intern_and_get_ident(loc.file.name.as_slice()); - let v_filename = str_slice_arg(bcx, filename); - let v_line = loc.line as int; - let args = vec!(v_str, v_filename, C_int(ccx, v_line)); + let filename = C_str_slice(ccx, filename); + let line = C_int(ccx, loc.line as int); + let expr_file_line_const = C_struct(ccx, &[v_str, filename, line], false); + let expr_file_line = consts::const_addr_of(ccx, expr_file_line_const); + let args = vec!(expr_file_line); let did = langcall(bcx, Some(sp), "", FailFnLangItem); let bcx = callee::trans_lang_call(bcx, did, @@ -509,6 +504,7 @@ pub fn trans_fail_bounds_check<'a>( index: ValueRef, len: ValueRef) -> &'a Block<'a> { + let ccx = bcx.ccx(); let _icx = push_ctxt("trans_fail_bounds_check"); // Extract the file/line from the span @@ -516,9 +512,11 @@ pub fn trans_fail_bounds_check<'a>( let filename = token::intern_and_get_ident(loc.file.name.as_slice()); // Invoke the lang item - let filename = str_slice_arg(bcx, filename); - let line = C_int(bcx.ccx(), loc.line as int); - let args = vec!(filename, line, index, len); + let filename = C_str_slice(ccx, filename); + let line = C_int(ccx, loc.line as int); + let file_line_const = C_struct(ccx, &[filename, line], false); + let file_line = consts::const_addr_of(ccx, file_line_const); + let args = vec!(file_line, index, len); let did = langcall(bcx, Some(sp), "", FailBoundsCheckFnLangItem); let bcx = callee::trans_lang_call(bcx, did, From 3b740d8203755e2cc1358e6e7947d7a4a9747438 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 29 Jul 2014 23:18:31 -0700 Subject: [PATCH 3/4] core: Add #[inline(never)] to failure functions For consistency, just because `fail_` has it. --- src/libcore/failure.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/failure.rs b/src/libcore/failure.rs index 6a638b5618ca6..543d3f2cbbbe1 100644 --- a/src/libcore/failure.rs +++ b/src/libcore/failure.rs @@ -69,7 +69,7 @@ fn fail_(expr_file_line: &(&'static str, &'static str, uint)) -> ! { } #[cfg(not(stage0))] -#[cold] +#[cold] #[inline(never)] #[lang="fail_bounds_check"] fn fail_bounds_check(file_line: &(&'static str, uint), index: uint, len: uint) -> ! { @@ -79,7 +79,7 @@ fn fail_bounds_check(file_line: &(&'static str, uint), unsafe { intrinsics::abort() } } -#[cold] +#[cold] #[inline(never)] pub fn begin_unwind(fmt: &fmt::Arguments, file_line: &(&'static str, uint)) -> ! { #[allow(ctypes)] extern { From b775bebdf299f2c92f7a12f74ff11d5f961b7bd2 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 29 Jul 2014 23:19:36 -0700 Subject: [PATCH 4/4] core: Fix failure doc comment --- src/libcore/failure.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/failure.rs b/src/libcore/failure.rs index 543d3f2cbbbe1..f5cfa2611d5e6 100644 --- a/src/libcore/failure.rs +++ b/src/libcore/failure.rs @@ -16,7 +16,7 @@ //! interface for failure is: //! //! ```ignore -//! fn begin_unwind(fmt: &fmt::Arguments, file: &str, line: uint) -> !; +//! fn begin_unwind(fmt: &fmt::Arguments, &(&'static str, uint)) -> !; //! ``` //! //! This definition allows for failing with any general message, but it does not