From 68d8ef99e0d108e356531eeb8077c98811176eb2 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Wed, 12 Jul 2023 14:19:33 -0300 Subject: [PATCH 1/6] Avoid boxing/apply generic in most cases --- src/codegen.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 04f7564dd3e33..6479f466116d4 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1099,6 +1099,18 @@ static const auto jlgetnthfieldchecked_func = new JuliaFunction{ + XSTR(jl_field_index), + [](LLVMContext &C) { + auto T_prjlvalue = JuliaType::get_prjlvalue_ty(C); + return FunctionType::get(getInt32Ty(C), + {T_prjlvalue, T_prjlvalue, getInt32Ty(C)}, false); + }, + [](LLVMContext &C) { return AttributeList::get(C, + Attributes(C, {Attribute::NoUnwind, Attribute::ReadOnly, Attribute::WillReturn}), + AttributeSet(), + None); }, // This function can error if the third argument is 1 so don't do that. +}; static const auto jlfieldisdefinedchecked_func = new JuliaFunction{ XSTR(jl_field_isdefined_checked), [](LLVMContext &C, Type *T_size) { @@ -3829,8 +3841,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, } } 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)) { + if (jl_is_datatype(utt) && (utt != jl_module_type) && jl_struct_try_layout(utt)) { + if ((jl_datatype_nfields(utt) == 1 && !jl_is_namedtuple_type(utt))) { 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)); @@ -3839,9 +3851,17 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, *ret = emit_getfield_knownidx(ctx, obj, 0, utt, order); return true; } + else { + Value *index = ctx.builder.CreateCall(prepare_call(jlfieldindex_func), + {emit_typeof(ctx, obj, false, false), boxed(ctx, fld), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)}); + Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); + emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); + Value *idx2 = ctx.builder.CreateAdd(ctx.builder.CreateIntCast(index, ctx.types().T_size, false), ConstantInt::get(ctx.types().T_size, 1)); + if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, boundscheck, order)) // Maybe we don't need a boundscheck? + return true; + } } } - // TODO: generic getfield func with more efficient calling convention return false; } @@ -9093,6 +9113,7 @@ static void init_jit_functions(void) add_named_global("jl_adopt_thread", &jl_adopt_thread); add_named_global(jlgetcfunctiontrampoline_func, &jl_get_cfunction_trampoline); add_named_global(jlgetnthfieldchecked_func, &jl_get_nth_field_checked); + add_named_global(jlfieldindex_func, &jl_field_index); add_named_global(diff_gc_total_bytes_func, &jl_gc_diff_total_bytes); add_named_global(sync_gc_total_bytes_func, &jl_gc_sync_total_bytes); add_named_global(jlarray_data_owner_func, &jl_array_data_owner); From 59edb5778e0fae14b54e0b00979c87b5b8d1af2f Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Wed, 12 Jul 2023 18:43:11 -0300 Subject: [PATCH 2/6] Don't emit bounds check --- src/codegen.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 6479f466116d4..cc4bc6cd40a4f 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1,5 +1,6 @@ // This file is a part of Julia. License is MIT: https://julialang.org/license +#include "julia.h" #undef DEBUG #include "llvm-version.h" #include "platform.h" @@ -3857,7 +3858,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); Value *idx2 = ctx.builder.CreateAdd(ctx.builder.CreateIntCast(index, ctx.types().T_size, false), ConstantInt::get(ctx.types().T_size, 1)); - if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, boundscheck, order)) // Maybe we don't need a boundscheck? + if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) return true; } } From e788fc53799165222feea61cccd6567999b96875 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Wed, 12 Jul 2023 19:04:35 -0300 Subject: [PATCH 3/6] Whitespace and header --- src/codegen.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index cc4bc6cd40a4f..162d4c8f9a9b6 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1,6 +1,5 @@ // This file is a part of Julia. License is MIT: https://julialang.org/license -#include "julia.h" #undef DEBUG #include "llvm-version.h" #include "platform.h" @@ -3858,7 +3857,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); Value *idx2 = ctx.builder.CreateAdd(ctx.builder.CreateIntCast(index, ctx.types().T_size, false), ConstantInt::get(ctx.types().T_size, 1)); - if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) + if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) return true; } } From dc5097847cbf1aa74786d7b68c7b49fa133fb978 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Thu, 13 Jul 2023 12:41:13 -0300 Subject: [PATCH 4/6] Simplify code and add tests/comments --- src/codegen.cpp | 25 +++++++------------------ test/compiler/codegen.jl | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 162d4c8f9a9b6..fd3800d7fe651 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3840,26 +3840,15 @@ 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) { + else if (fld.typ == (jl_value_t*)jl_symbol_type) { // Known type but unknown symbol if (jl_is_datatype(utt) && (utt != jl_module_type) && jl_struct_try_layout(utt)) { - if ((jl_datatype_nfields(utt) == 1 && !jl_is_namedtuple_type(utt))) { - 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); + Value *index = ctx.builder.CreateCall(prepare_call(jlfieldindex_func), + {emit_typeof(ctx, obj, false, false), boxed(ctx, fld), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)}); + Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); + emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); + 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 + if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) return true; - } - else { - Value *index = ctx.builder.CreateCall(prepare_call(jlfieldindex_func), - {emit_typeof(ctx, obj, false, false), boxed(ctx, fld), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)}); - Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); - emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); - Value *idx2 = ctx.builder.CreateAdd(ctx.builder.CreateIntCast(index, ctx.types().T_size, false), ConstantInt::get(ctx.types().T_size, 1)); - if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) - return true; - } } } return false; diff --git a/test/compiler/codegen.jl b/test/compiler/codegen.jl index 85013ce30d2ca..d5c0ecb7cc781 100644 --- a/test/compiler/codegen.jl +++ b/test/compiler/codegen.jl @@ -832,3 +832,19 @@ let res = @timed issue50317() @test res.bytes == 0 return res # must return otherwise the compiler may eliminate the result entirely end +struct Wrapper50317_2 + lock::ReentrantLock + fun::Vector{Int} +end +const MONITOR50317_2 = Wrapper50317_2(ReentrantLock(),[1]) +issue50317_2() = @noinline MONITOR50317.lock +issue50317_2() +let res = @timed issue50317_2() + @test res.bytes == 0 + return res +end +const a50317 = (b=3,) +let res = @timed a50317[:b] + @test res.bytes == 0 + return res +end \ No newline at end of file From 4125dfe0790ff62f37523db31f5f4e95ac04deba Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Thu, 13 Jul 2023 12:44:38 -0300 Subject: [PATCH 5/6] Whitespace --- test/compiler/codegen.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/compiler/codegen.jl b/test/compiler/codegen.jl index d5c0ecb7cc781..26d9f0c08e797 100644 --- a/test/compiler/codegen.jl +++ b/test/compiler/codegen.jl @@ -847,4 +847,4 @@ const a50317 = (b=3,) let res = @timed a50317[:b] @test res.bytes == 0 return res -end \ No newline at end of file +end From 5caf1caa9e7ed65930660405c072f4e12f8b5568 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Fri, 14 Jul 2023 16:24:09 -0300 Subject: [PATCH 6/6] Readd special case --- src/codegen.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index fd3800d7fe651..d11ba8cb851ec 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3842,13 +3842,24 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, } else if (fld.typ == (jl_value_t*)jl_symbol_type) { // Known type but unknown symbol if (jl_is_datatype(utt) && (utt != jl_module_type) && jl_struct_try_layout(utt)) { - Value *index = ctx.builder.CreateCall(prepare_call(jlfieldindex_func), - {emit_typeof(ctx, obj, false, false), boxed(ctx, fld), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)}); - Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); - emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); - 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 - if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) + if ((jl_datatype_nfields(utt) == 1 && !jl_is_namedtuple_type(utt))) { + 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; + } + else { + Value *index = ctx.builder.CreateCall(prepare_call(jlfieldindex_func), + {emit_typeof(ctx, obj, false, false), boxed(ctx, fld), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)}); + Value *cond = ctx.builder.CreateICmpNE(index, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), -1)); + emit_hasnofield_error_ifnot(ctx, cond, utt->name->name, fld); + 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 + if (emit_getfield_unknownidx(ctx, ret, obj, idx2, utt, jl_false, order)) + return true; + } } } return false;