Skip to content
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

avoid rooting in some cases #15735

Merged
merged 1 commit into from
Apr 2, 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
4 changes: 2 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ static jl_cgval_t typed_load(Value *ptr, Value *idx_0based, jl_value_t *jltype,
static void typed_store(Value *ptr, Value *idx_0based, const jl_cgval_t &rhs,
jl_value_t *jltype, jl_codectx_t *ctx, MDNode *tbaa,
Value *parent, // for the write barrier, NULL if no barrier needed
size_t alignment = 0)
size_t alignment = 0, bool root_box = true) // if the value to store needs a box, should we root it ?
{
Type *elty = julia_type_to_llvm(jltype);
assert(elty != NULL);
Expand All @@ -807,7 +807,7 @@ static void typed_store(Value *ptr, Value *idx_0based, const jl_cgval_t &rhs,
r = emit_unbox(elty, rhs, jltype);
}
else {
r = boxed(rhs, ctx);
r = boxed(rhs, ctx, root_box);
if (parent != NULL) emit_write_barrier(ctx, parent, r);
}
Value *data;
Expand Down
8 changes: 5 additions & 3 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2434,7 +2434,7 @@ static bool emit_builtin_call(jl_cgval_t *ret, jl_value_t *f, jl_value_t **args,
emit_expr(args[2], ctx);
}
else {
jl_cgval_t v = (ety == (jl_value_t*)jl_any_type ? emit_expr(args[2], ctx) : emit_expr(args[2], ctx));
jl_cgval_t v = emit_expr(args[2], ctx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice optimization =)

PHINode *data_owner = NULL; // owner object against which the write barrier must check
if (isboxed) { // if not boxed we don't need a write barrier
assert(ary.isboxed);
Expand Down Expand Up @@ -2476,7 +2476,8 @@ static bool emit_builtin_call(jl_cgval_t *ret, jl_value_t *f, jl_value_t **args,
data_owner->addIncoming(own_ptr, ownedBB);
}
typed_store(emit_arrayptr(ary,args[1],ctx), idx, v,
ety, ctx, tbaa_user, data_owner);
ety, ctx, tbaa_user, data_owner, 0,
false); // don't need to root the box if we had to make one since it's being stored in the array immediatly
}
*ret = ary;
JL_GC_POP();
Expand Down Expand Up @@ -2976,7 +2977,8 @@ static jl_cgval_t emit_local(int sl, jl_codectx_t *ctx)
Value *bp = vi.memloc;
if (vi.isArgument || !vi.usedUndef) { // arguments are always defined
Instruction *v = builder.CreateLoad(bp, vi.isVolatile);
return mark_julia_type(v, true, vi.value.typ, ctx);
return mark_julia_type(v, true, vi.value.typ, ctx,
vi.isAssigned); // means it's an argument so don't need an additional root
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this affect unmodified unboxed arguments?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, memloc is a boxed root

}
else {
jl_cgval_t v = emit_checked_var(bp, sym, ctx, vi.isVolatile);
Expand Down