From 4eb2840200116a0fc1e4b9f12afdd29463e23936 Mon Sep 17 00:00:00 2001 From: oscarddssmith Date: Tue, 17 Jun 2025 17:35:16 -0400 Subject: [PATCH 1/7] expand memoryrefnew capabilities --- Compiler/src/optimize.jl | 4 +++- Compiler/src/tfuncs.jl | 4 ++-- base/boot.jl | 2 +- base/essentials.jl | 4 ++-- src/builtins.c | 31 +++++++++++++++++++++++-------- src/codegen.cpp | 17 +++++++++++++---- 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/Compiler/src/optimize.jl b/Compiler/src/optimize.jl index da4a17c5d6913..27fb1e2168639 100644 --- a/Compiler/src/optimize.jl +++ b/Compiler/src/optimize.jl @@ -720,7 +720,9 @@ function iscall_with_boundscheck(@nospecialize(stmt), sv::PostOptAnalysisState) f === nothing && return false if f === getfield nargs = 4 - elseif f === memoryrefnew || f === memoryrefget || f === memoryref_isassigned + elseif f === memoryrefnew + nargs= 3 + elseif f === memoryrefget || f === memoryref_isassigned nargs = 4 elseif f === memoryrefset! nargs = 5 diff --git a/Compiler/src/tfuncs.jl b/Compiler/src/tfuncs.jl index e42afb065687c..b20616a519919 100644 --- a/Compiler/src/tfuncs.jl +++ b/Compiler/src/tfuncs.jl @@ -2107,7 +2107,7 @@ end add_tfunc(memoryrefoffset, 1, 1, memoryrefoffset_tfunc, 5) @nospecs function memoryref_builtin_common_errorcheck(mem, order, boundscheck) - hasintersect(widenconst(mem), GenericMemoryRef) || return false + hasintersect(widenconst(mem), Union{GenericMemory, GenericMemoryRef}) || return false hasintersect(widenconst(order), Symbol) || return false hasintersect(widenconst(unwrapva(boundscheck)), Bool) || return false return true @@ -2203,7 +2203,7 @@ function memoryref_builtin_common_nothrow(argtypes::Vector{Any}) idx = widenconst(argtypes[2]) idx ⊑ Int || return false boundscheck ⊑ Bool || return false - memtype ⊑ GenericMemoryRef || return false + memtype ⊑ Union{GenericMemory, GenericMemoryRef} || return false # If we have @inbounds (last argument is false), we're allowed to assume # we don't throw bounds errors. if isa(boundscheck, Const) diff --git a/base/boot.jl b/base/boot.jl index f1ef8bd37a276..d055c47516f91 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -592,7 +592,7 @@ const undef = UndefInitializer() # memoryref is simply convenience wrapper function around memoryrefnew memoryref(mem::GenericMemory) = memoryrefnew(mem) -memoryref(mem::GenericMemory, i::Integer) = memoryrefnew(memoryrefnew(mem), Int(i), @_boundscheck) +memoryref(mem::GenericMemory, i::Integer) = memoryrefnew(mem, Int(i), @_boundscheck) memoryref(ref::GenericMemoryRef, i::Integer) = memoryrefnew(ref, Int(i), @_boundscheck) GenericMemoryRef(mem::GenericMemory) = memoryref(mem) GenericMemoryRef(mem::GenericMemory, i::Integer) = memoryref(mem, i) diff --git a/base/essentials.jl b/base/essentials.jl index 82fd3d45c0429..f27346c4b43cb 100644 --- a/base/essentials.jl +++ b/base/essentials.jl @@ -395,7 +395,7 @@ default_access_order(a::GenericMemoryRef{:atomic}) = :monotonic function getindex(A::GenericMemory, i::Int) @_noub_if_noinbounds_meta (@_boundscheck) && checkbounds(A, i) - memoryrefget(memoryrefnew(memoryrefnew(A), i, false), default_access_order(A), false) + memoryrefget(memoryrefnew(A, i, false), default_access_order(A), false) end getindex(A::GenericMemoryRef) = memoryrefget(A, default_access_order(A), @_boundscheck) @@ -973,7 +973,7 @@ function setindex!(A::Array{Any}, @nospecialize(x), i::Int) memoryrefset!(memoryrefnew(getfield(A, :ref), i, false), x, :not_atomic, false) return A end -setindex!(A::Memory{Any}, @nospecialize(x), i::Int) = (memoryrefset!(memoryrefnew(memoryrefnew(A), i, @_boundscheck), x, :not_atomic, @_boundscheck); A) +setindex!(A::Memory{Any}, @nospecialize(x), i::Int) = (memoryrefset!(memoryrefnew(A, i, @_boundscheck), x, :not_atomic, @_boundscheck); A) setindex!(A::MemoryRef{T}, x) where {T} = (memoryrefset!(A, convert(T, x), :not_atomic, @_boundscheck); A) setindex!(A::MemoryRef{Any}, @nospecialize(x)) = (memoryrefset!(A, x, :not_atomic, @_boundscheck); A) diff --git a/src/builtins.c b/src/builtins.c index 46fceb91f86e7..c1352b8cf738b 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1765,30 +1765,45 @@ JL_CALLABLE(jl_f_memoryrefnew) return (jl_value_t*)jl_new_memoryref(typ, m, m->ptr); } else { - JL_TYPECHK(memoryrefnew, genericmemoryref, args[0]); JL_TYPECHK(memoryrefnew, long, args[1]); if (nargs == 3) JL_TYPECHK(memoryrefnew, bool, args[2]); - jl_genericmemoryref_t *m = (jl_genericmemoryref_t*)args[0]; size_t i = jl_unbox_long(args[1]) - 1; - const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(m->mem))->layout; - char *data = (char*)m->ptr_or_offset; + char *data; + if(jl_is_genericmemory(args[0])) { + jl_genericmemory_t *m = (jl_genericmemory_t*)args[0]; + jl_value_t *typ = jl_apply_type((jl_value_t*)jl_genericmemoryref_type, jl_svec_data(((jl_datatype_t*)jl_typetagof(m))->parameters), 3); + JL_GC_PROMISE_ROOTED(typ); // it is a concrete type + if (i >= m->length) + jl_bounds_error((jl_value_t*)m, args[1]); + const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(m))->layout; + if (layout->flags.arrayelem_isunion || layout->size == 0) + return (jl_value_t*)jl_new_memoryref(typ, m, (char*)i); + else if (layout->flags.arrayelem_isboxed) + return (jl_value_t*)jl_new_memoryref(typ, m, (char*)m->ptr + sizeof(jl_value_t*)*i); + return (jl_value_t*)jl_new_memoryref(typ, m, (char*)m->ptr + layout->size*i); + } + JL_TYPECHK(memoryrefnew, genericmemoryref, args[0]); + jl_genericmemoryref_t *m = (jl_genericmemoryref_t*)args[0]; + jl_genericmemory_t *mem = m->mem; + data = (char*)m->ptr_or_offset; + const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(mem))->layout; if (layout->flags.arrayelem_isboxed) { - if (((data - (char*)m->mem->ptr) / sizeof(jl_value_t*)) + i >= m->mem->length) + if (((data - (char*)mem->ptr) / sizeof(jl_value_t*)) + i >= mem->length) jl_bounds_error((jl_value_t*)m, args[1]); data += sizeof(jl_value_t*) * i; } else if (layout->flags.arrayelem_isunion || layout->size == 0) { - if ((size_t)data + i >= m->mem->length) + if ((size_t)data + i >= mem->length) jl_bounds_error((jl_value_t*)m, args[1]); data += i; } else { - if (((data - (char*)m->mem->ptr) / layout->size) + i >= m->mem->length) + if (((data - (char*)mem->ptr) / layout->size) + i >= mem->length) jl_bounds_error((jl_value_t*)m, args[1]); data += layout->size * i; } - return (jl_value_t*)jl_new_memoryref((jl_value_t*)jl_typetagof(m), m->mem, data); + return (jl_value_t*)jl_new_memoryref((jl_value_t*)jl_typetagof(m), mem, data); } } diff --git a/src/codegen.cpp b/src/codegen.cpp index 88b6e9f1f4513..8ce0403988794 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -4138,16 +4138,25 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, else if (f == BUILTIN(memoryrefnew) && (nargs == 2 || nargs == 3)) { const jl_cgval_t &ref = argv[1]; - jl_value_t *mty_dt = jl_unwrap_unionall(ref.typ); - if (jl_is_genericmemoryref_type(mty_dt) && jl_is_concrete_type(mty_dt)) { - mty_dt = jl_field_type_concrete((jl_datatype_t*)mty_dt, 1); - const jl_datatype_layout_t *layout = ((jl_datatype_t*)mty_dt)->layout; + jl_datatype_t *mty_dt = (jl_datatype_t*)jl_unwrap_unionall(ref.typ); + if (jl_is_genericmemoryref_type(mty_dt) && jl_is_concrete_type((jl_value_t*)mty_dt)) { + mty_dt = (jl_datatype_t*)jl_field_type_concrete(mty_dt, 1); + const jl_datatype_layout_t *layout = mty_dt->layout; jl_value_t *boundscheck = nargs == 3 ? argv[3].constant : nullptr; if (nargs == 3) emit_typecheck(ctx, argv[3], (jl_value_t*)jl_bool_type, "memoryrefnew"); *ret = emit_memoryref(ctx, ref, argv[2], boundscheck, layout); return true; } + if (jl_is_genericmemory_type(mty_dt) && jl_is_concrete_type((jl_value_t*)mty_dt)) { + const jl_datatype_layout_t *layout = mty_dt->layout; + jl_value_t *boundscheck = nargs == 3 ? argv[3].constant : nullptr; + if (nargs == 3) + emit_typecheck(ctx, argv[3], (jl_value_t*)jl_bool_type, "memoryrefnew"); + jl_value_t *typ = jl_apply_type((jl_value_t*)jl_genericmemoryref_type, jl_svec_data(mty_dt->parameters), jl_svec_len(mty_dt->parameters)); + *ret = emit_memoryref(ctx, _emit_memoryref(ctx, ref, layout, typ), argv[2], boundscheck, layout); + return true; + } } else if (f == BUILTIN(memoryrefoffset) && nargs == 1) { From b151b58013d9e753721a11fcee83c2b274662323 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Wed, 18 Jun 2025 20:50:45 -0400 Subject: [PATCH 2/7] fix --- Compiler/src/tfuncs.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/Compiler/src/tfuncs.jl b/Compiler/src/tfuncs.jl index b20616a519919..fb173759ab122 100644 --- a/Compiler/src/tfuncs.jl +++ b/Compiler/src/tfuncs.jl @@ -2096,6 +2096,7 @@ end @nospecs function memoryref_tfunc(𝕃::AbstractLattice, ref, idx, boundscheck) memoryref_builtin_common_errorcheck(ref, Const(:not_atomic), boundscheck) || return Bottom hasintersect(widenconst(idx), Int) || return Bottom + hasintersect(widenconst(ref), GenericMemory) && return memoryref_tfunc(𝕃, ref) return ref end add_tfunc(memoryrefnew, 1, 3, memoryref_tfunc, 1) From becfc1d7727aa906aea1c6d3d92da72e84442ea0 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Wed, 18 Jun 2025 22:47:33 -0400 Subject: [PATCH 3/7] fix compiler tests --- Compiler/test/inference.jl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Compiler/test/inference.jl b/Compiler/test/inference.jl index 21d0f079b91d7..58f2231319af9 100644 --- a/Compiler/test/inference.jl +++ b/Compiler/test/inference.jl @@ -1597,9 +1597,12 @@ let memoryref_tfunc(@nospecialize xs...) = Compiler.memoryref_tfunc(Compiler.fal @test memoryref_tfunc(MemoryRef{Int}, Int, Symbol) == Union{} @test memoryref_tfunc(MemoryRef{Int}, Int, Bool) == MemoryRef{Int} @test memoryref_tfunc(MemoryRef{Int}, Int, Vararg{Bool}) == MemoryRef{Int} - @test memoryref_tfunc(Memory{Int}, Int) == Union{} - @test memoryref_tfunc(Any, Any, Any) == Any # also probably could be GenericMemoryRef - @test memoryref_tfunc(Any, Any) == Any # also probably could be GenericMemoryRef + @test memoryref_tfunc(Memory{Int}, Int) == MemoryRef{Int} + @test memoryref_tfunc(Memory{Int}, Int, Symbol) == Union{} + @test memoryref_tfunc(Memory{Int}, Int, Bool) == MemoryRef{Int} + @test memoryref_tfunc(Memory{Int}, Int, Vararg{Bool}) == MemoryRef{Int} + @test memoryref_tfunc(Any, Any, Any) == GenericMemoryRef + @test memoryref_tfunc(Any, Any) == GenericMemoryRef @test memoryref_tfunc(Any) == GenericMemoryRef @test memoryrefget_tfunc(MemoryRef{Int}, Symbol, Bool) === Int @test memoryrefget_tfunc(MemoryRef{Int}, Any, Any) === Int From 973057c870d1a42b90b77d9948155c7a2187af14 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Fri, 20 Jun 2025 11:36:34 -0400 Subject: [PATCH 4/7] add codegen --- src/cgutils.cpp | 39 +++++++++++++++++++++++++++++++++++++++ src/codegen.cpp | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 7c2f62ac3350c..44f44dc6050f8 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -4684,6 +4684,45 @@ static jl_cgval_t _emit_memoryref(jl_codectx_t &ctx, const jl_cgval_t &mem, cons return _emit_memoryref(ctx, boxed(ctx, mem), data, layout, typ); } +static jl_cgval_t emit_memoryref_direct(jl_codectx_t &ctx, const jl_cgval_t &mem, jl_cgval_t idx, jl_value_t *typ, jl_value_t *inbounds, const jl_datatype_layout_t *layout) +{ + bool isboxed = layout->flags.arrayelem_isboxed; + bool isunion = layout->flags.arrayelem_isunion; + bool isghost = layout->size == 0; + Value *boxmem = boxed(ctx, mem); + Value *i = emit_unbox(ctx, ctx.types().T_size, idx, (jl_value_t*)jl_long_type); + Value *idx0 = ctx.builder.CreateSub(i, ConstantInt::get(ctx.types().T_size, 1)); + bool bc = bounds_check_enabled(ctx, inbounds); + if (bc) { + BasicBlock *failBB, *endBB; + failBB = BasicBlock::Create(ctx.builder.getContext(), "oob"); + endBB = BasicBlock::Create(ctx.builder.getContext(), "idxend"); + Value *mlen = emit_genericmemorylen(ctx, boxmem, typ); + Value *inbound = ctx.builder.CreateICmpULT(idx0, mlen); + setName(ctx.emission_context, inbound, "memoryref_isinbounds"); + ctx.builder.CreateCondBr(inbound, endBB, failBB); + failBB->insertInto(ctx.f); + ctx.builder.SetInsertPoint(failBB); + ctx.builder.CreateCall(prepare_call(jlboundserror_func), + { mark_callee_rooted(ctx, boxmem), i }); + ctx.builder.CreateUnreachable(); + endBB->insertInto(ctx.f); + ctx.builder.SetInsertPoint(endBB); + } + Value *data; + + if ((!isboxed && isunion) || isghost) { + data = idx0; + + } else { + data = emit_genericmemoryptr(ctx, boxmem, layout, 0); + Type *elty = isboxed ? ctx.types().T_prjlvalue : julia_type_to_llvm(ctx, jl_tparam1(typ)); + data = ctx.builder.CreateInBoundsGEP(elty, data, idx0); + } + + return _emit_memoryref(ctx, boxmem, data, layout, typ); +} + static Value *emit_memoryref_FCA(jl_codectx_t &ctx, const jl_cgval_t &ref, const jl_datatype_layout_t *layout) { if (!ref.inline_roots.empty()) { diff --git a/src/codegen.cpp b/src/codegen.cpp index 8ce0403988794..2c38ea4e5d907 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -4154,7 +4154,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, if (nargs == 3) emit_typecheck(ctx, argv[3], (jl_value_t*)jl_bool_type, "memoryrefnew"); jl_value_t *typ = jl_apply_type((jl_value_t*)jl_genericmemoryref_type, jl_svec_data(mty_dt->parameters), jl_svec_len(mty_dt->parameters)); - *ret = emit_memoryref(ctx, _emit_memoryref(ctx, ref, layout, typ), argv[2], boundscheck, layout); + *ret = emit_memoryref_direct(ctx, ref, argv[2], typ, boundscheck, layout); return true; } } From c4d9730d6951087f5ddd76e9ddf1b285411ea5db Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Fri, 20 Jun 2025 11:47:51 -0400 Subject: [PATCH 5/7] whitespace --- src/cgutils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 44f44dc6050f8..ef36eeb728689 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -4710,7 +4710,7 @@ static jl_cgval_t emit_memoryref_direct(jl_codectx_t &ctx, const jl_cgval_t &mem ctx.builder.SetInsertPoint(endBB); } Value *data; - + if ((!isboxed && isunion) || isghost) { data = idx0; From 83146f8d2946d425a387de69ae3a50d41206886c Mon Sep 17 00:00:00 2001 From: oscarddssmith Date: Fri, 11 Jul 2025 14:21:32 -0400 Subject: [PATCH 6/7] bump expected precompile because Jameson told me to. --- stdlib/REPL/test/precompilation.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stdlib/REPL/test/precompilation.jl b/stdlib/REPL/test/precompilation.jl index 7efcf0b5e8282..fcd6f9aa1ae2b 100644 --- a/stdlib/REPL/test/precompilation.jl +++ b/stdlib/REPL/test/precompilation.jl @@ -33,7 +33,8 @@ if !Sys.iswindows() # given this test checks that startup is snappy, it's best to add workloads to # contrib/generate_precompile.jl rather than increase this number. But if that's not # possible, it'd be helpful to add a comment with the statement and a reason below - expected_precompiles = 0 + expected_precompiles = 1 + # Jameson told me to bump this n_precompiles = count(r"precompile\(", tracecompile_out) From db8ddf12323a49fa8eb71dd987da9491ac6d9275 Mon Sep 17 00:00:00 2001 From: oscarddssmith Date: Fri, 11 Jul 2025 17:41:39 -0400 Subject: [PATCH 7/7] fix minor ub --- src/builtins.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/builtins.c b/src/builtins.c index c1352b8cf738b..79953001662fd 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1768,9 +1768,9 @@ JL_CALLABLE(jl_f_memoryrefnew) JL_TYPECHK(memoryrefnew, long, args[1]); if (nargs == 3) JL_TYPECHK(memoryrefnew, bool, args[2]); - size_t i = jl_unbox_long(args[1]) - 1; + size_t i = (size_t) jl_unbox_long(args[1]) - 1; char *data; - if(jl_is_genericmemory(args[0])) { + if (jl_is_genericmemory(args[0])) { jl_genericmemory_t *m = (jl_genericmemory_t*)args[0]; jl_value_t *typ = jl_apply_type((jl_value_t*)jl_genericmemoryref_type, jl_svec_data(((jl_datatype_t*)jl_typetagof(m))->parameters), 3); JL_GC_PROMISE_ROOTED(typ); // it is a concrete type