Skip to content

RFC: create an extra slot for arguments that get assigned #16410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 10 additions & 25 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const MAX_TUPLE_SPLAT = 16
const Slot_Assigned = 2
const Slot_AssignedOnce = 16
const Slot_UsedUndef = 32
const Slot_Called = 64

#### inference state types ####

Expand Down Expand Up @@ -2544,26 +2543,18 @@ function inlineable(f::ANY, ft::ANY, e::Expr, atypes::Vector{Any}, sv::Inference
end
end

islocal = false # if the argument name is also used as a local variable,
# we need to keep it as a variable name
if linfo.slotflags[i] & (Slot_Assigned | Slot_AssignedOnce) != 0
islocal = true
aeitype = tmerge(aeitype, linfo.slottypes[i])
end

# ok for argument to occur more than once if the actual argument
# is a symbol or constant, or is not affected by previous statements
# that will exist after the inlining pass finishes
if needtypeassert
vnew1 = unique_name(enclosing_ast, ast)
add_variable(enclosing_ast, vnew1, aeitype, !islocal)
add_variable(enclosing_ast, vnew1, aeitype, true)
v1 = (aeitype===Any ? vnew1 : SymbolNode(vnew1,aeitype))
push!(spvals, v1)
vnew2 = unique_name(enclosing_ast, ast)
v2 = (argtype===Any ? vnew2 : SymbolNode(vnew2,argtype))
unshift!(body.args, Expr(:(=), args_i, v2))
args[i] = args_i = vnew2
islocal = false
aeitype = argtype
affect_free = stmts_free
occ = 3
Expand All @@ -2578,7 +2569,7 @@ function inlineable(f::ANY, ft::ANY, e::Expr, atypes::Vector{Any}, sv::Inference
cond.typ = Bool
partmatch.args[1] = cond
else
affect_free = stmts_free && !islocal # false = previous statements might affect the result of evaluating argument
affect_free = stmts_free # false = previous statements might affect the result of evaluating argument
occ = 0
for j = length(body.args):-1:1
b = body.args[j]
Expand All @@ -2595,16 +2586,11 @@ function inlineable(f::ANY, ft::ANY, e::Expr, atypes::Vector{Any}, sv::Inference
end
end
free = effect_free(aei,sv,true)
if ((occ==0 && is(aeitype,Bottom)) || islocal || (occ > 1 && !inline_worthy(aei, occ*2000)) ||
if ((occ==0 && is(aeitype,Bottom)) || (occ > 1 && !inline_worthy(aei, occ*2000)) ||
(affect_free && !free) || (!affect_free && !effect_free(aei,sv,false)))
if occ != 0 # islocal=true is implied by occ!=0
if !islocal
vnew = newvar!(sv, aeitype)
argexprs[i] = vnew
else
vnew = add_slot!(enclosing, aeitype, #=SSA=#false)
argexprs[i] = vnew
end
if occ != 0
vnew = newvar!(sv, aeitype)
argexprs[i] = vnew
unshift!(prelude_stmts, Expr(:(=), vnew, aei))
stmts_free &= free
elseif !free && !isType(aeitype)
Expand Down Expand Up @@ -3033,8 +3019,6 @@ function is_known_call_p(e::Expr, pred, sv)
return isa(f,Const) && pred(f.val)
end

is_var_assigned(linfo, v) = isa(v,Slot) && linfo.slotflags[v.id]&Slot_Assigned != 0

function delete_var!(linfo, id, T)
filter!(x->!(isa(x,Expr) && (x.head === :(=) || x.head === :const) &&
isa(x.args[1],T) && x.args[1].id == id),
Expand Down Expand Up @@ -3064,16 +3048,17 @@ end
occurs_undef(var::Int, expr, flags) =
flags[var]&Slot_UsedUndef != 0 && occurs_more(expr, e->(isa(e,Slot) && e.id==var), 0)>0

# remove all single-assigned vars v in "v = x" where x is an argument
# and not assigned.
is_argument(linfo, v) = isa(v,Slot) && v.id <= linfo.nargs

# remove all single-assigned vars v in "v = x" where x is an argument.
# "sa" is the result of find_sa_vars
# T: Slot or SSAValue
function remove_redundant_temp_vars(linfo, sa, T)
flags = linfo.slotflags
ssavalue_types = linfo.ssavaluetypes
bexpr = Expr(:block); bexpr.args = linfo.code
for (v,init) in sa
if (isa(init, Slot) && !is_var_assigned(linfo, init::Slot))
if (isa(init, Slot) && is_argument(linfo, init::Slot))
# this transformation is not valid for vars used before def.
# we need to preserve the point of assignment to know where to
# throw errors (issue #4645).
Expand Down
34 changes: 13 additions & 21 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,6 @@ struct jl_varinfo_t {
#else
DIVariable dinfo;
#endif
bool isAssigned;
bool isSA;
bool isVolatile;
bool isArgument;
Expand All @@ -533,7 +532,7 @@ struct jl_varinfo_t {
#else
dinfo(DIVariable()),
#endif
isAssigned(true), isSA(false),
isSA(false),
isVolatile(false), isArgument(false),
escapes(true), usedUndef(false), used(false)
{
Expand Down Expand Up @@ -2504,7 +2503,7 @@ static bool emit_builtin_call(jl_cgval_t *ret, jl_value_t *f, jl_value_t **args,
}

else if (f==jl_builtin_nfields && nargs==1) {
if (ctx->vaStack && slot_eq(args[1], ctx->vaSlot) && !ctx->slots[ctx->vaSlot].isAssigned) {
if (ctx->vaStack && slot_eq(args[1], ctx->vaSlot)) {
*ret = mark_julia_type(emit_n_varargs(ctx), false, jl_long_type, ctx);
JL_GC_POP();
return true;
Expand Down Expand Up @@ -2912,10 +2911,6 @@ static jl_cgval_t emit_local(jl_value_t *slotload, jl_codectx_t *ctx)
size_t sl = jl_slot_number(slotload) - 1;
jl_varinfo_t &vi = ctx->slots[sl];
jl_sym_t *sym = slot_symbol(sl, ctx);
if (!vi.isArgument && !vi.isAssigned) {
undef_var_error_if_null(V_null, sym, ctx);
return jl_cgval_t();
}
if (vi.memloc) {
Value *bp = vi.memloc;
jl_value_t *typ;
Expand All @@ -2933,15 +2928,15 @@ static jl_cgval_t emit_local(jl_value_t *slotload, jl_codectx_t *ctx)
if (vi.isArgument || !vi.usedUndef) { // arguments are always defined
Instruction *v = builder.CreateLoad(bp, vi.isVolatile);
return mark_julia_type(v, true, typ, ctx,
vi.isAssigned); // means it's an argument so don't need an additional root
!vi.isArgument); // if an argument, doesn't need an additional root
}
else {
jl_cgval_t v = emit_checked_var(bp, sym, ctx, vi.isVolatile, nullptr);
v = remark_julia_type(v, typ); // patch up type, if possible
return v;
}
}
else if (!vi.isVolatile || !vi.isAssigned) {
else if (!vi.isVolatile || vi.isArgument) {
return vi.value;
}
else {
Expand Down Expand Up @@ -3025,7 +3020,6 @@ static void emit_assignment(jl_value_t *l, jl_value_t *r, jl_codectx_t *ctx)
assign_arrayvar(*av, rval_info, ctx);
}

assert(vi.isAssigned);
if (vi.memloc) {
// boxed variables
if (((!vi.isSA && rval_info.gcroot) || !rval_info.isboxed) && isa<AllocaInst>(vi.memloc)) {
Expand Down Expand Up @@ -4036,11 +4030,10 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
for(i=0; i < vinfoslen; i++) {
jl_varinfo_t &varinfo = ctx.slots[i];
uint8_t flags = jl_array_uint8_ref(lam->slotflags, i);
varinfo.isAssigned = (jl_vinfo_assigned(flags)!=0);
varinfo.escapes = false;
varinfo.isSA = (jl_vinfo_sa(flags)!=0);
varinfo.usedUndef = (jl_vinfo_usedundef(flags)!=0) || (!varinfo.isArgument && !lam->inferred);
if (!varinfo.isArgument || varinfo.isAssigned) {
if (!varinfo.isArgument) {
jl_value_t *typ = jl_is_array(lam->slottypes) ? jl_array_ptr_ref(lam->slottypes,i) : (jl_value_t*)jl_any_type;
if (!jl_is_type(typ))
typ = (jl_value_t*)jl_any_type;
Expand Down Expand Up @@ -4440,7 +4433,7 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
jl_sym_t *s = slot_symbol(i, &ctx);
if (s == unused_sym) continue;
jl_varinfo_t &varinfo = ctx.slots[i];
assert(!varinfo.memloc); // variables shouldn't also have memory locs already
assert(!varinfo.memloc); // variables shouldn't have memory locs already
if (!varinfo.usedUndef) {
if (varinfo.value.constant) {
// no need to explicitly load/store a constant/ghost value
Expand All @@ -4452,7 +4445,7 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
continue;
}
else if (store_unboxed_p(i, &ctx)) {
if (varinfo.isAssigned) { // otherwise, just leave it in the input register
if (!varinfo.isArgument) { // otherwise, just leave it in the input register
Value *lv = alloc_local(i, &ctx); (void)lv;
#ifdef LLVM36
if (ctx.debug_enabled) {
Expand All @@ -4468,8 +4461,8 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
continue;
}
}
if (varinfo.isAssigned || // always need a slot if the variable is assigned
specsig || // for arguments, give them stack slots if then aren't in `argArray` (otherwise, will use that pointer)
if (!varinfo.isArgument || // always need a slot if the variable is assigned
specsig || // for arguments, give them stack slots if they aren't in `argArray` (otherwise, will use that pointer)
(va && (int)i == ctx.vaSlot && varinfo.escapes) || // or it's the va arg tuple
(s != unused_sym && i == 0)) { // or it is the first argument (which isn't in `argArray`)
AllocaInst *av = new AllocaInst(T_pjlvalue, jl_symbol_name(s), /*InsertBefore*/ctx.ptlsStates);
Expand Down Expand Up @@ -4557,15 +4550,14 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
if (vi.memloc == NULL) {
if (vi.value.V) {
// copy theArg into its local variable slot (unboxed)
assert(vi.isAssigned && vi.value.ispointer());
assert(vi.value.ispointer());
tbaa_decorate(vi.value.tbaa,
builder.CreateStore(emit_unbox(vi.value.V->getType()->getContainedType(0),
theArg, vi.value.typ),
vi.value.V));
}
else {
// keep track of original (possibly boxed) value to avoid re-boxing or moving
assert(!vi.isAssigned || vi.value.constant);
vi.value = theArg;
#ifdef LLVM36
if (specsig && theArg.V && ctx.debug_enabled) {
Expand All @@ -4579,9 +4571,9 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
}
ctx.dbuilder->insertDeclare(parg, vi.dinfo, ctx.dbuilder->createExpression(addr),
#ifdef LLVM37
builder.getCurrentDebugLocation(),
builder.getCurrentDebugLocation(),
#endif
builder.GetInsertBlock());
builder.GetInsertBlock());
}
#endif
}
Expand All @@ -4603,7 +4595,7 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
// step 11. allocate rest argument if necessary
if (va && ctx.vaSlot != -1) {
jl_varinfo_t &vi = ctx.slots[ctx.vaSlot];
if (!vi.escapes && !vi.isAssigned) {
if (!vi.escapes) {
ctx.vaStack = true;
}
else if (!vi.value.constant) {
Expand Down
52 changes: 37 additions & 15 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -2950,6 +2950,7 @@ f(x) = yt(x)
(filename #f)
(first-line #t)
(rett #f)
(arg-map #f) ;; map arguments to new names if they are assigned
(label-counter 0) ;; counter for generating label addresses
(label-map (table)) ;; maps label names to generated addresses
(label-level (table)) ;; exception handler level of each label
Expand Down Expand Up @@ -2979,8 +2980,8 @@ f(x) = yt(x)
(if rett
(emit `(return ,(convert-for-type-decl rv rett)))
(emit `(return ,rv)))))
(define (new-mutable-var)
(let ((g (gensy)))
(define (new-mutable-var . name)
(let ((g (if (null? name) (gensy) (named-gensy (car name)))))
(set-car! (lam:vinfo lam) (append (car (lam:vinfo lam)) `((,g Any 2))))
g))
;; evaluate the arguments of a call, creating temporary locations as needed
Expand Down Expand Up @@ -3033,11 +3034,14 @@ f(x) = yt(x)
(define (compile e break-labels value tail)
(if (or (not (pair? e)) (memq (car e) '(null ssavalue quote inert top core copyast the_exception $
globalref cdecl stdcall fastcall thiscall)))
(cond (tail (emit-return e))
(value e)
((or (eq? e 'true) (eq? e 'false)) #f)
((symbol? e) (emit e) #f) ;; keep symbols for undefined-var checking
(else #f))
(let ((e (if (and arg-map (symbol? e))
(get arg-map e e)
e)))
(cond (tail (emit-return e))
(value e)
((or (eq? e 'true) (eq? e 'false)) #f)
((symbol? e) (emit e) #f) ;; keep symbols for undefined-var checking
(else #f)))
(case (car e)
((call new)
(let* ((ccall? (and (eq? (car e) 'call) (equal? (cadr e) '(core ccall))))
Expand All @@ -3052,16 +3056,20 @@ f(x) = yt(x)
((eq? (car e) 'new) #f)
(else (emit callex)))))
((=)
(let ((rhs (compile (caddr e) break-labels #t #f)))
(let* ((rhs (compile (caddr e) break-labels #t #f))
(lhs (cadr e))
(lhs (if (and arg-map (symbol? lhs))
(get arg-map lhs lhs)
lhs)))
(if value
(let ((rr (if (or (atom? rhs) (ssavalue? rhs) (eq? (car rhs) 'null))
rhs (make-ssavalue))))
(if (not (eq? rr rhs))
(emit `(= ,rr ,rhs)))
(emit `(= ,(cadr e) ,rr))
(emit `(= ,lhs ,rr))
(if tail (emit-return rr))
rr)
(emit `(= ,(cadr e) ,rhs)))))
(emit `(= ,lhs ,rhs)))))
((block body)
(let* ((last-fname filename)
(fnm (first-non-meta e))
Expand Down Expand Up @@ -3256,6 +3264,14 @@ f(x) = yt(x)
(error "\"...\" expression outside call"))
(else
(error (string "unhandled expr " e))))))
;; introduce new slots for assigned arguments
(for-each (lambda (v)
(if (vinfo:asgn v)
(begin
(if (not arg-map)
(set! arg-map (table)))
(put! arg-map (car v) (new-mutable-var (car v))))))
(list-head vi (length (lam:args lam))))
(compile e '() #t #t)
(for-each (lambda (x)
(let ((point (car x))
Expand All @@ -3272,11 +3288,17 @@ f(x) = yt(x)
(set-car! (cdr point) `(leave ,(- hl target-level))))))))
handler-goto-fixups)
(let* ((stmts (reverse! code))
(di (definitely-initialized-vars stmts vi)))
(cons 'body (filter (lambda (e)
(not (and (pair? e) (eq? (car e) 'newvar)
(has? di (cadr e)))))
stmts)))))
(di (definitely-initialized-vars stmts vi))
(body (cons 'body (filter (lambda (e)
(not (and (pair? e) (eq? (car e) 'newvar)
(has? di (cadr e)))))
stmts))))
(if arg-map
(insert-after-meta
body
(table.foldl (lambda (k v lst) (cons `(= ,v ,k) lst))
'() arg-map))
body))))

;; find newvar nodes that are unnecessary because (1) the variable is not
;; captured, and (2) the variable is assigned before any branches.
Expand Down
5 changes: 0 additions & 5 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1361,11 +1361,6 @@ JL_DLLEXPORT jl_array_t *jl_uncompress_ast(jl_lambda_info_t *li, jl_array_t *dat
JL_DLLEXPORT int jl_is_operator(char *sym);
JL_DLLEXPORT int jl_operator_precedence(char *sym);

STATIC_INLINE int jl_vinfo_assigned(uint8_t vi)
{
return (vi&2)!=0;
}

STATIC_INLINE int jl_vinfo_sa(uint8_t vi)
{
return (vi&16)!=0;
Expand Down