Skip to content

Commit d673a25

Browse files
gbaraldiKristofferC
authored andcommitted
Avoid generic call in most cases for getproperty (#50523)
More generic than #50444. Should we keep the boundscheck on the getfield since we got the index from the runtime itself? (cherry picked from commit 207c09a)
1 parent 6ea6ddd commit d673a25

File tree

2 files changed

+41
-4
lines changed

2 files changed

+41
-4
lines changed

src/codegen.cpp

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,18 @@ static const auto jlgetnthfieldchecked_func = new JuliaFunction<TypeFnContextAnd
10991099
Attributes(C, {Attribute::NonNull}),
11001100
None); },
11011101
};
1102+
static const auto jlfieldindex_func = new JuliaFunction<>{
1103+
XSTR(jl_field_index),
1104+
[](LLVMContext &C) {
1105+
auto T_prjlvalue = JuliaType::get_prjlvalue_ty(C);
1106+
return FunctionType::get(getInt32Ty(C),
1107+
{T_prjlvalue, T_prjlvalue, getInt32Ty(C)}, false);
1108+
},
1109+
[](LLVMContext &C) { return AttributeList::get(C,
1110+
Attributes(C, {Attribute::NoUnwind, Attribute::ReadOnly, Attribute::WillReturn}),
1111+
AttributeSet(),
1112+
None); }, // This function can error if the third argument is 1 so don't do that.
1113+
};
11021114
static const auto jlfieldisdefinedchecked_func = new JuliaFunction<TypeFnContextAndSizeT>{
11031115
XSTR(jl_field_isdefined_checked),
11041116
[](LLVMContext &C, Type *T_size) {
@@ -3828,9 +3840,9 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
38283840
return true;
38293841
}
38303842
}
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)) {
3843+
else if (fld.typ == (jl_value_t*)jl_symbol_type) { // Known type but unknown symbol
3844+
if (jl_is_datatype(utt) && (utt != jl_module_type) && jl_struct_try_layout(utt)) {
3845+
if ((jl_datatype_nfields(utt) == 1 && !jl_is_namedtuple_type(utt))) {
38343846
jl_svec_t *fn = jl_field_names(utt);
38353847
assert(jl_svec_len(fn) == 1);
38363848
Value *typ_sym = literal_pointer_val(ctx, jl_svecref(fn, 0));
@@ -3839,9 +3851,17 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
38393851
*ret = emit_getfield_knownidx(ctx, obj, 0, utt, order);
38403852
return true;
38413853
}
3854+
else {
3855+
Value *index = ctx.builder.CreateCall(prepare_call(jlfieldindex_func),
3856+
{emit_typeof(ctx, obj, false, false), boxed(ctx, fld), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)});
3857+
Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1));
3858+
emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld);
3859+
Value *idx2 = ctx.builder.CreateAdd(ctx.builder.CreateIntCast(index, ctx.types().T_size, false), ConstantInt::get(ctx.types().T_size, 1)); // getfield_unknown is 1 based
3860+
if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order))
3861+
return true;
3862+
}
38423863
}
38433864
}
3844-
// TODO: generic getfield func with more efficient calling convention
38453865
return false;
38463866
}
38473867

@@ -9093,6 +9113,7 @@ static void init_jit_functions(void)
90939113
add_named_global("jl_adopt_thread", &jl_adopt_thread);
90949114
add_named_global(jlgetcfunctiontrampoline_func, &jl_get_cfunction_trampoline);
90959115
add_named_global(jlgetnthfieldchecked_func, &jl_get_nth_field_checked);
9116+
add_named_global(jlfieldindex_func, &jl_field_index);
90969117
add_named_global(diff_gc_total_bytes_func, &jl_gc_diff_total_bytes);
90979118
add_named_global(sync_gc_total_bytes_func, &jl_gc_sync_total_bytes);
90989119
add_named_global(jlarray_data_owner_func, &jl_array_data_owner);

test/compiler/codegen.jl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,3 +832,19 @@ let res = @timed issue50317()
832832
@test res.bytes == 0
833833
return res # must return otherwise the compiler may eliminate the result entirely
834834
end
835+
struct Wrapper50317_2
836+
lock::ReentrantLock
837+
fun::Vector{Int}
838+
end
839+
const MONITOR50317_2 = Wrapper50317_2(ReentrantLock(),[1])
840+
issue50317_2() = @noinline MONITOR50317.lock
841+
issue50317_2()
842+
let res = @timed issue50317_2()
843+
@test res.bytes == 0
844+
return res
845+
end
846+
const a50317 = (b=3,)
847+
let res = @timed a50317[:b]
848+
@test res.bytes == 0
849+
return res
850+
end

0 commit comments

Comments
 (0)