From 7febdb7b15681a6a70da191128a9552f35ed5644 Mon Sep 17 00:00:00 2001 From: Corey Richardson Date: Tue, 18 Mar 2014 09:46:43 -0400 Subject: [PATCH 1/3] rustc: mark references w/anonymous lifetime nocapture Closes #6751 --- src/librustc/middle/trans/base.rs | 13 +++++++++- src/librustc/middle/trans/callee.rs | 39 +++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index b5e64972115..fee15230575 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -246,6 +246,7 @@ fn get_extern_rust_fn(ccx: &CrateContext, inputs: &[ty::t], output: ty::t, pub fn decl_rust_fn(ccx: &CrateContext, has_env: bool, inputs: &[ty::t], output: ty::t, name: &str) -> ValueRef { + use middle::ty::{FreeRegion, BrAnon, ReFree, ReLateBound}; let llfty = type_of_rust_fn(ccx, has_env, inputs, output); let llfn = decl_cdecl_fn(ccx.llmod, name, llfty, output); @@ -265,7 +266,17 @@ pub fn decl_rust_fn(ccx: &CrateContext, has_env: bool, unsafe { llvm::LLVMAddAttribute(llarg, lib::llvm::NoAliasAttribute as c_uint); } - } + }, + // When a reference in an argument has no named lifetime, it's + // impossible for that reference to escape this function(ie, be + // returned). + ty::ty_rptr(ReFree(FreeRegion { scope_id: _, bound_region: BrAnon(_) }), _) | + ty::ty_rptr(ReLateBound(_, BrAnon(_)), _) => { + debug!("marking argument of {} as nocapture because of anonymous lifetime", name); + unsafe { + llvm::LLVMAddAttribute(llarg, lib::llvm::NoCaptureAttribute as c_uint); + } + }, _ => { // For non-immediate arguments the callee gets its own copy of // the value on the stack, so there are no aliases diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 92b97cdfc9f..30f272d16b0 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -20,7 +20,7 @@ use std::slice; use back::abi; use driver::session; -use lib::llvm::{ValueRef, NoAliasAttribute, StructRetAttribute}; +use lib::llvm::{ValueRef, NoAliasAttribute, StructRetAttribute, NoCaptureAttribute}; use lib::llvm::llvm; use metadata::csearch; use middle::trans::base; @@ -661,9 +661,15 @@ pub fn trans_call_inner<'a>( llargs.push(opt_llretslot.unwrap()); } + // start at 1, because index 0 is the return value of the llvm func + let mut first_arg_offset = 1; + // Push the environment (or a trait object's self). match (llenv, llself) { - (Some(llenv), None) => llargs.push(llenv), + (Some(llenv), None) => { + first_arg_offset += 1; + llargs.push(llenv) + }, (None, Some(llself)) => llargs.push(llself), _ => {} } @@ -682,6 +688,11 @@ pub fn trans_call_inner<'a>( let mut attrs = Vec::new(); if type_of::return_uses_outptr(ccx, ret_ty) { attrs.push((1, StructRetAttribute)); + // The outptr can be noalias and nocapture because it's entirely + // invisible to the program. + attrs.push((1, NoAliasAttribute)); + attrs.push((1, NoCaptureAttribute)); + first_arg_offset += 1; } // The `noalias` attribute on the return value is useful to a @@ -695,6 +706,30 @@ pub fn trans_call_inner<'a>( _ => {} } + debug!("trans_callee_inner: first_arg_offset={}", first_arg_offset); + + for (idx, &t) in ty::ty_fn_args(callee_ty).iter().enumerate().map(|(i, v)| (i+first_arg_offset, v)) { + use middle::ty::{FreeRegion, BrAnon, ReFree, ReLateBound}; + if !type_is_immediate(ccx, t) { + // if it's not immediate, we have a program-invisible pointer, + // which it can't possibly capture + attrs.push((idx, NoCaptureAttribute)); + debug!("trans_callee_inner: argument {} nocapture because it's non-immediate", idx); + continue; + } + + let t_ = ty::get(t); + match t_.sty { + ty::ty_rptr(ReFree(FreeRegion { scope_id: _, bound_region: BrAnon(_) }), _) | + ty::ty_rptr(ReLateBound(_, BrAnon(_)), _) => { + + debug!("trans_callee_inner: argument {} nocapture because of anonymous lifetime", idx); + attrs.push((idx, NoCaptureAttribute)); + }, + _ => { } + } + } + // Invoke the actual rust fn and update bcx/llresult. let (llret, b) = base::invoke(bcx, llfn, From b8ed13686ad1f883f3de1e7fa5ba5bf9f92ae5d2 Mon Sep 17 00:00:00 2001 From: Corey Richardson Date: Mon, 24 Mar 2014 12:38:23 -0400 Subject: [PATCH 2/3] Address review --- src/librustc/middle/trans/base.rs | 6 +++--- src/librustc/middle/trans/callee.rs | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index fee15230575..a81301f4f3e 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -246,7 +246,8 @@ fn get_extern_rust_fn(ccx: &CrateContext, inputs: &[ty::t], output: ty::t, pub fn decl_rust_fn(ccx: &CrateContext, has_env: bool, inputs: &[ty::t], output: ty::t, name: &str) -> ValueRef { - use middle::ty::{FreeRegion, BrAnon, ReFree, ReLateBound}; + use middle::ty::{BrAnon, ReLateBound}; + let llfty = type_of_rust_fn(ccx, has_env, inputs, output); let llfn = decl_cdecl_fn(ccx.llmod, name, llfty, output); @@ -270,8 +271,7 @@ pub fn decl_rust_fn(ccx: &CrateContext, has_env: bool, // When a reference in an argument has no named lifetime, it's // impossible for that reference to escape this function(ie, be // returned). - ty::ty_rptr(ReFree(FreeRegion { scope_id: _, bound_region: BrAnon(_) }), _) | - ty::ty_rptr(ReLateBound(_, BrAnon(_)), _) => { + ty::ty_rptr(ReLateBound(_, BrAnon(_)), _) => { debug!("marking argument of {} as nocapture because of anonymous lifetime", name); unsafe { llvm::LLVMAddAttribute(llarg, lib::llvm::NoCaptureAttribute as c_uint); diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 30f272d16b0..ca751c11bda 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -708,8 +708,9 @@ pub fn trans_call_inner<'a>( debug!("trans_callee_inner: first_arg_offset={}", first_arg_offset); - for (idx, &t) in ty::ty_fn_args(callee_ty).iter().enumerate().map(|(i, v)| (i+first_arg_offset, v)) { - use middle::ty::{FreeRegion, BrAnon, ReFree, ReLateBound}; + for (idx, &t) in ty::ty_fn_args(callee_ty).iter().enumerate() + .map(|(i, v)| (i+first_arg_offset, v)) { + use middle::ty::{BrAnon, ReLateBound}; if !type_is_immediate(ccx, t) { // if it's not immediate, we have a program-invisible pointer, // which it can't possibly capture @@ -720,10 +721,9 @@ pub fn trans_call_inner<'a>( let t_ = ty::get(t); match t_.sty { - ty::ty_rptr(ReFree(FreeRegion { scope_id: _, bound_region: BrAnon(_) }), _) | - ty::ty_rptr(ReLateBound(_, BrAnon(_)), _) => { - - debug!("trans_callee_inner: argument {} nocapture because of anonymous lifetime", idx); + ty::ty_rptr(ReLateBound(_, BrAnon(_)), _) => { + debug!("trans_callee_inner: argument {} nocapture because \ + of anonymous lifetime", idx); attrs.push((idx, NoCaptureAttribute)); }, _ => { } From 5258e13d0b5ed1fb201579115d618728ab0b69c0 Mon Sep 17 00:00:00 2001 From: Corey Richardson Date: Thu, 27 Mar 2014 07:54:41 -0400 Subject: [PATCH 3/3] test/run-pass/out-of-stack: prevent tco We really do *not* want TCO to kick in. If it does, we'll never blow the stack, and never trigger the condition the test is checking for. To that end, do a meaningless alloc that serves only to get a destructor to run. The addition of nocapture/noalias seems to have let LLVM do more TCO, which hurt this testcase. --- src/test/run-pass/out-of-stack.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/run-pass/out-of-stack.rs b/src/test/run-pass/out-of-stack.rs index 9344c29d5ca..d1daa1e365e 100644 --- a/src/test/run-pass/out-of-stack.rs +++ b/src/test/run-pass/out-of-stack.rs @@ -28,6 +28,7 @@ fn silent_recurse() { fn loud_recurse() { println!("hello!"); loud_recurse(); + black_box(()); // don't optimize this into a tail call. please. } fn main() {