Skip to content

expand memoryrefnew capabilities #58768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Compiler/src/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a fun bugfix...

nargs= 3
elseif f === memoryrefget || f === memoryref_isassigned
nargs = 4
elseif f === memoryrefset!
nargs = 5
Expand Down
5 changes: 3 additions & 2 deletions Compiler/src/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -2107,7 +2108,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
Expand Down Expand Up @@ -2203,7 +2204,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)
Expand Down
9 changes: 6 additions & 3 deletions Compiler/test/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand Down
31 changes: 23 additions & 8 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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
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);
}
}

Expand Down
39 changes: 39 additions & 0 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4672,6 +4672,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()) {
Expand Down
17 changes: 13 additions & 4 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_direct(ctx, ref, argv[2], typ, boundscheck, layout);
return true;
}
}

else if (f == BUILTIN(memoryrefoffset) && nargs == 1) {
Expand Down