codegen: complete handling for partial-layout objects

vtjnash · vtjnash · commit c545eab1254a · 2021-07-01T16:01:05.000-04:00
Fixes #41425
diff --git a/src/ccall.cpp b/src/ccall.cpp
@@ -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)
diff --git a/src/cgutils.cpp b/src/cgutils.cpp
@@ -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;
@@ -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;
@@ -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);
@@ -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;
@@ -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;
diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -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;
@@ -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;
                 }
@@ -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
@@ -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;
     }
@@ -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);
diff --git a/src/datatype.c b/src/datatype.c
@@ -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;
diff --git a/src/jltypes.c b/src/jltypes.c
@@ -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);
@@ -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
 }
@@ -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);
@@ -1463,7 +1466,7 @@ 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;
diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -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 ?
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -493,7 +493,7 @@ static Value *julia_to_native(`
`493`	`493`	`assert(!byRef); // don't expect any ABI to pass pointers by pointer`
`494`	`494`	`return boxed(ctx, jvinfo);`
`495`	`495`	`}`
`496`		`- assert(jl_is_datatype(jlto) && julia_struct_has_layout((jl_datatype_t*)jlto));`
	`496`	`+ assert(jl_is_datatype(jlto) && jl_struct_try_layout((jl_datatype_t*)jlto));`
`497`	`497`
`498`	`498`	`typeassert_input(ctx, jvinfo, jlto, jlto_env, argn);`
`499`	`499`	`if (!byRef)`
Original file line number	Diff line number	Diff line change
`@@ -933,12 +933,12 @@ static MDNode best_tbaa(jl_value_t jt) {`
`933`	`933`	`// note that this includes jl_isbits, although codegen should work regardless`
`934`	`934`	`static bool jl_is_concrete_immutable(jl_value_t* t)`
`935`	`935`	`{`
`936`		`- return jl_is_immutable_datatype(t) && ((jl_datatype_t*)t)->layout;`
	`936`	`+ return jl_is_immutable_datatype(t) && ((jl_datatype_t*)t)->isconcretetype;`
`937`	`937`	`}`
`938`	`938`
`939`	`939`	`static bool jl_is_pointerfree(jl_value_t* t)`
`940`	`940`	`{`
`941`		`- if (!jl_is_immutable_datatype(t))`
	`941`	`+ if (!jl_is_concrete_immutable(t))`
`942`	`942`	`return 0;`
`943`	`943`	`const jl_datatype_layout_t layout = ((jl_datatype_t)t)->layout;`
`944`	`944`	`return layout && layout->npointers == 0;`
`@@ -3033,9 +3033,9 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t ret, jl_value_t f,`
`3033`	`3033`	`return true;`
`3034`	`3034`	`}`
`3035`	`3035`
`3036`		`- if (jl_is_datatype(utt) && utt->layout) {`
	`3036`	`+ if (jl_is_datatype(utt) && jl_struct_try_layout(utt)) {`
`3037`	`3037`	`ssize_t idx = jl_field_index(utt, name, 0);`
`3038`		`- if (idx != -1) {`
	`3038`	`+ if (idx != -1 && !jl_has_free_typevars(jl_field_type(utt, idx))) {`
`3039`	`3039`	`*ret = emit_getfield_knownidx(ctx, obj, idx, utt, order);`
`3040`	`3040`	`return true;`
`3041`	`3041`	`}`
`@@ -3063,14 +3063,16 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t ret, jl_value_t f,`
`3063`	`3063`	`}`
`3064`	`3064`
`3065`	`3065`	`if (jl_is_datatype(utt)) {`
`3066`		`- if (jl_is_structtype(utt) && utt->layout) {`
	`3066`	`+ if (jl_struct_try_layout(utt)) {`
`3067`	`3067`	`size_t nfields = jl_datatype_nfields(utt);`
`3068`	`3068`	`// integer index`
`3069`	`3069`	`size_t idx;`
`3070`	`3070`	`if (fld.constant && (idx = jl_unbox_long(fld.constant) - 1) < nfields) {`
`3071`		`- // known index`
`3072`		`- *ret = emit_getfield_knownidx(ctx, obj, idx, utt, order);`
`3073`		`- return true;`
	`3071`	`+ if (!jl_has_free_typevars(jl_field_type(utt, idx))) {`
	`3072`	`+ // known index`
	`3073`	`+ *ret = emit_getfield_knownidx(ctx, obj, idx, utt, order);`
	`3074`	`+ return true;`
	`3075`	`+ }`
`3074`	`3076`	`}`
`3075`	`3077`	`else {`
`3076`	`3078`	`// unknown index`
`@@ -3115,8 +3117,6 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t ret, jl_value_t f,`
`3115`	`3117`	`}`
`3116`	`3118`	`}`
`3117`	`3119`	`}`
`3118`		`- // TODO: attempt better codegen for approximate types, if the types`
`3119`		`- // and offsets of some fields are independent of parameters.`
`3120`	`3120`	`// TODO: generic getfield func with more efficient calling convention`
`3121`	`3121`	`return false;`
`3122`	`3122`	`}`
`@@ -3155,7 +3155,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t ret, jl_value_t f,`
`3155`	`3155`	`}`
`3156`	`3156`
`3157`	`3157`	`jl_datatype_t uty = (jl_datatype_t)jl_unwrap_unionall(obj.typ);`
`3158`		`- if (jl_is_structtype(uty) && uty->layout) {`
	`3158`	`+ if (jl_is_datatype(uty) && jl_struct_try_layout(uty)) {`
`3159`	`3159`	`ssize_t idx = -1;`
`3160`	`3160`	`if (fld.constant && fld.typ == (jl_value_t*)jl_symbol_type) {`
`3161`	`3161`	`idx = jl_field_index(uty, (jl_sym_t*)fld.constant, 0);`
Original file line number	Diff line number	Diff line change
`@@ -223,7 +223,9 @@ int jl_has_fixed_layout(jl_datatype_t *dt)`
`223`	`223`	`{`
`224`	`224`	`if (dt->layout \|\| dt->isconcretetype)`
`225`	`225`	`return 1;`
`226`		`- if (jl_is_tuple_type(dt))`
	`226`	`+ if (dt->name->abstract)`
	`227`	`+ return 0;`
	`228`	`+ if (jl_is_tuple_type(dt) \|\| jl_is_namedtuple_type(dt))`
`227`	`229`	`return 0; // TODO: relax more?`
`228`	`230`	`jl_svec_t *types = jl_get_fieldtypes(dt);`
`229`	`231`	`size_t i, l = jl_svec_len(types);`
`@@ -242,9 +244,10 @@ int jl_type_mappable_to_c(jl_value_t *ty)`
`242`	`244`	`assert(!jl_is_typevar(ty) && jl_is_type(ty));`
`243`	`245`	`if (jl_is_structtype(ty)) {`
`244`	`246`	`jl_datatype_t jst = (jl_datatype_t)ty;`
`245`		`- return jst->layout \|\| jl_has_fixed_layout(jst);`
	`247`	`+ return jl_has_fixed_layout(jst);`
`246`	`248`	`}`
`247`		`- if (jl_is_tuple_type(jl_unwrap_unionall(ty)))`
	`249`	`+ ty = jl_unwrap_unionall(ty);`
	`250`	`+ if (jl_is_tuple_type(ty) \|\| jl_is_namedtuple_type(ty))`
`248`	`251`	`return 0; // TODO: relax some?`
`249`	`252`	`return 1; // as boxed or primitive`
`250`	`253`	`}`
`@@ -1437,7 +1440,7 @@ static jl_value_t inst_datatype_inner(jl_datatype_t dt, jl_svec_t *p, jl_value`
`1437`	`1440`	`jl_gc_wb(ndt, ndt->parameters);`
`1438`	`1441`	`ndt->types = NULL; // to be filled in below`
`1439`	`1442`	`if (istuple) {`
`1440`		`- ndt->types = p;`
	`1443`	`+ ndt->types = p; // TODO: this may need to filter out certain types`
`1441`	`1444`	`}`
`1442`	`1445`	`else if (isnamedtuple) {`
`1443`	`1446`	`jl_value_t *names_tup = jl_svecref(p, 0);`
`@@ -1463,7 +1466,7 @@ static jl_value_t inst_datatype_inner(jl_datatype_t dt, jl_svec_t *p, jl_value`
`1463`	`1466`	`jl_gc_wb(ndt, ndt->types);`
`1464`	`1467`	`}`
`1465`	`1468`	`else {`
`1466`		`- ndt->types = jl_emptysvec;`
	`1469`	`+ ndt->types = jl_emptysvec; // XXX: this is essentially always false`
`1467`	`1470`	`}`
`1468`	`1471`	`}`
`1469`	`1472`	`ndt->size = 0;`