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

Fix #10208 - Unnecessary boxing to call jl_object_id #31771

Merged
merged 1 commit into from
Apr 22, 2019
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/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ typedef struct _varidx {
struct _varidx *prev;
} jl_varidx_t;

static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;

static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOINT
{
Expand Down Expand Up @@ -277,7 +277,7 @@ static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOIN
return jl_object_id_((jl_value_t*)tv, v);
}

static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT
JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT
{
if (tv == (jl_value_t*)jl_sym_type)
return ((jl_sym_t*)v)->hash;
Expand Down
21 changes: 21 additions & 0 deletions src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1713,6 +1713,27 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
JL_GC_POP();
return ghostValue(jl_void_type);
}
else if (is_libjulia_func(jl_object_id) && nargt == 1 &&
rt == (jl_value_t*)jl_ulong_type) {
jl_cgval_t val = argv[0];
if (!val.isboxed) {
// If the value is not boxed, try to compute the object id without
// reboxing it.
auto T_pint8_derived = PointerType::get(T_int8, AddressSpace::Derived);
if (!val.isghost && !val.ispointer())
val = value_to_pointer(ctx, val);
Value *args[] = {
emit_typeof_boxed(ctx, val),
val.isghost ? ConstantPointerNull::get(T_pint8_derived) :
ctx.builder.CreateBitCast(
decay_derived(data_pointer(ctx, val)),
Copy link
Member

Choose a reason for hiding this comment

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

Only .ispointer values have pointers (otherwise we just return garbage from data_pointer)

T_pint8_derived)
};
Value *ret = ctx.builder.CreateCall(prepare_call(jl_object_id__func), makeArrayRef(args));
JL_GC_POP();
return mark_or_box_ccall_result(ctx, ret, retboxed, rt, unionall, static_rt);
}
}

jl_cgval_t retval = sig.emit_a_ccall(
ctx,
Expand Down
1 change: 1 addition & 0 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,7 @@ static Value *julia_bool(jl_codectx_t &ctx, Value *cond)
static Constant *julia_const_to_llvm(jl_value_t *e);
static Value *data_pointer(jl_codectx_t &ctx, const jl_cgval_t &x)
{
assert(x.ispointer());
Value *data = x.V;
if (x.constant) {
Constant *val = julia_const_to_llvm(x.constant);
Expand Down
10 changes: 10 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ static Function *jl_write_barrier_func;
static Function *jlisa_func;
static Function *jlsubtype_func;
static Function *jlapplytype_func;
static Function *jl_object_id__func;
static Function *setjmp_func;
static Function *memcmp_derived_func;
static Function *box_int8_func;
Expand Down Expand Up @@ -7434,6 +7435,15 @@ static void init_julia_llvm_env(Module *m)
add_return_attr(jlapplytype_func, Attribute::NonNull);
add_named_global(jlapplytype_func, &jl_instantiate_type_in_env);

std::vector<Type *> objectid__args(0);
objectid__args.push_back(T_prjlvalue);
objectid__args.push_back(T_pint8_derived);
jl_object_id__func =
Function::Create(FunctionType::get(T_size, objectid__args, false),
Function::ExternalLinkage,
"jl_object_id_", m);
add_named_global(jl_object_id__func, &jl_object_id_);

std::vector<Type*> gc_alloc_args(0);
gc_alloc_args.push_back(T_pint8);
gc_alloc_args.push_back(T_size);
Expand Down
2 changes: 2 additions & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,8 @@ JL_DLLEXPORT int jl_array_isassigned(jl_array_t *a, size_t i);

JL_DLLEXPORT void jl_uv_stop(uv_loop_t* loop);

JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;

// -- synchronization utilities -- //

extern jl_mutex_t typecache_lock;
Expand Down
24 changes: 24 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -364,3 +364,27 @@ str = String(take!(io))
@test occursin("alias.scope", str)
@test occursin("aliasscope", str)
@test occursin("noalias", str)

# Issue #10208 - Unnecessary boxing for calling objectid
struct FooDictHash{T}
x::T
end

function f_dict_hash_alloc()
d = Dict{FooDictHash{Int},Int}()
for i in 1:10000
d[FooDictHash(i)] = i+1
end
d
end

function g_dict_hash_alloc()
d = Dict{Int,Int}()
for i in 1:10000
d[i] = i+1
end
d
end
# Warm up
f_dict_hash_alloc(); g_dict_hash_alloc();
@test (@allocated f_dict_hash_alloc()) == (@allocated g_dict_hash_alloc())