Skip to content

Commit 51207fa

Browse files
gbaraldiKristofferC
authored andcommitted
Optimize getfield lowering to avoid boxing in some cases (#50444)
(cherry picked from commit 0718995)
1 parent 0ae05d3 commit 51207fa

File tree

6 files changed

+59
-2
lines changed

6 files changed

+59
-2
lines changed

src/codegen.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,13 @@ static const auto jlundefvarerror_func = new JuliaFunction<>{
739739
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
740740
get_attrs_noreturn,
741741
};
742+
static const auto jlhasnofield_func = new JuliaFunction<>{
743+
XSTR(jl_has_no_field_error),
744+
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
745+
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted),
746+
PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
747+
get_attrs_noreturn,
748+
};
742749
static const auto jlboundserrorv_func = new JuliaFunction<TypeFnContextAndSizeT>{
743750
XSTR(jl_bounds_error_ints),
744751
[](LLVMContext &C, Type *T_size) { return FunctionType::get(getVoidTy(C),
@@ -3318,6 +3325,8 @@ static jl_llvm_functions_t
33183325
jl_value_t *jlrettype,
33193326
jl_codegen_params_t &params);
33203327

3328+
static void emit_hasnofield_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *type, jl_cgval_t name);
3329+
33213330
static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
33223331
const jl_cgval_t *argv, size_t nargs, jl_value_t *rt,
33233332
jl_expr_t *ex, bool is_promotable)
@@ -3819,6 +3828,19 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
38193828
return true;
38203829
}
38213830
}
3831+
else if (fld.typ == (jl_value_t*)jl_symbol_type) {
3832+
if (jl_is_datatype(utt) && !jl_is_namedtuple_type(utt)) { // TODO: Look into this for NamedTuple
3833+
if (jl_struct_try_layout(utt) && (jl_datatype_nfields(utt) == 1)) {
3834+
jl_svec_t *fn = jl_field_names(utt);
3835+
assert(jl_svec_len(fn) == 1);
3836+
Value *typ_sym = literal_pointer_val(ctx, jl_svecref(fn, 0));
3837+
Value *cond = ctx.builder.CreateICmpEQ(mark_callee_rooted(ctx, typ_sym), mark_callee_rooted(ctx, boxed(ctx, fld)));
3838+
emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld);
3839+
*ret = emit_getfield_knownidx(ctx, obj, 0, utt, order);
3840+
return true;
3841+
}
3842+
}
3843+
}
38223844
// TODO: generic getfield func with more efficient calling convention
38233845
return false;
38243846
}
@@ -4612,6 +4634,22 @@ static void undef_var_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *name)
46124634
ctx.builder.SetInsertPoint(ifok);
46134635
}
46144636

4637+
static void emit_hasnofield_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *type, jl_cgval_t name)
4638+
{
4639+
++EmittedUndefVarErrors;
4640+
assert(name.typ == (jl_value_t*)jl_symbol_type);
4641+
BasicBlock *err = BasicBlock::Create(ctx.builder.getContext(), "err", ctx.f);
4642+
BasicBlock *ifok = BasicBlock::Create(ctx.builder.getContext(), "ok");
4643+
ctx.builder.CreateCondBr(ok, ifok, err);
4644+
ctx.builder.SetInsertPoint(err);
4645+
ctx.builder.CreateCall(prepare_call(jlhasnofield_func),
4646+
{mark_callee_rooted(ctx, literal_pointer_val(ctx, (jl_value_t*)type)),
4647+
mark_callee_rooted(ctx, boxed(ctx, name))});
4648+
ctx.builder.CreateUnreachable();
4649+
ctx.f->getBasicBlockList().push_back(ifok);
4650+
ctx.builder.SetInsertPoint(ifok);
4651+
}
4652+
46154653
// returns a jl_ppvalue_t location for the global variable m.s
46164654
// if the reference currently bound or assign == true,
46174655
// pbnd will also be assigned with the binding address
@@ -9011,6 +9049,7 @@ static void init_jit_functions(void)
90119049
add_named_global(jlatomicerror_func, &jl_atomic_error);
90129050
add_named_global(jlthrow_func, &jl_throw);
90139051
add_named_global(jlundefvarerror_func, &jl_undefined_var_error);
9052+
add_named_global(jlhasnofield_func, &jl_has_no_field_error);
90149053
add_named_global(jlboundserrorv_func, &jl_bounds_error_ints);
90159054
add_named_global(jlboundserror_func, &jl_bounds_error_int);
90169055
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
@@ -1558,8 +1558,7 @@ JL_DLLEXPORT int jl_field_index(jl_datatype_t *t, jl_sym_t *fld, int err)
15581558
}
15591559
}
15601560
if (err)
1561-
jl_errorf("type %s has no field %s", jl_symbol_name(t->name->name),
1562-
jl_symbol_name(fld));
1561+
jl_has_no_field_error(t->name->name, fld);
15631562
return -1;
15641563
}
15651564

src/jl_exported_funcs.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@
510510
XX(jl_uncompress_argname_n) \
511511
XX(jl_uncompress_ir) \
512512
XX(jl_undefined_var_error) \
513+
XX(jl_has_no_field_error) \
513514
XX(jl_value_ptr) \
514515
XX(jl_ver_is_release) \
515516
XX(jl_ver_major) \

src/julia.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,6 +1790,7 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname,
17901790
jl_value_t *ty JL_MAYBE_UNROOTED,
17911791
jl_value_t *got JL_MAYBE_UNROOTED);
17921792
JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var);
1793+
JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var);
17931794
JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str);
17941795
JL_DLLEXPORT void JL_NORETURN jl_bounds_error(jl_value_t *v JL_MAYBE_UNROOTED,
17951796
jl_value_t *t JL_MAYBE_UNROOTED);

src/rtutils.c

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

137+
JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_sym_t *type_name, jl_sym_t *var)
138+
{
139+
jl_errorf("type %s has no field %s", jl_symbol_name(type_name), jl_symbol_name(var));
140+
}
141+
137142
JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str) // == jl_exceptionf(jl_atomicerror_type, "%s", str)
138143
{
139144
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
@@ -820,3 +820,15 @@ end
820820
# issue 48917, hoisting load to above the parent
821821
f48917(x, w) = (y = (a=1, b=x); z = (; a=(a=(1, w), b=(3, y))))
822822
@test f48917(1,2) == (a = (a = (1, 2), b = (3, (a = 1, b = 1))),)
823+
824+
# https://github.com/JuliaLang/julia/issues/50317 getproperty allocation on struct with 1 field
825+
struct Wrapper50317
826+
lock::ReentrantLock
827+
end
828+
const MONITOR50317 = Wrapper50317(ReentrantLock())
829+
issue50317() = @noinline MONITOR50317.lock
830+
issue50317()
831+
let res = @timed issue50317()
832+
@test res.bytes == 0
833+
return res # must return otherwise the compiler may eliminate the result entirely
834+
end

0 commit comments

Comments
 (0)