Skip to content

Commit

Permalink
codegen: complete handling for partial-layout objects (JuliaLang#41438)
Browse files Browse the repository at this point in the history
  • Loading branch information
vtjnash authored and johanmon committed Jul 5, 2021
1 parent f49a7ae commit f300dd3
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ static Value *julia_to_native(
assert(!byRef); // don't expect any ABI to pass pointers by pointer
return boxed(ctx, jvinfo);
}
assert(jl_is_datatype(jlto) && julia_struct_has_layout((jl_datatype_t*)jlto));
assert(jl_is_datatype(jlto) && jl_struct_try_layout((jl_datatype_t*)jlto));

typeassert_input(ctx, jvinfo, jlto, jlto_env, argn);
if (!byRef)
Expand Down
20 changes: 7 additions & 13 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ static size_t dereferenceable_size(jl_value_t *jt)
// Array has at least this much data
return sizeof(jl_array_t);
}
else if (jl_is_datatype(jt) && ((jl_datatype_t*)jt)->layout) {
else if (jl_is_datatype(jt) && jl_struct_try_layout((jl_datatype_t*)jt)) {
return jl_datatype_size(jt);
}
return 0;
Expand All @@ -339,7 +339,7 @@ static unsigned julia_alignment(jl_value_t *jt)
// and this is the guarantee we have for the GC bits
return 16;
}
assert(jl_is_datatype(jt) && ((jl_datatype_t*)jt)->layout);
assert(jl_is_datatype(jt) && jl_struct_try_layout((jl_datatype_t*)jt));
unsigned alignment = jl_datatype_align(jt);
if (alignment > JL_HEAP_ALIGNMENT)
return JL_HEAP_ALIGNMENT;
Expand Down Expand Up @@ -555,16 +555,10 @@ static Type *bitstype_to_llvm(jl_value_t *bt, bool llvmcall = false)
}

static bool jl_type_hasptr(jl_value_t* typ)
{ // assumes that jl_stored_inline(typ) is true
{ // assumes that jl_stored_inline(typ) is true (and therefore that layout is defined)
return jl_is_datatype(typ) && ((jl_datatype_t*)typ)->layout->npointers > 0;
}

// return whether all concrete subtypes of this type have the same layout
static bool julia_struct_has_layout(jl_datatype_t *dt)
{
return dt->layout || jl_has_fixed_layout(dt);
}

static unsigned jl_field_align(jl_datatype_t *dt, size_t i)
{
unsigned al = jl_field_offset(dt, i);
Expand All @@ -588,11 +582,8 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, boo
bool isTuple = jl_is_tuple_type(jt);
jl_svec_t *ftypes = jl_get_fieldtypes(jst);
size_t i, ntypes = jl_svec_len(ftypes);
if (!julia_struct_has_layout(jst))
if (!jl_struct_try_layout(jst))
return NULL; // caller should have checked jl_type_mappable_to_c already, but we'll be nice
if (jst->layout == NULL)
jl_compute_field_offsets(jst);
assert(jst->layout);
if (ntypes == 0 || jl_datatype_nbits(jst) == 0)
return T_void;
Type *_struct_decl = NULL;
Expand Down Expand Up @@ -1904,6 +1895,9 @@ static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
return true;
}
if (nfields == 1) {
if (jl_has_free_typevars(jl_field_type(stt, 0))) {
return false;
}
(void)idx0();
*ret = emit_getfield_knownidx(ctx, strct, 0, stt, order);
return true;
Expand Down
22 changes: 11 additions & 11 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -933,12 +933,12 @@ static MDNode *best_tbaa(jl_value_t *jt) {
// note that this includes jl_isbits, although codegen should work regardless
static bool jl_is_concrete_immutable(jl_value_t* t)
{
return jl_is_immutable_datatype(t) && ((jl_datatype_t*)t)->layout;
return jl_is_immutable_datatype(t) && ((jl_datatype_t*)t)->isconcretetype;
}

static bool jl_is_pointerfree(jl_value_t* t)
{
if (!jl_is_immutable_datatype(t))
if (!jl_is_concrete_immutable(t))
return 0;
const jl_datatype_layout_t *layout = ((jl_datatype_t*)t)->layout;
return layout && layout->npointers == 0;
Expand Down Expand Up @@ -3033,9 +3033,9 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
return true;
}

if (jl_is_datatype(utt) && utt->layout) {
if (jl_is_datatype(utt) && jl_struct_try_layout(utt)) {
ssize_t idx = jl_field_index(utt, name, 0);
if (idx != -1) {
if (idx != -1 && !jl_has_free_typevars(jl_field_type(utt, idx))) {
*ret = emit_getfield_knownidx(ctx, obj, idx, utt, order);
return true;
}
Expand Down Expand Up @@ -3063,14 +3063,16 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
}

if (jl_is_datatype(utt)) {
if (jl_is_structtype(utt) && utt->layout) {
if (jl_struct_try_layout(utt)) {
size_t nfields = jl_datatype_nfields(utt);
// integer index
size_t idx;
if (fld.constant && (idx = jl_unbox_long(fld.constant) - 1) < nfields) {
// known index
*ret = emit_getfield_knownidx(ctx, obj, idx, utt, order);
return true;
if (!jl_has_free_typevars(jl_field_type(utt, idx))) {
// known index
*ret = emit_getfield_knownidx(ctx, obj, idx, utt, order);
return true;
}
}
else {
// unknown index
Expand Down Expand Up @@ -3115,8 +3117,6 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
}
}
}
// TODO: attempt better codegen for approximate types, if the types
// and offsets of some fields are independent of parameters.
// TODO: generic getfield func with more efficient calling convention
return false;
}
Expand Down Expand Up @@ -3155,7 +3155,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
}

jl_datatype_t *uty = (jl_datatype_t*)jl_unwrap_unionall(obj.typ);
if (jl_is_structtype(uty) && uty->layout) {
if (jl_is_datatype(uty) && jl_struct_try_layout(uty)) {
ssize_t idx = -1;
if (fld.constant && fld.typ == (jl_value_t*)jl_symbol_type) {
idx = jl_field_index(uty, (jl_sym_t*)fld.constant, 0);
Expand Down
41 changes: 26 additions & 15 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -220,27 +220,36 @@ unsigned jl_special_vector_alignment(size_t nfields, jl_value_t *t)
return next_power_of_two(size);
}

STATIC_INLINE int jl_is_datatype_make_singleton(jl_datatype_t *d)
STATIC_INLINE int jl_is_datatype_make_singleton(jl_datatype_t *d) JL_NOTSAFEPOINT
{
return (!d->name->abstract && jl_datatype_size(d) == 0 && d != jl_symbol_type && d->name != jl_array_typename &&
d->isconcretetype && !d->name->mutabl);
}

STATIC_INLINE void jl_maybe_allocate_singleton_instance(jl_datatype_t *st)
STATIC_INLINE void jl_maybe_allocate_singleton_instance(jl_datatype_t *st) JL_NOTSAFEPOINT
{
if (jl_is_datatype_make_singleton(st)) {
// It's possible for st to already have an ->instance if it was redefined
if (!st->instance) {
jl_task_t *ct = jl_current_task;
st->instance = jl_gc_alloc(ct->ptls, 0, st);
jl_gc_wb(st, st->instance);
}
if (!st->instance)
st->instance = jl_gc_permobj(0, st);
}
}

// return whether all concrete subtypes of this type have the same layout
int jl_struct_try_layout(jl_datatype_t *dt)
{
if (dt->layout)
return 1;
else if (!jl_has_fixed_layout(dt))
return 0;
jl_compute_field_offsets(dt);
assert(dt->layout);
return 1;
}

int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree) JL_NOTSAFEPOINT
{
if (ty->name->mayinlinealloc && ty->layout) {
if (ty->name->mayinlinealloc && (ty->isconcretetype || ((jl_datatype_t*)jl_unwrap_unionall(ty->name->wrapper))->layout)) { // TODO: use jl_struct_try_layout(dt) (but it is a safepoint)
if (ty->layout->npointers > 0) {
if (pointerfree)
return 0;
Expand Down Expand Up @@ -348,28 +357,31 @@ void jl_compute_field_offsets(jl_datatype_t *st)
if (st->name->wrapper == NULL)
return; // we got called too early--we'll be back
jl_datatype_t *w = (jl_datatype_t*)jl_unwrap_unionall(st->name->wrapper);
assert(st->types && w->types);
size_t i, nfields = jl_svec_len(st->types);
assert(st->name->n_uninitialized <= nfields);
if (st == w && st->layout) {
// this check allows us to force re-computation of the layout for some types during init
st->layout = NULL;
st->size = 0;
st->zeroinit = 0;
st->has_concrete_subtype = 1;
}
int isbitstype = st->isconcretetype && st->name->mayinlinealloc;
// If layout doesn't depend on type parameters, it's stored in st->name->wrapper
// and reused by all subtypes.
if (w->layout) {
st->layout = w->layout;
st->size = w->size;
st->zeroinit = w->zeroinit;
st->has_concrete_subtype = w->has_concrete_subtype;
if (jl_is_layout_opaque(st->layout)) { // e.g. jl_array_typename
return;
if (!jl_is_layout_opaque(st->layout)) { // e.g. jl_array_typename
st->isbitstype = isbitstype && st->layout->npointers == 0;
jl_maybe_allocate_singleton_instance(st);
}
return;
}
else if (nfields == 0) {
assert(st->types && w->types);
size_t i, nfields = jl_svec_len(st->types);
assert(st->name->n_uninitialized <= nfields);
if (nfields == 0) {
// if we have no fields, we can trivially skip the rest
if (st == jl_symbol_type || st == jl_string_type) {
// opaque layout - heap-allocated blob
Expand Down Expand Up @@ -404,7 +416,6 @@ void jl_compute_field_offsets(jl_datatype_t *st)
}
}

int isbitstype = st->isconcretetype && st->name->mayinlinealloc;
for (i = 0; isbitstype && i < nfields; i++) {
jl_value_t *fld = jl_field_type(st, i);
isbitstype = jl_isbits(fld);
Expand Down
28 changes: 14 additions & 14 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@ int jl_has_fixed_layout(jl_datatype_t *dt)
{
if (dt->layout || dt->isconcretetype)
return 1;
if (jl_is_tuple_type(dt))
if (dt->name->abstract)
return 0;
if (jl_is_tuple_type(dt) || jl_is_namedtuple_type(dt))
return 0; // TODO: relax more?
jl_svec_t *types = jl_get_fieldtypes(dt);
size_t i, l = jl_svec_len(types);
Expand All @@ -242,9 +244,10 @@ int jl_type_mappable_to_c(jl_value_t *ty)
assert(!jl_is_typevar(ty) && jl_is_type(ty));
if (jl_is_structtype(ty)) {
jl_datatype_t *jst = (jl_datatype_t*)ty;
return jst->layout || jl_has_fixed_layout(jst);
return jl_has_fixed_layout(jst);
}
if (jl_is_tuple_type(jl_unwrap_unionall(ty)))
ty = jl_unwrap_unionall(ty);
if (jl_is_tuple_type(ty) || jl_is_namedtuple_type(ty))
return 0; // TODO: relax some?
return 1; // as boxed or primitive
}
Expand Down Expand Up @@ -1437,7 +1440,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
jl_gc_wb(ndt, ndt->parameters);
ndt->types = NULL; // to be filled in below
if (istuple) {
ndt->types = p;
ndt->types = p; // TODO: this may need to filter out certain types
}
else if (isnamedtuple) {
jl_value_t *names_tup = jl_svecref(p, 0);
Expand All @@ -1463,19 +1466,16 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
jl_gc_wb(ndt, ndt->types);
}
else {
ndt->types = jl_emptysvec;
ndt->types = jl_emptysvec; // XXX: this is essentially always false
}
}
ndt->size = 0;
jl_precompute_memoized_dt(ndt, cacheable);

if (jl_is_primitivetype(dt)) {
ndt->size = dt->size;
ndt->layout = dt->layout;
ndt->isbitstype = ndt->isconcretetype;
}

jl_datatype_t *primarydt = ((jl_datatype_t*)jl_unwrap_unionall(tn->wrapper));
jl_precompute_memoized_dt(ndt, cacheable);
ndt->size = 0;
if (primarydt->layout)
jl_compute_field_offsets(ndt);

if (istuple || isnamedtuple) {
ndt->super = jl_any_type;
}
Expand Down Expand Up @@ -1516,7 +1516,7 @@ static jl_value_t *inst_datatype_inner(jl_datatype_t *dt, jl_svec_t *p, jl_value
// leading to incorrect layouts and data races (#40050: the A{T} should be
// an isbitstype singleton of size 0)
if (cacheable) {
if (!jl_is_primitivetype(dt) && ndt->types != NULL && ndt->isconcretetype) {
if (dt->layout == NULL && !jl_is_primitivetype(dt) && ndt->types != NULL && ndt->isconcretetype) {
jl_compute_field_offsets(ndt);
}
jl_cache_type_(ndt);
Expand Down
3 changes: 2 additions & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ STATIC_INLINE jl_gc_tracked_buffer_t *jl_gc_alloc_buf(jl_ptls_t ptls, size_t sz)
return jl_gc_alloc(ptls, sz, (void*)jl_buff_tag);
}

STATIC_INLINE jl_value_t *jl_gc_permobj(size_t sz, void *ty)
STATIC_INLINE jl_value_t *jl_gc_permobj(size_t sz, void *ty) JL_NOTSAFEPOINT
{
const size_t allocsz = sz + sizeof(jl_taggedvalue_t);
unsigned align = (sz == 0 ? sizeof(void*) : (allocsz <= sizeof(void*) * 2 ?
Expand Down Expand Up @@ -539,6 +539,7 @@ jl_datatype_t *jl_mk_builtin_func(jl_datatype_t *dt, const char *name, jl_fptr_a
int jl_obviously_unequal(jl_value_t *a, jl_value_t *b);
JL_DLLEXPORT jl_array_t *jl_find_free_typevars(jl_value_t *v);
int jl_has_fixed_layout(jl_datatype_t *t);
int jl_struct_try_layout(jl_datatype_t *dt);
int jl_type_mappable_to_c(jl_value_t *ty);
jl_svec_t *jl_outer_unionall_vars(jl_value_t *u);
jl_value_t *jl_type_intersection_env_s(jl_value_t *a, jl_value_t *b, jl_svec_t **penv, int *issubty);
Expand Down
13 changes: 13 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -580,3 +580,16 @@ g40612(a, b) = a[]|a[] === b[]|b[]
@test g40612(Union{Bool,Missing}[missing], Union{Bool,Missing}[missing])
@test g40612(Union{Bool,Missing}[true], Union{Bool,Missing}[true])
@test g40612(Union{Bool,Missing}[false], Union{Bool,Missing}[false])

# issue #41438
struct A41438{T}
x::Ptr{T}
end
struct B41438{T}
x::T
end
f41438(y) = y[].x
@test A41438.body.layout != C_NULL
@test B41438.body.layout === C_NULL
@test f41438(Ref{A41438}(A41438(C_NULL))) === C_NULL
@test f41438(Ref{B41438}(B41438(C_NULL))) === C_NULL

0 comments on commit f300dd3

Please sign in to comment.