Skip to content

Commit fa5fbbb

Browse files
gbaraldikpamnany
authored andcommitted
Backport: Optimize getfield lowering to avoid boxing in some cases (JuliaLang#50444)
1 parent a74f9e2 commit fa5fbbb

File tree

6 files changed

+58
-2
lines changed

6 files changed

+58
-2
lines changed

src/codegen.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,13 @@ static const auto jlundefvarerror_func = new JuliaFunction{
693693
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
694694
get_attrs_noreturn,
695695
};
696+
static const auto jlhasnofield_func = new JuliaFunction{
697+
XSTR(jl_has_no_field_error),
698+
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
699+
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted),
700+
PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
701+
get_attrs_noreturn,
702+
};
696703
static const auto jlboundserrorv_func = new JuliaFunction{
697704
XSTR(jl_bounds_error_ints),
698705
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
@@ -3236,6 +3243,8 @@ static jl_llvm_functions_t
32363243
jl_value_t *jlrettype,
32373244
jl_codegen_params_t &params);
32383245

3246+
static void emit_hasnofield_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *type, jl_cgval_t name);
3247+
32393248
static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
32403249
const jl_cgval_t *argv, size_t nargs, jl_value_t *rt,
32413250
jl_expr_t *ex, bool is_promotable)
@@ -3706,6 +3715,18 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
37063715
*ret = mark_julia_type(ctx, fld_val, true, jl_any_type);
37073716
return true;
37083717
}
3718+
} else if (fld.typ == (jl_value_t*)jl_symbol_type) {
3719+
if (jl_is_datatype(utt) && !jl_is_namedtuple_type(utt)) { // TODO: Look into this for NamedTuple
3720+
if (jl_struct_try_layout(utt) && (jl_datatype_nfields(utt) == 1)) {
3721+
jl_svec_t *fn = jl_field_names(utt);
3722+
assert(jl_svec_len(fn) == 1);
3723+
Value *typ_sym = literal_pointer_val(ctx, jl_svecref(fn, 0));
3724+
Value *cond = ctx.builder.CreateICmpEQ(mark_callee_rooted(ctx, typ_sym), mark_callee_rooted(ctx, boxed(ctx, fld)));
3725+
emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld);
3726+
*ret = emit_getfield_knownidx(ctx, obj, 0, utt, order);
3727+
return true;
3728+
}
3729+
}
37093730
}
37103731
// TODO: generic getfield func with more efficient calling convention
37113732
return false;
@@ -4426,6 +4447,22 @@ static void undef_var_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *name)
44264447
ctx.builder.SetInsertPoint(ifok);
44274448
}
44284449

4450+
static void emit_hasnofield_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *type, jl_cgval_t name)
4451+
{
4452+
++EmittedUndefVarErrors;
4453+
assert(name.typ == (jl_value_t*)jl_symbol_type);
4454+
BasicBlock *err = BasicBlock::Create(ctx.builder.getContext(), "err", ctx.f);
4455+
BasicBlock *ifok = BasicBlock::Create(ctx.builder.getContext(), "ok");
4456+
ctx.builder.CreateCondBr(ok, ifok, err);
4457+
ctx.builder.SetInsertPoint(err);
4458+
ctx.builder.CreateCall(prepare_call(jlhasnofield_func),
4459+
{mark_callee_rooted(ctx, literal_pointer_val(ctx, (jl_value_t*)type)),
4460+
mark_callee_rooted(ctx, boxed(ctx, name))});
4461+
ctx.builder.CreateUnreachable();
4462+
ctx.f->getBasicBlockList().push_back(ifok);
4463+
ctx.builder.SetInsertPoint(ifok);
4464+
}
4465+
44294466
// returns a jl_ppvalue_t location for the global variable m.s
44304467
// if the reference currently bound or assign == true,
44314468
// pbnd will also be assigned with the binding address
@@ -8692,6 +8729,7 @@ static void init_jit_functions(void)
86928729
add_named_global(jlatomicerror_func, &jl_atomic_error);
86938730
add_named_global(jlthrow_func, &jl_throw);
86948731
add_named_global(jlundefvarerror_func, &jl_undefined_var_error);
8732+
add_named_global(jlhasnofield_func, &jl_has_no_field_error);
86958733
add_named_global(jlboundserrorv_func, &jl_bounds_error_ints);
86968734
add_named_global(jlboundserror_func, &jl_bounds_error_int);
86978735
add_named_global(jlvboundserror_func, &jl_bounds_error_tuple_int);

src/datatype.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,8 +1511,7 @@ JL_DLLEXPORT int jl_field_index(jl_datatype_t *t, jl_sym_t *fld, int err)
15111511
}
15121512
}
15131513
if (err)
1514-
jl_errorf("type %s has no field %s", jl_symbol_name(t->name->name),
1515-
jl_symbol_name(fld));
1514+
jl_has_no_field_error(t->name->name, fld);
15161515
return -1;
15171516
}
15181517

src/jl_exported_funcs.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@
513513
XX(jl_uncompress_argname_n) \
514514
XX(jl_uncompress_ir) \
515515
XX(jl_undefined_var_error) \
516+
XX(jl_has_no_field_error) \
516517
XX(jl_value_ptr) \
517518
XX(jl_ver_is_release) \
518519
XX(jl_ver_major) \

src/julia.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,7 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname,
17061706
jl_value_t *ty JL_MAYBE_UNROOTED,
17071707
jl_value_t *got JL_MAYBE_UNROOTED);
17081708
JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var);
1709+
JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var);
17091710
JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str);
17101711
JL_DLLEXPORT void JL_NORETURN jl_bounds_error(jl_value_t *v JL_MAYBE_UNROOTED,
17111712
jl_value_t *t JL_MAYBE_UNROOTED);

src/rtutils.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,11 @@ JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var)
132132
jl_throw(jl_new_struct(jl_undefvarerror_type, var));
133133
}
134134

135+
JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var)
136+
{
137+
jl_errorf("type %s has no field %s", jl_symbol_name(type_name), jl_symbol_name(var));
138+
}
139+
135140
JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str) // == jl_exceptionf(jl_atomicerror_type, "%s", str)
136141
{
137142
jl_value_t *msg = jl_pchar_to_string((char*)str, strlen(str));

test/compiler/codegen.jl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,3 +791,15 @@ end
791791
f48085(@nospecialize x...) = length(x)
792792
@test Core.Compiler.get_compileable_sig(which(f48085, (Vararg{Any},)), Tuple{typeof(f48085), Vararg{Int}}, Core.svec()) === nothing
793793
@test Core.Compiler.get_compileable_sig(which(f48085, (Vararg{Any},)), Tuple{typeof(f48085), Int, Vararg{Int}}, Core.svec()) === Tuple{typeof(f48085), Any, Vararg{Any}}
794+
795+
# https://github.com/JuliaLang/julia/issues/50317 getproperty allocation on struct with 1 field
796+
struct Wrapper50317
797+
lock::ReentrantLock
798+
end
799+
const MONITOR50317 = Wrapper50317(ReentrantLock())
800+
issue50317() = @noinline MONITOR50317.lock
801+
issue50317()
802+
let res = @timed issue50317()
803+
@test res.bytes == 0
804+
return res # must return otherwise the compiler may eliminate the result entirely
805+
end

0 commit comments

Comments
 (0)