Skip to content

Commit

Permalink
remove svec(#null) from the language (#46975)
Browse files Browse the repository at this point in the history
This was not supported, and almost never used, so audit all places it
could happen and avoid it at the source.

Fix #46784

Co-authored-by: Keno Fischer <keno@juliacomputing.com>
  • Loading branch information
vtjnash and Keno authored Oct 3, 2022
1 parent e34860e commit 33afb81
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 70 deletions.
7 changes: 5 additions & 2 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,10 @@ end

@eval getindex(v::SimpleVector, i::Int) = Core._svec_ref($(Expr(:boundscheck)), v, i)
function length(v::SimpleVector)
return ccall(:jl_svec_len, Int, (Any,), v)
t = @_gc_preserve_begin v
len = unsafe_load(Ptr{Int}(pointer_from_objref(v)))
@_gc_preserve_end t
return len
end
firstindex(v::SimpleVector) = 1
lastindex(v::SimpleVector) = length(v)
Expand Down Expand Up @@ -745,7 +748,7 @@ function isassigned end

function isassigned(v::SimpleVector, i::Int)
@boundscheck 1 <= i <= length(v) || return false
return ccall(:jl_svec_isassigned, Bool, (Any, Int), v, i - 1)
return true
end


Expand Down
2 changes: 1 addition & 1 deletion src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ STATIC_INLINE void _grow_to(jl_value_t **root, jl_value_t ***oldargs, jl_svec_t
*n_alloc = newalloc;
}

static jl_value_t *do_apply( jl_value_t **args, uint32_t nargs, jl_value_t *iterate)
static jl_value_t *do_apply(jl_value_t **args, uint32_t nargs, jl_value_t *iterate)
{
jl_function_t *f = args[0];
if (nargs == 2) {
Expand Down
24 changes: 0 additions & 24 deletions src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ TRANSFORMED_CCALL_STAT(jl_set_next_task);
TRANSFORMED_CCALL_STAT(jl_sigatomic_begin);
TRANSFORMED_CCALL_STAT(jl_sigatomic_end);
TRANSFORMED_CCALL_STAT(jl_svec_len);
TRANSFORMED_CCALL_STAT(jl_svec_isassigned);
TRANSFORMED_CCALL_STAT(jl_svec_ref);
TRANSFORMED_CCALL_STAT(jl_array_isassigned);
TRANSFORMED_CCALL_STAT(jl_string_ptr);
Expand Down Expand Up @@ -1687,28 +1686,6 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
JL_GC_POP();
return mark_or_box_ccall_result(ctx, len, retboxed, rt, unionall, static_rt);
}
else if (is_libjulia_func(jl_svec_isassigned) &&
argv[1].typ == (jl_value_t*)jl_long_type) {
++CCALL_STAT(jl_svec_isassigned);
assert(!isVa && !llvmcall && nccallargs == 2);
const jl_cgval_t &svecv = argv[0];
const jl_cgval_t &idxv = argv[1];
Value *idx = emit_unbox(ctx, getSizeTy(ctx.builder.getContext()), idxv, (jl_value_t*)jl_long_type);
idx = ctx.builder.CreateAdd(idx, ConstantInt::get(getSizeTy(ctx.builder.getContext()), 1));
auto ptr = emit_bitcast(ctx, boxed(ctx, svecv), ctx.types().T_pprjlvalue);
Value *slot_addr = ctx.builder.CreateInBoundsGEP(ctx.types().T_prjlvalue,
decay_derived(ctx, ptr), idx);
LoadInst *load = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, slot_addr,
Align(sizeof(void*)));
load->setAtomic(AtomicOrdering::Unordered);
// Only mark with TBAA if we are sure about the type.
// This could otherwise be in a dead branch
if (svecv.typ == (jl_value_t*)jl_simplevector_type)
tbaa_decorate(ctx.tbaa().tbaa_const, load);
Value *res = ctx.builder.CreateZExt(ctx.builder.CreateICmpNE(load, Constant::getNullValue(ctx.types().T_prjlvalue)), getInt8Ty(ctx.builder.getContext()));
JL_GC_POP();
return mark_or_box_ccall_result(ctx, res, retboxed, rt, unionall, static_rt);
}
else if (is_libjulia_func(jl_svec_ref) && argv[1].typ == (jl_value_t*)jl_long_type) {
++CCALL_STAT(jl_svec_ref);
assert(lrt == ctx.types().T_prjlvalue);
Expand All @@ -1727,7 +1704,6 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
// This could otherwise be in a dead branch
if (svecv.typ == (jl_value_t*)jl_simplevector_type)
tbaa_decorate(ctx.tbaa().tbaa_const, load);
null_pointer_check(ctx, load);
JL_GC_POP();
return mark_or_box_ccall_result(ctx, load, retboxed, rt, unionall, static_rt);
}
Expand Down
1 change: 1 addition & 0 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,7 @@ static jl_method_instance_t *cache_method(
// NULL, jl_emptysvec, /*guard*/NULL, jl_cachearg_offset(mt), other->min_world, other->max_world);
}
}
assert(guards == jl_svec_len(guardsigs));
}
if (!cache_with_orig) {
// determined above that there's no ambiguity in also using compilationsig as the cacheablesig
Expand Down
1 change: 0 additions & 1 deletion src/jl_exported_funcs.inc
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,6 @@
XX(jl_svec2) \
XX(jl_svec_copy) \
XX(jl_svec_fill) \
XX(jl_svec_isassigned) \
XX(jl_svec_ref) \
XX(jl_switch) \
XX(jl_switchto) \
Expand Down
28 changes: 13 additions & 15 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,9 @@ static jl_datatype_t *lookup_type_set(jl_svec_t *cache, jl_value_t **key, size_t
size_t iter = 0;
do {
jl_datatype_t *val = jl_atomic_load_relaxed(&tab[index]);
if (val == NULL)
if ((jl_value_t*)val == jl_nothing)
return NULL;
if ((jl_value_t*)val != jl_nothing && val->hash == hv && typekey_eq(val, key, n))
if (val->hash == hv && typekey_eq(val, key, n))
return val;
index = (index + 1) & (sz - 1);
iter++;
Expand All @@ -680,9 +680,9 @@ static jl_datatype_t *lookup_type_setvalue(jl_svec_t *cache, jl_value_t *key1, j
size_t iter = 0;
do {
jl_datatype_t *val = jl_atomic_load_relaxed(&tab[index]);
if (val == NULL)
if ((jl_value_t*)val == jl_nothing)
return NULL;
if ((jl_value_t*)val != jl_nothing && val->hash == hv && typekeyvalue_eq(val, key1, key, n, leaf))
if (val->hash == hv && typekeyvalue_eq(val, key1, key, n, leaf))
return val;
index = (index + 1) & (sz - 1);
iter++;
Expand All @@ -702,7 +702,7 @@ static ssize_t lookup_type_idx_linear(jl_svec_t *cache, jl_value_t **key, size_t
ssize_t i;
for (i = 0; i < cl; i++) {
jl_datatype_t *tt = jl_atomic_load_relaxed(&data[i]);
if (tt == NULL)
if ((jl_value_t*)tt == jl_nothing)
return ~i;
if (typekey_eq(tt, key, n))
return i;
Expand All @@ -719,7 +719,7 @@ static ssize_t lookup_type_idx_linearvalue(jl_svec_t *cache, jl_value_t *key1, j
ssize_t i;
for (i = 0; i < cl; i++) {
jl_datatype_t *tt = jl_atomic_load_relaxed(&data[i]);
if (tt == NULL)
if ((jl_value_t*)tt == jl_nothing)
return ~i;
if (typekeyvalue_eq(tt, key1, key, n, 1))
return i;
Expand Down Expand Up @@ -777,7 +777,7 @@ static int cache_insert_type_set_(jl_svec_t *a, jl_datatype_t *val, uint_t hv, i
size_t maxprobe = max_probe(sz);
do {
jl_value_t *tab_i = jl_atomic_load_relaxed(&tab[index]);
if (tab_i == NULL || tab_i == jl_nothing) {
if (tab_i == jl_nothing) {
if (atomic)
jl_atomic_store_release(&tab[index], (jl_value_t*)val);
else
Expand All @@ -792,8 +792,6 @@ static int cache_insert_type_set_(jl_svec_t *a, jl_datatype_t *val, uint_t hv, i
return 0;
}

static jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz);

static void cache_insert_type_set(jl_datatype_t *val, uint_t hv)
{
jl_svec_t *a = jl_atomic_load_relaxed(&val->name->cache);
Expand All @@ -820,17 +818,17 @@ static void cache_insert_type_set(jl_datatype_t *val, uint_t hv)
}
}

static jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz)
jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz)
{
jl_value_t **ol = jl_svec_data(a);
size_t sz = jl_svec_len(a);
while (1) {
size_t i;
jl_svec_t *newa = jl_alloc_svec(newsz);
jl_svec_t *newa = jl_svec_fill(newsz, jl_nothing);
JL_GC_PUSH1(&newa);
for (i = 0; i < sz; i += 1) {
jl_value_t *val = ol[i];
if (val != NULL && val != jl_nothing) {
if (val != jl_nothing) {
uint_t hv = ((jl_datatype_t*)val)->hash;
if (!cache_insert_type_set_(newa, (jl_datatype_t*)val, hv, 0)) {
break;
Expand All @@ -849,15 +847,15 @@ static void cache_insert_type_linear(jl_datatype_t *type, ssize_t insert_at)
jl_svec_t *cache = jl_atomic_load_relaxed(&type->name->linearcache);
assert(jl_is_svec(cache));
size_t n = jl_svec_len(cache);
if (n == 0 || jl_svecref(cache, n - 1) != NULL) {
jl_svec_t *nc = jl_alloc_svec(n < 8 ? 8 : (n*3)>>1);
if (n == 0 || jl_svecref(cache, n - 1) != jl_nothing) {
jl_svec_t *nc = jl_svec_fill(n < 4 ? 4 : n * 2, jl_nothing);
memcpy(jl_svec_data(nc), jl_svec_data(cache), sizeof(void*) * n);
jl_atomic_store_release(&type->name->linearcache, nc);
jl_gc_wb(type->name, nc);
cache = nc;
n = jl_svec_len(nc);
}
assert(jl_svecref(cache, insert_at) == NULL);
assert(jl_svecref(cache, insert_at) == jl_nothing);
jl_svecset(cache, insert_at, (jl_value_t*)type); // todo: make this an atomic-store
}

Expand Down
2 changes: 1 addition & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ jl_vararg_t *jl_wrap_vararg(jl_value_t *t, jl_value_t *n);
void jl_reinstantiate_inner_types(jl_datatype_t *t);
jl_datatype_t *jl_lookup_cache_type_(jl_datatype_t *type);
void jl_cache_type_(jl_datatype_t *type);
jl_svec_t *cache_rehash_set(jl_svec_t *a, size_t newsz);
void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic) JL_NOTSAFEPOINT;
jl_value_t *swap_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs, int isatomic);
jl_value_t *modify_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *op, jl_value_t *rhs, int isatomic);
Expand Down Expand Up @@ -1430,7 +1431,6 @@ int jl_typemap_intersection_visitor(jl_typemap_t *a, int offs, struct typemap_in

// For codegen only.
JL_DLLEXPORT size_t (jl_svec_len)(jl_svec_t *t) JL_NOTSAFEPOINT;
JL_DLLEXPORT int8_t jl_svec_isassigned(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i) JL_NOTSAFEPOINT;
JL_DLLEXPORT jl_value_t *jl_svec_ref(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i);


Expand Down
8 changes: 1 addition & 7 deletions src/simplevector.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,9 @@ JL_DLLEXPORT size_t (jl_svec_len)(jl_svec_t *t) JL_NOTSAFEPOINT
return jl_svec_len(t);
}

JL_DLLEXPORT int8_t jl_svec_isassigned(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i) JL_NOTSAFEPOINT
{
return jl_svecref(t, (size_t)i) != NULL;
}

JL_DLLEXPORT jl_value_t *jl_svec_ref(jl_svec_t *t JL_PROPAGATES_ROOT, ssize_t i)
{
jl_value_t *v = jl_svecref(t, (size_t)i);
if (__unlikely(v == NULL))
jl_throw(jl_undefref_exception);
assert(v != NULL);
return v;
}
21 changes: 13 additions & 8 deletions src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -1646,7 +1646,7 @@ static void jl_scan_type_cache_gv(jl_serializer_state *s, jl_svec_t *cache)
size_t l = jl_svec_len(cache), i;
for (i = 0; i < l; i++) {
jl_value_t *ti = jl_svecref(cache, i);
if (ti == NULL || ti == jl_nothing)
if (ti == jl_nothing)
continue;
if (jl_get_llvm_gv(native_functions, ti)) {
jl_serialize_value(s, ti);
Expand All @@ -1660,31 +1660,35 @@ static void jl_scan_type_cache_gv(jl_serializer_state *s, jl_svec_t *cache)
}

// remove cached types not referenced in the stream
static void jl_prune_type_cache_hash(jl_svec_t *cache)
static jl_svec_t *jl_prune_type_cache_hash(jl_svec_t *cache) JL_GC_DISABLED
{
size_t l = jl_svec_len(cache), i;
for (i = 0; i < l; i++) {
jl_value_t *ti = jl_svecref(cache, i);
if (ti == NULL || ti == jl_nothing)
if (ti == jl_nothing)
continue;
if (ptrhash_get(&backref_table, ti) == HT_NOTFOUND)
jl_svecset(cache, i, jl_nothing);
}
void *idx = ptrhash_get(&backref_table, cache);
ptrhash_remove(&backref_table, cache);
cache = cache_rehash_set(cache, l);
ptrhash_put(&backref_table, cache, idx);
return cache;
}

static void jl_prune_type_cache_linear(jl_svec_t *cache)
{
size_t l = jl_svec_len(cache), ins = 0, i;
for (i = 0; i < l; i++) {
jl_value_t *ti = jl_svecref(cache, i);
if (ti == NULL)
if (ti == jl_nothing)
break;
if (ptrhash_get(&backref_table, ti) != HT_NOTFOUND)
jl_svecset(cache, ins++, ti);
}
if (i > ins) {
memset(&jl_svec_data(cache)[ins], 0, (i - ins) * sizeof(jl_value_t*));
}
while (ins < l)
jl_svecset(cache, ins++, jl_nothing);
}

static jl_value_t *strip_codeinfo_meta(jl_method_t *m, jl_value_t *ci_, int orig)
Expand Down Expand Up @@ -1958,7 +1962,8 @@ static void jl_save_system_image_to_stream(ios_t *f) JL_GC_DISABLED
// built-in type caches
for (i = 0; i < typenames.len; i++) {
jl_typename_t *tn = (jl_typename_t*)typenames.items[i];
jl_prune_type_cache_hash(tn->cache);
tn->cache = jl_prune_type_cache_hash(tn->cache);
jl_gc_wb(tn, tn->cache);
jl_prune_type_cache_linear(tn->linearcache);
}
arraylist_free(&typenames);
Expand Down
25 changes: 21 additions & 4 deletions src/subtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -3547,17 +3547,26 @@ jl_value_t *jl_type_intersection_env_s(jl_value_t *a, jl_value_t *b, jl_svec_t *
}
}
if (sz == 0 && szb > 0) {
while (jl_is_unionall(b)) {
env[i++] = (jl_value_t*)((jl_unionall_t*)b)->var;
b = ((jl_unionall_t*)b)->body;
jl_unionall_t *ub = (jl_unionall_t*)b;
while (jl_is_unionall(ub)) {
env[i++] = (jl_value_t*)ub->var;
ub = (jl_unionall_t*)ub->body;
}
sz = szb;
}
if (penv) {
jl_svec_t *e = jl_alloc_svec(sz);
*penv = e;
for(i=0; i < sz; i++)
for (i = 0; i < sz; i++)
jl_svecset(e, i, env[i]);
jl_unionall_t *ub = (jl_unionall_t*)b;
for (i = 0; i < sz; i++) {
assert(jl_is_unionall(ub));
// TODO: assert(env[i] != NULL);
if (env[i] == NULL)
env[i] = (jl_value_t*)ub->var;
ub = (jl_unionall_t*)ub->body;
}
}
bot:
JL_GC_POP();
Expand Down Expand Up @@ -3601,6 +3610,14 @@ int jl_subtype_matching(jl_value_t *a, jl_value_t *b, jl_svec_t **penv)
*penv = e;
for (i = 0; i < szb; i++)
jl_svecset(e, i, env[i]);
jl_unionall_t *ub = (jl_unionall_t*)b;
for (i = 0; i < szb; i++) {
assert(jl_is_unionall(ub));
// TODO: assert(env[i] != NULL);
if (env[i] == NULL)
env[i] = (jl_value_t*)ub->var;
ub = (jl_unionall_t*)ub->body;
}
}
JL_GC_POP();
return sub;
Expand Down
2 changes: 1 addition & 1 deletion test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ end
mutable struct Type11167{T,N} end
function count11167()
let cache = Type11167.body.body.name.cache
return sum(i -> isassigned(cache, i), 0:length(cache))
return count(!isnothing, cache)
end
end
@test count11167() == 0
Expand Down
6 changes: 0 additions & 6 deletions test/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1275,12 +1275,6 @@ end
let repr = sprint(dump, Core.svec())
@test repr == "empty SimpleVector\n"
end
let sv = Core.svec(:a, :b, :c)
# unsafe replacement of :c with #undef to test handling of incomplete SimpleVectors
unsafe_store!(convert(Ptr{Ptr{Cvoid}}, Base.pointer_from_objref(sv)) + 3 * sizeof(Ptr), C_NULL)
repr = sprint(dump, sv)
@test repr == "SimpleVector\n 1: Symbol a\n 2: Symbol b\n 3: #undef\n"
end
let repr = sprint(dump, sin)
@test repr == "sin (function of type typeof(sin))\n"
end
Expand Down

0 comments on commit 33afb81

Please sign in to comment.