Skip to content

Commit 75b1730

Browse files
vtjnashLilithHafner
authored andcommitted
datatype: finish layout corrections (JuliaLang#43306)
We still had some discrepancies between what the code thought could be computed for a layout, and what we actually could compute layout for. This hopefully makes those two computations now always give the same answers (with the assistance of the layout cache). Fixes JuliaLang#43303
1 parent 1e677e9 commit 75b1730

File tree

5 files changed

+29
-45
lines changed

5 files changed

+29
-45
lines changed

src/array.c

+8-31
Original file line numberDiff line numberDiff line change
@@ -219,51 +219,28 @@ JL_DLLEXPORT jl_array_t *jl_reshape_array(jl_value_t *atype, jl_array_t *data,
219219
jl_value_t *_dims)
220220
{
221221
jl_task_t *ct = jl_current_task;
222-
jl_array_t *a;
222+
assert(jl_types_equal(jl_tparam0(jl_typeof(data)), jl_tparam0(atype)));
223+
223224
size_t ndims = jl_nfields(_dims);
224225
assert(is_ntuple_long(_dims));
225226
size_t *dims = (size_t*)_dims;
226-
assert(jl_types_equal(jl_tparam0(jl_typeof(data)), jl_tparam0(atype)));
227-
228227
int ndimwords = jl_array_ndimwords(ndims);
229228
int tsz = sizeof(jl_array_t) + ndimwords * sizeof(size_t) + sizeof(void*);
230-
a = (jl_array_t*)jl_gc_alloc(ct->ptls, tsz, atype);
229+
jl_array_t *a = (jl_array_t*)jl_gc_alloc(ct->ptls, tsz, atype);
231230
// No allocation or safepoint allowed after this
231+
// copy data (except dims) from the old object
232232
a->flags.pooled = tsz <= GC_MAX_SZCLASS;
233233
a->flags.ndims = ndims;
234234
a->offset = 0;
235235
a->data = NULL;
236236
a->flags.isaligned = data->flags.isaligned;
237-
jl_array_t *owner = (jl_array_t*)jl_array_owner(data);
238-
jl_value_t *eltype = jl_tparam0(atype);
239-
size_t elsz = 0, align = 0;
240-
int isboxed = !jl_islayout_inline(eltype, &elsz, &align);
241-
assert(isboxed == data->flags.ptrarray);
242-
if (!isboxed) {
243-
a->elsize = LLT_ALIGN(elsz, align);
244-
jl_value_t *ownerty = jl_typeof(owner);
245-
size_t oldelsz = 0, oldalign = 0;
246-
if (ownerty == (jl_value_t*)jl_string_type) {
247-
oldalign = 1;
248-
}
249-
else {
250-
jl_islayout_inline(jl_tparam0(ownerty), &oldelsz, &oldalign);
251-
}
252-
if (oldalign < align)
253-
jl_exceptionf(jl_argumenterror_type,
254-
"reinterpret from alignment %d bytes to alignment %d bytes not allowed",
255-
(int) oldalign, (int) align);
256-
a->flags.ptrarray = 0;
257-
a->flags.hasptr = data->flags.hasptr;
258-
}
259-
else {
260-
a->elsize = sizeof(void*);
261-
a->flags.ptrarray = 1;
262-
a->flags.hasptr = 0;
263-
}
237+
a->elsize = data->elsize;
238+
a->flags.ptrarray = data->flags.ptrarray;
239+
a->flags.hasptr = data->flags.hasptr;
264240

265241
// if data is itself a shared wrapper,
266242
// owner should point back to the original array
243+
jl_array_t *owner = (jl_array_t*)jl_array_owner(data);
267244
jl_array_data_owner(a) = (jl_value_t*)owner;
268245

269246
a->flags.how = 3;

src/datatype.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,9 @@ int jl_struct_try_layout(jl_datatype_t *dt)
248248
return 1;
249249
}
250250

251-
int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree) JL_NOTSAFEPOINT
251+
int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree)
252252
{
253-
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)
253+
if (ty->name->mayinlinealloc && jl_struct_try_layout(ty)) {
254254
if (ty->layout->npointers > 0) {
255255
if (pointerfree)
256256
return 0;
@@ -264,7 +264,7 @@ int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree) JL_NOTSAFEPOIN
264264
return 0;
265265
}
266266

267-
static unsigned union_isinlinable(jl_value_t *ty, int pointerfree, size_t *nbytes, size_t *align, int asfield) JL_NOTSAFEPOINT
267+
static unsigned union_isinlinable(jl_value_t *ty, int pointerfree, size_t *nbytes, size_t *align, int asfield)
268268
{
269269
if (jl_is_uniontype(ty)) {
270270
unsigned na = union_isinlinable(((jl_uniontype_t*)ty)->a, 1, nbytes, align, asfield);
@@ -290,19 +290,19 @@ static unsigned union_isinlinable(jl_value_t *ty, int pointerfree, size_t *nbyte
290290
return 0;
291291
}
292292

293-
int jl_uniontype_size(jl_value_t *ty, size_t *sz) JL_NOTSAFEPOINT
293+
int jl_uniontype_size(jl_value_t *ty, size_t *sz)
294294
{
295295
size_t al = 0;
296296
return union_isinlinable(ty, 0, sz, &al, 0) != 0;
297297
}
298298

299-
JL_DLLEXPORT int jl_islayout_inline(jl_value_t *eltype, size_t *fsz, size_t *al) JL_NOTSAFEPOINT
299+
JL_DLLEXPORT int jl_islayout_inline(jl_value_t *eltype, size_t *fsz, size_t *al)
300300
{
301301
unsigned countbits = union_isinlinable(eltype, 0, fsz, al, 1);
302302
return (countbits > 0 && countbits < 127) ? countbits : 0;
303303
}
304304

305-
JL_DLLEXPORT int jl_stored_inline(jl_value_t *eltype) JL_NOTSAFEPOINT
305+
JL_DLLEXPORT int jl_stored_inline(jl_value_t *eltype)
306306
{
307307
size_t fsz = 0, al = 0;
308308
return jl_islayout_inline(eltype, &fsz, &al);

src/jltypes.c

+4-6
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,10 @@ static int layout_uses_free_typevars(jl_value_t *v, jl_typeenv_t *env)
4646
layout_uses_free_typevars(((jl_uniontype_t*)v)->b, env);
4747
if (jl_is_vararg(v)) {
4848
jl_vararg_t *vm = (jl_vararg_t*)v;
49-
if (vm->T) {
50-
if (layout_uses_free_typevars(vm->T, env))
51-
return 1;
52-
if (vm->N && layout_uses_free_typevars(vm->N, env))
53-
return 1;
54-
}
49+
if (vm->T && layout_uses_free_typevars(vm->T, env))
50+
return 1;
51+
if (vm->N && layout_uses_free_typevars(vm->N, env))
52+
return 1;
5553
return 0;
5654
}
5755
if (jl_is_unionall(v)) {

src/julia.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1480,8 +1480,8 @@ JL_DLLEXPORT void jl_set_nth_field(jl_value_t *v, size_t i, jl_value_t *r
14801480
JL_DLLEXPORT int jl_field_isdefined(jl_value_t *v, size_t i) JL_NOTSAFEPOINT;
14811481
JL_DLLEXPORT jl_value_t *jl_get_field(jl_value_t *o, const char *fld);
14821482
JL_DLLEXPORT jl_value_t *jl_value_ptr(jl_value_t *a);
1483-
int jl_uniontype_size(jl_value_t *ty, size_t *sz) JL_NOTSAFEPOINT;
1484-
JL_DLLEXPORT int jl_islayout_inline(jl_value_t *eltype, size_t *fsz, size_t *al) JL_NOTSAFEPOINT;
1483+
int jl_uniontype_size(jl_value_t *ty, size_t *sz);
1484+
JL_DLLEXPORT int jl_islayout_inline(jl_value_t *eltype, size_t *fsz, size_t *al);
14851485

14861486
// arrays
14871487
JL_DLLEXPORT jl_array_t *jl_new_array(jl_value_t *atype, jl_value_t *dims);

test/compiler/codegen.jl

+9
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,15 @@ get_llvm(g41438, ()); # cause allocation of layout
603603
@test S41438{Int}.layout != C_NULL
604604
@test !Base.datatype_pointerfree(S41438{Int})
605605

606+
607+
# issue #43303
608+
struct A43303{T}
609+
x::Pair{Ptr{T},Ptr{T}}
610+
end
611+
@test A43303.body.layout != C_NULL
612+
@test isbitstype(A43303{Int})
613+
@test A43303.body.types[1].layout != C_NULL
614+
606615
# issue #41157
607616
f41157(a, b) = a[1] = b[1]
608617
@test_throws BoundsError f41157(Tuple{Int}[], Tuple{Union{}}[])

0 commit comments

Comments
 (0)