Skip to content

compute mayinlinealloc more accurately (for ccall) #40947

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 28, 2021
Merged

compute mayinlinealloc more accurately (for ccall) #40947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions src/builtins.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,8 @@ JL_CALLABLE(jl_f__typebody)
break;
}
}
if (!dt->name->mutabl && !dt->name->references_self)
dt->name->mayinlinealloc = 1;
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/ccall.cpp
View file
Open in desktop
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, jlto_env));
assert(jl_is_datatype(jlto) && julia_struct_has_layout((jl_datatype_t*)jlto));

typeassert_input(ctx, jvinfo, jlto, jlto_env, argn);
if (!byRef)
Expand Down Expand Up @@ -1065,7 +1065,7 @@ std::string generate_func_sig(const char *fname)
}
}

t = _julia_struct_to_llvm(ctx, tti, unionall_env, &isboxed, llvmcall);
t = _julia_struct_to_llvm(ctx, tti, &isboxed, llvmcall);
if (t == NULL || t == T_void) {
return make_errmsg(fname, i + 1, " doesn't correspond to a C type");
}
Expand Down Expand Up @@ -1211,7 +1211,7 @@ static const std::string verify_ccall_sig(jl_value_t *&rt, jl_value_t *at,
rt = (jl_value_t*)jl_any_type;
}

lrt = _julia_struct_to_llvm(ctx, rt, unionall_env, &retboxed, llvmcall);
lrt = _julia_struct_to_llvm(ctx, rt, &retboxed, llvmcall);
if (lrt == NULL)
return "return type doesn't correspond to a C type";

Expand Down Expand Up @@ -1405,7 +1405,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
isboxed = false;
}
else {
largty = _julia_struct_to_llvm(&ctx.emission_context, tti, unionall, &isboxed, llvmcall);
largty = _julia_struct_to_llvm(&ctx.emission_context, tti, &isboxed, llvmcall);
}
if (isboxed) {
ary = boxed(ctx, argv[0]);
Expand Down
65 changes: 21 additions & 44 deletions src/cgutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ static Value *emit_struct_gep(jl_codectx_t &ctx, Type *lty, Value *base, unsigne
return ctx.builder.CreateConstInBoundsGEP2_32(lty, base, 0, idx);
}

static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_unionall_t *ua, bool *isboxed, bool llvmcall=false);
static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, bool *isboxed, bool llvmcall=false);

static Type *_julia_type_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, bool *isboxed)
{
Expand All @@ -486,7 +486,7 @@ static Type *_julia_type_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, bool
if (jl_is_concrete_immutable(jt)) {
if (jl_datatype_nbits(jt) == 0)
return T_void;
Type *t = _julia_struct_to_llvm(ctx, jt, NULL, isboxed);
Type *t = _julia_struct_to_llvm(ctx, jt, isboxed);
assert(t != NULL);
return t;
}
Expand Down Expand Up @@ -542,23 +542,10 @@ static bool jl_type_hasptr(jl_value_t* typ)
return jl_is_datatype(typ) && ((jl_datatype_t*)typ)->layout->npointers > 0;
}

// compute whether all concrete subtypes of this type have the same layout
// (which is conservatively approximated here by asking whether the types of any of the
// fields depend on any of the parameters of the containing type)
static bool julia_struct_has_layout(jl_datatype_t *dt, jl_unionall_t *ua)
// return whether all concrete subtypes of this type have the same layout
static bool julia_struct_has_layout(jl_datatype_t *dt)
{
if (dt->layout)
return true;
if (ua) {
jl_svec_t *types = jl_get_fieldtypes(dt);
size_t i, ntypes = jl_svec_len(types);
for (i = 0; i < ntypes; i++) {
jl_value_t *ty = jl_svecref(types, i);
if (jl_has_typevar_from_unionall(ty, ua))
return false;
}
}
return true;
return dt->layout || jl_has_fixed_layout(dt);
}

static unsigned jl_field_align(jl_datatype_t *dt, size_t i)
Expand All @@ -569,7 +556,7 @@ static unsigned jl_field_align(jl_datatype_t *dt, size_t i)
return std::min({al, (unsigned)jl_datatype_align(dt), (unsigned)JL_HEAP_ALIGNMENT});
}

static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_unionall_t *ua_env, bool *isboxed, bool llvmcall)
static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, bool *isboxed, bool llvmcall)
{
// this function converts a Julia Type into the equivalent LLVM struct
// use this where C-compatible (unboxed) structs are desired
Expand All @@ -584,16 +571,19 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_
bool isTuple = jl_is_tuple_type(jt);
jl_svec_t *ftypes = jl_get_fieldtypes(jst);
size_t i, ntypes = jl_svec_len(ftypes);
if (ntypes == 0 || (jst->layout && jl_datatype_nbits(jst) == 0))
if (!julia_struct_has_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;
// TODO: we should probably make a temporary root for `jst` somewhere
// don't use pre-filled struct_decl for llvmcall (f16, etc. may be different)
Type *&struct_decl = (ctx && !llvmcall ? ctx->llvmtypes[jst] : _struct_decl);
if (struct_decl)
return struct_decl;
if (!julia_struct_has_layout(jst, ua_env))
return NULL;
std::vector<Type*> latypes(0);
bool isarray = true;
bool isvector = true;
Expand All @@ -605,17 +595,8 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_
if (jlasttype != NULL && ty != jlasttype)
isvector = false;
jlasttype = ty;
size_t fsz = 0, al = 0;
bool isptr = !jl_islayout_inline(ty, &fsz, &al);
if (jst->layout) {
// NOTE: jl_field_isptr can disagree with jl_islayout_inline here if the
// struct decided this field must be a pointer due to a type circularity.
// Example from issue #40050: `struct B <: Ref{Tuple{B}}; end`
isptr = jl_field_isptr(jst, i);
assert((isptr ? sizeof(void*) : fsz + jl_is_uniontype(ty)) == jl_field_size(jst, i));
}
Type *lty;
if (isptr) {
if (jl_field_isptr(jst, i)) {
lty = T_prjlvalue;
isvector = false;
}
Expand All @@ -626,13 +607,15 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_
// pick an Integer type size such that alignment will generally be correct,
// and always end with an Int8 (selector byte).
// We may need to insert padding first to get to the right offset
size_t fsz = 0, al = 0;
bool isptr = !jl_islayout_inline(ty, &fsz, &al);
assert(!isptr && fsz == jl_field_size(jst, i) - 1); (void)isptr;
if (al > MAX_ALIGN) {
Type *AlignmentType;
AlignmentType = ArrayType::get(FixedVectorType::get(T_int8, al), 0);
latypes.push_back(AlignmentType);
al = MAX_ALIGN;
}
assert(al <= jl_field_align(jst, i));
Type *AlignmentType = IntegerType::get(jl_LLVMContext, 8 * al);
unsigned NumATy = fsz / al;
unsigned remainder = fsz % al;
Expand All @@ -647,8 +630,9 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_
continue;
}
else {
lty = _julia_struct_to_llvm(ctx, ty, NULL, &isptr, llvmcall);
assert(!isptr);
bool isptr;
lty = _julia_struct_to_llvm(ctx, ty, &isptr, llvmcall);
assert(lty && !isptr);
}
if (lasttype != NULL && lasttype != lty)
isarray = false;
Expand Down Expand Up @@ -703,16 +687,9 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, jl_value_t *jt, jl_
return T_prjlvalue;
}

static Type *julia_struct_to_llvm(jl_codectx_t &ctx, jl_value_t *jt, jl_unionall_t *ua, bool *isboxed)
{
return _julia_struct_to_llvm(&ctx.emission_context, jt, ua, isboxed);
}

bool jl_type_mappable_to_c(jl_value_t *ty)
static Type *julia_struct_to_llvm(jl_codectx_t &ctx, jl_value_t *jt, bool *isboxed)
{
jl_codegen_params_t params;
bool toboxed;
return _julia_struct_to_llvm(&params, ty, NULL, &toboxed) != NULL;
return _julia_struct_to_llvm(&ctx.emission_context, jt, isboxed);
}

static bool is_datatype_all_pointers(jl_datatype_t *dt)
Expand Down
16 changes: 7 additions & 9 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,11 +920,11 @@ static bool jl_is_pointerfree(jl_value_t* t)

// these queries are usually related, but we split them out here
// for convenience and clarity (and because it changes the calling convention)
static bool deserves_stack(jl_value_t* t, bool pointerfree=false)
static bool deserves_stack(jl_value_t* t)
{
if (!jl_is_concrete_immutable(t))
return false;
return ((jl_datatype_t*)t)->isinlinealloc;
return jl_datatype_isinlinealloc((jl_datatype_t*)t, 0);
}
static bool deserves_argbox(jl_value_t* t)
{
Expand Down Expand Up @@ -3042,6 +3042,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
jl_value_t *jt = jl_tparam0(utt);
if (jl_is_vararg(jt))
jt = jl_unwrap_vararg(jt);
assert(jl_is_datatype(jt));
Value *vidx = emit_unbox(ctx, T_size, fld, (jl_value_t*)jl_long_type);
// This is not necessary for correctness, but allows to omit
// the extra code for getting the length of the tuple
Expand All @@ -3053,7 +3054,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
emit_datatype_nfields(ctx, emit_typeof_boxed(ctx, obj)),
jl_true);
}
bool isboxed = !jl_datatype_isinlinealloc(jt);
bool isboxed = !jl_datatype_isinlinealloc((jl_datatype_t*)jt, 0);
Value *ptr = maybe_decay_tracked(ctx, data_pointer(ctx, obj));
*ret = typed_load(ctx, ptr, vidx,
isboxed ? (jl_value_t*)jl_any_type : jt,
Expand Down Expand Up @@ -5424,10 +5425,7 @@ static jl_cgval_t emit_cfunction(jl_codectx_t &ctx, jl_value_t *output_type, con
// some sanity checking and check whether there's a vararg
size_t nargt = jl_svec_len(argt);
bool isVa = (nargt > 0 && jl_is_vararg(jl_svecref(argt, nargt - 1)));
if (isVa) {
emit_error(ctx, "cfunction: Vararg syntax not allowed for argument list");
return jl_cgval_t();
}
assert(!isVa);

jl_array_t *closure_types = NULL;
jl_value_t *sigt = NULL; // dispatch-sig = type signature with Ref{} annotations removed and applied to the env
Expand Down Expand Up @@ -5576,7 +5574,7 @@ const char *jl_generate_ccallable(void *llvmmod, void *sysimg_handle, jl_value_t
crt = (jl_value_t*)jl_any_type;
}
bool toboxed;
Type *lcrt = _julia_struct_to_llvm(&params, crt, NULL, &toboxed);
Type *lcrt = _julia_struct_to_llvm(&params, crt, &toboxed);
if (toboxed)
lcrt = T_prjlvalue;
size_t nargs = jl_nparams(sigt)-1;
Expand Down Expand Up @@ -6325,7 +6323,7 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
if (allunbox)
return;
}
else if (deserves_stack(jt, true)) {
else if (deserves_stack(jt)) {
bool isboxed;
Type *vtype = julia_type_to_llvm(ctx, jt, &isboxed);
assert(!isboxed);
Expand Down
55 changes: 29 additions & 26 deletions src/datatype.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ JL_DLLEXPORT jl_typename_t *jl_new_typename_in(jl_sym_t *name, jl_module_t *modu
tn->abstract = abstract;
tn->mutabl = mutabl;
tn->references_self = 0;
tn->mayinlinealloc = 0;
tn->mt = NULL;
tn->partial = NULL;
return tn;
Expand All @@ -97,7 +98,6 @@ jl_datatype_t *jl_new_uninitialized_datatype(void)
t->isdispatchtuple = 0;
t->isbitstype = 0;
t->zeroinit = 0;
t->isinlinealloc = 0;
t->has_concrete_subtype = 1;
t->cached_by_hash = 0;
t->name = NULL;
Expand Down Expand Up @@ -238,6 +238,22 @@ STATIC_INLINE void jl_maybe_allocate_singleton_instance(jl_datatype_t *st)
}
}

int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree) JL_NOTSAFEPOINT
{
if (ty->name->mayinlinealloc && ty->layout) {
if (ty->layout->npointers > 0) {
if (pointerfree)
return 0;
if (ty->ninitialized != jl_svec_len(ty->types))
Copy link
Member

@JeffBezanson JeffBezanson May 26, 2021

Choose a reason for hiding this comment

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

No need to do it now, but it occurs to me ninitialized can move to TypeName as well?

Copy link
Member Author

@vtjnash vtjnash May 27, 2021

Choose a reason for hiding this comment

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

Good observation. I can make it n_uninitialized, then it would be shared. I'll do that once this is merged. I've also thought about removing ->names, since that is always shared too.

Copy link
Member

@JeffBezanson JeffBezanson May 27, 2021

Choose a reason for hiding this comment

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

names isn't shared for NamedTuples. I guess if we wanted we could fetch it out of the parameters in that case though.

return 0;
if (ty->layout->fielddesc_type > 1) // GC only implements support for 8 and 16 (not array32)
return 0;
}
return 1;
}
return 0;
}

static unsigned union_isinlinable(jl_value_t *ty, int pointerfree, size_t *nbytes, size_t *align, int asfield) JL_NOTSAFEPOINT
{
if (jl_is_uniontype(ty)) {
Expand All @@ -249,7 +265,7 @@ static unsigned union_isinlinable(jl_value_t *ty, int pointerfree, size_t *nbyte
return 0;
return na + nb;
}
if (jl_is_datatype(ty) && jl_datatype_isinlinealloc(ty) && (!pointerfree || ((jl_datatype_t*)ty)->layout->npointers == 0)) {
if (jl_is_datatype(ty) && jl_datatype_isinlinealloc((jl_datatype_t*)ty, pointerfree)) {
size_t sz = jl_datatype_size(ty);
size_t al = jl_datatype_align(ty);
// primitive types in struct slots need their sizes aligned. issue #37974
Expand Down Expand Up @@ -329,15 +345,11 @@ void jl_compute_field_offsets(jl_datatype_t *st)
const uint64_t max_offset = (((uint64_t)1) << 32) - 1;
const uint64_t max_size = max_offset >> 1;

if (st->types == NULL || st->name->wrapper == NULL)
return;
if ((jl_is_tuple_type(st) || jl_is_namedtuple_type(st)) && !jl_is_concrete_type((jl_value_t*)st))
return;
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);
if (w->types == NULL) // we got called too early--we'll be back
return;
assert(st->types && w->types);
size_t i, nfields = jl_svec_len(st->types);
int isinlinealloc = st->isconcretetype && !st->name->mutabl && !st->name->references_self;
assert(st->ninitialized <= nfields);
if (st == w && st->layout) {
// this check allows us to force re-computation of the layout for some types during init
Expand Down Expand Up @@ -386,17 +398,13 @@ void jl_compute_field_offsets(jl_datatype_t *st)
st->has_concrete_subtype = !jl_is_datatype(fld) || ((jl_datatype_t *)fld)->has_concrete_subtype;
}
// compute layout for the wrapper object if the field types have no free variables
if (!st->isconcretetype) {
if (st != w)
return; // otherwise we would leak memory
for (i = 0; i < nfields; i++) {
if (jl_has_free_typevars(jl_field_type(st, i)))
return; // not worthwhile computing the rest
}
if (!st->isconcretetype && !jl_has_fixed_layout(st)) {
assert(st == w); // otherwise caller should not have requested this layout
return;
}
}

int isbitstype = isinlinealloc;
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 Expand Up @@ -513,14 +521,7 @@ void jl_compute_field_offsets(jl_datatype_t *st)
}
// now finish deciding if this instantiation qualifies for special properties
assert(!isbitstype || st->layout->npointers == 0); // the definition of isbits
if (isinlinealloc && st->layout->npointers > 0) {
if (st->ninitialized != nfields)
isinlinealloc = 0;
else if (st->layout->fielddesc_type > 1) // GC only implements support for 8 and 16 (not array32)
isinlinealloc = 0;
}
st->isbitstype = isbitstype;
st->isinlinealloc = isinlinealloc;
jl_maybe_allocate_singleton_instance(st);
return;
}
Expand Down Expand Up @@ -595,10 +596,12 @@ JL_DLLEXPORT jl_datatype_t *jl_new_datatype(
t->name->wrapper = jl_new_struct(jl_unionall_type, jl_svecref(parameters, i), t->name->wrapper);
jl_gc_wb(t->name, t->name->wrapper);
}
if (!mutabl && !abstract && ftypes != NULL)
tn->mayinlinealloc = 1;
}
jl_precompute_memoized_dt(t, 0);

if (!abstract)
if (!abstract && t->types != NULL)
jl_compute_field_offsets(t);

JL_GC_POP();
Expand All @@ -615,7 +618,7 @@ JL_DLLEXPORT jl_datatype_t *jl_new_primitivetype(jl_value_t *name, jl_module_t *
uint32_t alignm = next_power_of_two(nbytes);
if (alignm > MAX_ALIGN)
alignm = MAX_ALIGN;
bt->isbitstype = bt->isinlinealloc = (parameters == jl_emptysvec);
bt->isbitstype = (parameters == jl_emptysvec);
bt->size = nbytes;
bt->layout = jl_get_layout(0, 0, alignm, 0, NULL, NULL);
bt->instance = NULL;
Expand Down
Loading