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

Optimize getfield lowering to avoid boxing in some cases #50444

Merged
merged 5 commits into from
Jul 7, 2023
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
39 changes: 39 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,13 @@ static const auto jlundefvarerror_func = new JuliaFunction<>{
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
get_attrs_noreturn,
};
static const auto jlhasnofield_func = new JuliaFunction<>{
XSTR(jl_has_no_field_error),
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted),
PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
get_attrs_noreturn,
};
static const auto jlboundserrorv_func = new JuliaFunction<TypeFnContextAndSizeT>{
XSTR(jl_bounds_error_ints),
[](LLVMContext &C, Type *T_size) { return FunctionType::get(getVoidTy(C),
Expand Down Expand Up @@ -3318,6 +3325,8 @@ static jl_llvm_functions_t
jl_value_t *jlrettype,
jl_codegen_params_t &params);

static void emit_hasnofield_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *type, jl_cgval_t name);

static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
const jl_cgval_t *argv, size_t nargs, jl_value_t *rt,
jl_expr_t *ex, bool is_promotable)
Expand Down Expand Up @@ -3819,6 +3828,19 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
return true;
}
}
else if (fld.typ == (jl_value_t*)jl_symbol_type) {
if (jl_is_datatype(utt) && !jl_is_namedtuple_type(utt)) { // TODO: Look into this for NamedTuple
if (jl_struct_try_layout(utt) && (jl_datatype_nfields(utt) == 1)) {
jl_svec_t *fn = jl_field_names(utt);
assert(jl_svec_len(fn) == 1);
Value *typ_sym = literal_pointer_val(ctx, jl_svecref(fn, 0));
Value *cond = ctx.builder.CreateICmpEQ(mark_callee_rooted(ctx, typ_sym), mark_callee_rooted(ctx, boxed(ctx, fld)));
emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld);
*ret = emit_getfield_knownidx(ctx, obj, 0, utt, order);
return true;
}
}
}
// TODO: generic getfield func with more efficient calling convention
return false;
}
Expand Down Expand Up @@ -4612,6 +4634,22 @@ static void undef_var_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *name)
ctx.builder.SetInsertPoint(ifok);
}

static void emit_hasnofield_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *type, jl_cgval_t name)
{
++EmittedUndefVarErrors;
assert(name.typ == (jl_value_t*)jl_symbol_type);
BasicBlock *err = BasicBlock::Create(ctx.builder.getContext(), "err", ctx.f);
BasicBlock *ifok = BasicBlock::Create(ctx.builder.getContext(), "ok");
ctx.builder.CreateCondBr(ok, ifok, err);
ctx.builder.SetInsertPoint(err);
ctx.builder.CreateCall(prepare_call(jlhasnofield_func),
{mark_callee_rooted(ctx, literal_pointer_val(ctx, (jl_value_t*)type)),
mark_callee_rooted(ctx, boxed(ctx, name))});
ctx.builder.CreateUnreachable();
ctx.f->getBasicBlockList().push_back(ifok);
ctx.builder.SetInsertPoint(ifok);
}

// returns a jl_ppvalue_t location for the global variable m.s
// if the reference currently bound or assign == true,
// pbnd will also be assigned with the binding address
Expand Down Expand Up @@ -9011,6 +9049,7 @@ static void init_jit_functions(void)
add_named_global(jlatomicerror_func, &jl_atomic_error);
add_named_global(jlthrow_func, &jl_throw);
add_named_global(jlundefvarerror_func, &jl_undefined_var_error);
add_named_global(jlhasnofield_func, &jl_has_no_field_error);
add_named_global(jlboundserrorv_func, &jl_bounds_error_ints);
add_named_global(jlboundserror_func, &jl_bounds_error_int);
add_named_global(jlvboundserror_func, &jl_bounds_error_tuple_int);
Expand Down
3 changes: 1 addition & 2 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -1558,8 +1558,7 @@ JL_DLLEXPORT int jl_field_index(jl_datatype_t *t, jl_sym_t *fld, int err)
}
}
if (err)
jl_errorf("type %s has no field %s", jl_symbol_name(t->name->name),
jl_symbol_name(fld));
jl_has_no_field_error(t->name->name, fld);
return -1;
}

Expand Down
1 change: 1 addition & 0 deletions src/jl_exported_funcs.inc
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@
XX(jl_uncompress_argname_n) \
XX(jl_uncompress_ir) \
XX(jl_undefined_var_error) \
XX(jl_has_no_field_error) \
XX(jl_value_ptr) \
XX(jl_ver_is_release) \
XX(jl_ver_major) \
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,7 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname,
jl_value_t *ty JL_MAYBE_UNROOTED,
jl_value_t *got JL_MAYBE_UNROOTED);
JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var);
JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var);
JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str);
JL_DLLEXPORT void JL_NORETURN jl_bounds_error(jl_value_t *v JL_MAYBE_UNROOTED,
jl_value_t *t JL_MAYBE_UNROOTED);
Expand Down
5 changes: 5 additions & 0 deletions src/rtutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var)
jl_throw(jl_new_struct(jl_undefvarerror_type, var));
}

JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var)
{
jl_errorf("type %s has no field %s", jl_symbol_name(type_name), jl_symbol_name(var));
}

JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str) // == jl_exceptionf(jl_atomicerror_type, "%s", str)
{
jl_value_t *msg = jl_pchar_to_string((char*)str, strlen(str));
Expand Down
12 changes: 12 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -820,3 +820,15 @@ end
# issue 48917, hoisting load to above the parent
f48917(x, w) = (y = (a=1, b=x); z = (; a=(a=(1, w), b=(3, y))))
@test f48917(1,2) == (a = (a = (1, 2), b = (3, (a = 1, b = 1))),)

# https://github.com/JuliaLang/julia/issues/50317 getproperty allocation on struct with 1 field
struct Wrapper50317
lock::ReentrantLock
end
const MONITOR50317 = Wrapper50317(ReentrantLock())
issue50317() = @noinline MONITOR50317.lock
issue50317()
let res = @timed issue50317()
@test res.bytes == 0
return res # must return otherwise the compiler may eliminate the result entirely
end