Skip to content

Commit ad47ff9

Browse files
vtjnashvtjnash
committed
add array element mutex offset in print and gc
The layout and printing logic need to correctly offset and align the inset fields to account for the per-element mutex of an atomic array with large elements. Fix #58993
1 parent d7bdd21 commit ad47ff9

File tree

6 files changed

+72
-33
lines changed

6 files changed

+72
-33
lines changed

‎base/runtime_internals.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,10 @@ struct DataTypeLayout
528528
# fielddesc_type : 2;
529529
# arrayelem_isboxed : 1;
530530
# arrayelem_isunion : 1;
531+
# arrayelem_isatomic : 1;
532+
# arrayelem_islocked : 1;
533+
# isbitsegal : 1;
534+
# padding : 8;
531535
end
532536

533537
"""

‎src/codegen.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3862,8 +3862,8 @@ static bool emit_f_opmemory(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
38623862
const jl_datatype_layout_t *layout = ((jl_datatype_t*)mty_dt)->layout;
38633863
bool isboxed = layout->flags.arrayelem_isboxed;
38643864
bool isunion = layout->flags.arrayelem_isunion;
3865-
bool isatomic = kind == (jl_value_t*)jl_atomic_sym;
3866-
bool needlock = isatomic && layout->size > MAX_ATOMIC_SIZE;
3865+
bool isatomic = layout->flags.arrayelem_isatomic || layout->flags.arrayelem_islocked;
3866+
bool needlock = layout->flags.arrayelem_islocked;
38673867
size_t elsz = layout->size;
38683868
size_t al = layout->alignment;
38693869
if (al > JL_HEAP_ALIGNMENT)
@@ -4222,7 +4222,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
42224222
size_t al = layout->alignment;
42234223
if (al > JL_HEAP_ALIGNMENT)
42244224
al = JL_HEAP_ALIGNMENT;
4225-
bool needlock = isatomic && !isboxed && elsz > MAX_ATOMIC_SIZE;
4225+
bool needlock = layout->flags.arrayelem_islocked;
42264226
AtomicOrdering Order = (needlock || order <= jl_memory_order_notatomic)
42274227
? (isboxed ? AtomicOrdering::Unordered : AtomicOrdering::NotAtomic)
42284228
: get_llvm_atomic_order(order);
@@ -4308,7 +4308,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
43084308
*ret = jl_cgval_t(); // unreachable
43094309
return true;
43104310
}
4311-
bool isatomic = kind == (jl_value_t*)jl_atomic_sym;
4311+
const jl_datatype_layout_t *layout = ((jl_datatype_t*)mty_dt)->layout;
4312+
bool isatomic = layout->flags.arrayelem_isatomic || layout->flags.arrayelem_islocked;
43124313
if (!isatomic && order != jl_memory_order_notatomic && order != jl_memory_order_unspecified) {
43134314
emit_atomic_error(ctx, "memoryref_isassigned: non-atomic memory cannot be accessed atomically");
43144315
*ret = jl_cgval_t(); // unreachable
@@ -4324,13 +4325,12 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
43244325
}
43254326
jl_value_t *boundscheck = argv[3].constant;
43264327
emit_typecheck(ctx, argv[3], (jl_value_t*)jl_bool_type, fname);
4327-
const jl_datatype_layout_t *layout = ((jl_datatype_t*)mty_dt)->layout;
43284328
Value *mem = emit_memoryref_mem(ctx, ref, layout);
43294329
Value *mlen = emit_genericmemorylen(ctx, mem, ref.typ);
43304330
Value *oob = bounds_check_enabled(ctx, boundscheck) ? ctx.builder.CreateIsNull(mlen) : nullptr;
43314331
bool isboxed = layout->flags.arrayelem_isboxed;
43324332
if (isboxed || layout->first_ptr >= 0) {
4333-
bool needlock = isatomic && !isboxed && layout->size > MAX_ATOMIC_SIZE;
4333+
bool needlock = layout->flags.arrayelem_islocked;
43344334
AtomicOrdering Order = (needlock || order <= jl_memory_order_notatomic)
43354335
? (isboxed ? AtomicOrdering::Unordered : AtomicOrdering::NotAtomic)
43364336
: get_llvm_atomic_order(order);

‎src/datatype.c

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,10 @@ static jl_datatype_layout_t *jl_get_layout(uint32_t sz,
239239
flddesc->flags.haspadding = haspadding;
240240
flddesc->flags.isbitsegal = isbitsegal;
241241
flddesc->flags.fielddesc_type = fielddesc_type;
242-
flddesc->flags.arrayelem_isboxed = arrayelem == 1;
243-
flddesc->flags.arrayelem_isunion = arrayelem == 2;
242+
flddesc->flags.arrayelem_isboxed = (arrayelem & 1) != 0;
243+
flddesc->flags.arrayelem_isunion = (arrayelem & 2) != 0;
244+
flddesc->flags.arrayelem_isatomic = (arrayelem & 4) != 0;
245+
flddesc->flags.arrayelem_islocked = (arrayelem & 8) != 0;
244246
flddesc->flags.padding = 0;
245247
flddesc->npointers = npointers;
246248
flddesc->first_ptr = first_ptr;
@@ -537,6 +539,7 @@ void jl_get_genericmemory_layout(jl_datatype_t *st)
537539
uint32_t *pointers = &first_ptr;
538540
int needlock = 0;
539541

542+
const jl_datatype_layout_t *el_layout = NULL;
540543
if (isunboxed) {
541544
elsz = LLT_ALIGN(elsz, al);
542545
if (kind == (jl_value_t*)jl_atomic_sym) {
@@ -551,12 +554,12 @@ void jl_get_genericmemory_layout(jl_datatype_t *st)
551554
else {
552555
assert(jl_is_datatype(eltype));
553556
zi = ((jl_datatype_t*)eltype)->zeroinit;
554-
const jl_datatype_layout_t *layout = ((jl_datatype_t*)eltype)->layout;
555-
if (layout->first_ptr >= 0) {
556-
first_ptr = layout->first_ptr;
557-
npointers = layout->npointers;
558-
if (layout->flags.fielddesc_type == 2) {
559-
pointers = (uint32_t*)jl_dt_layout_ptrs(layout);
557+
el_layout = ((jl_datatype_t*)eltype)->layout;
558+
if (el_layout->first_ptr >= 0) {
559+
first_ptr = el_layout->first_ptr;
560+
npointers = el_layout->npointers;
561+
if (el_layout->flags.fielddesc_type == 2 && !needlock) {
562+
pointers = (uint32_t*)jl_dt_layout_ptrs(el_layout);
560563
}
561564
else {
562565
pointers = (uint32_t*)alloca(npointers * sizeof(uint32_t));
@@ -568,10 +571,22 @@ void jl_get_genericmemory_layout(jl_datatype_t *st)
568571
}
569572
if (needlock) {
570573
assert(al <= JL_SMALL_BYTE_ALIGNMENT);
571-
size_t offset = LLT_ALIGN(sizeof(jl_mutex_t), JL_SMALL_BYTE_ALIGNMENT);
572-
elsz += offset;
574+
size_t lock_offset = LLT_ALIGN(sizeof(jl_mutex_t), JL_SMALL_BYTE_ALIGNMENT);
575+
elsz += lock_offset;
576+
if (al < sizeof(void*)) {
577+
al = sizeof(void*);
578+
elsz = LLT_ALIGN(elsz, al);
579+
}
573580
haspadding = 1;
574581
zi = 1;
582+
// Adjust pointer offsets to account for the lock at the beginning
583+
if (first_ptr >= 0) {
584+
uint32_t lock_offset_words = lock_offset / sizeof(void*);
585+
first_ptr += lock_offset_words;
586+
for (int j = 0; j < npointers; j++) {
587+
pointers[j] += lock_offset_words;
588+
}
589+
}
575590
}
576591
}
577592
else {
@@ -580,13 +595,17 @@ void jl_get_genericmemory_layout(jl_datatype_t *st)
580595
zi = 1;
581596
}
582597

583-
int arrayelem;
598+
// arrayelem is a bitfield: 1=isboxed, 2=isunion, 4=isatomic, 8=islocked
599+
int arrayelem = 0;
584600
if (!isunboxed)
585-
arrayelem = 1;
586-
else if (isunion)
587-
arrayelem = 2;
588-
else
589-
arrayelem = 0;
601+
arrayelem |= 1; // arrayelem_isboxed
602+
if (isunion)
603+
arrayelem |= 2; // arrayelem_isunion
604+
if (kind == (jl_value_t*)jl_atomic_sym) {
605+
arrayelem |= 4; // arrayelem_isatomic
606+
if (needlock)
607+
arrayelem |= 8; // arrayelem_islocked
608+
}
590609
assert(!st->layout);
591610
st->layout = jl_get_layout(elsz, nfields, npointers, al, haspadding, isbitsegal, arrayelem, NULL, pointers);
592611
st->zeroinit = zi;
@@ -647,17 +666,17 @@ void jl_compute_field_offsets(jl_datatype_t *st)
647666
// if we have no fields, we can trivially skip the rest
648667
if (st == jl_symbol_type || st == jl_string_type) {
649668
// opaque layout - heap-allocated blob
650-
static const jl_datatype_layout_t opaque_byte_layout = {0, 0, 1, -1, 1, { .haspadding = 0, .fielddesc_type=0, .isbitsegal=1, .arrayelem_isboxed=0, .arrayelem_isunion=0 }};
669+
static const jl_datatype_layout_t opaque_byte_layout = {0, 0, 1, -1, 1, { .isbitsegal=1 }};
651670
st->layout = &opaque_byte_layout;
652671
return;
653672
}
654673
else if (st == jl_simplevector_type || st == jl_module_type) {
655-
static const jl_datatype_layout_t opaque_ptr_layout = {0, 0, 1, -1, sizeof(void*), { .haspadding = 0, .fielddesc_type=0, .isbitsegal=1, .arrayelem_isboxed=0, .arrayelem_isunion=0 }};
674+
static const jl_datatype_layout_t opaque_ptr_layout = {0, 0, 1, -1, sizeof(void*), { .isbitsegal=1 }};
656675
st->layout = &opaque_ptr_layout;
657676
return;
658677
}
659678
else {
660-
static const jl_datatype_layout_t singleton_layout = {0, 0, 0, -1, 1, { .haspadding = 0, .fielddesc_type=0, .isbitsegal=1, .arrayelem_isboxed=0, .arrayelem_isunion=0 }};
679+
static const jl_datatype_layout_t singleton_layout = {0, 0, 0, -1, 1, { .isbitsegal=1 }};
661680
st->layout = &singleton_layout;
662681
}
663682
}
@@ -1001,6 +1020,8 @@ JL_DLLEXPORT jl_datatype_t * jl_new_foreign_type(jl_sym_t *name,
10011020
layout->flags.padding = 0;
10021021
layout->flags.arrayelem_isboxed = 0;
10031022
layout->flags.arrayelem_isunion = 0;
1023+
layout->flags.arrayelem_isatomic = 0;
1024+
layout->flags.arrayelem_islocked = 0;
10041025
jl_fielddescdyn_t * desc =
10051026
(jl_fielddescdyn_t *) ((char *)layout + sizeof(*layout));
10061027
desc->markfunc = markfunc;

‎src/genericmemory.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ JL_DLLEXPORT void jl_genericmemory_copyto(jl_genericmemory_t *dest, char* destda
262262

263263
JL_DLLEXPORT jl_value_t *jl_genericmemoryref(jl_genericmemory_t *mem, size_t i)
264264
{
265-
int isatomic = (jl_tparam0(jl_typetagof(mem)) == (jl_value_t*)jl_atomic_sym);
266265
const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(mem))->layout;
266+
int isatomic = layout->flags.arrayelem_isatomic || layout->flags.arrayelem_islocked;
267267
jl_genericmemoryref_t m;
268268
m.mem = mem;
269269
m.ptr_or_offset = (layout->flags.arrayelem_isunion || layout->size == 0) ? (void*)i : (void*)((char*)mem->ptr + layout->size * i);
@@ -342,8 +342,8 @@ static jl_value_t *jl_ptrmemrefget(jl_genericmemoryref_t m JL_PROPAGATES_ROOT, i
342342

343343
JL_DLLEXPORT jl_value_t *jl_memoryrefget(jl_genericmemoryref_t m, int isatomic)
344344
{
345-
assert(isatomic == (jl_tparam0(jl_typetagof(m.mem)) == (jl_value_t*)jl_atomic_sym));
346345
const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(m.mem))->layout;
346+
assert(isatomic == (layout->flags.arrayelem_isatomic || layout->flags.arrayelem_islocked));
347347
if (layout->flags.arrayelem_isboxed)
348348
return jl_ptrmemrefget(m, isatomic);
349349
jl_value_t *eltype = jl_tparam1(jl_typetagof(m.mem));
@@ -365,7 +365,7 @@ JL_DLLEXPORT jl_value_t *jl_memoryrefget(jl_genericmemoryref_t m, int isatomic)
365365
assert(data - (char*)m.mem->ptr < layout->size * m.mem->length);
366366
jl_value_t *r;
367367
size_t fsz = jl_datatype_size(eltype);
368-
int needlock = isatomic && fsz > MAX_ATOMIC_SIZE;
368+
int needlock = layout->flags.arrayelem_islocked;
369369
if (isatomic && !needlock) {
370370
r = jl_atomic_new_bits(eltype, data);
371371
}
@@ -393,7 +393,7 @@ static int _jl_memoryref_isassigned(jl_genericmemoryref_t m, int isatomic)
393393
if (layout->flags.arrayelem_isboxed) {
394394
}
395395
else if (layout->first_ptr >= 0) {
396-
int needlock = isatomic && layout->size > MAX_ATOMIC_SIZE;
396+
int needlock = layout->flags.arrayelem_islocked;
397397
if (needlock)
398398
elem = elem + LLT_ALIGN(sizeof(jl_mutex_t), JL_SMALL_BYTE_ALIGNMENT) / sizeof(jl_value_t*);
399399
elem = &elem[layout->first_ptr];
@@ -411,15 +411,15 @@ JL_DLLEXPORT jl_value_t *jl_memoryref_isassigned(jl_genericmemoryref_t m, int is
411411

412412
JL_DLLEXPORT void jl_memoryrefset(jl_genericmemoryref_t m JL_ROOTING_ARGUMENT, jl_value_t *rhs JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED, int isatomic)
413413
{
414-
assert(isatomic == (jl_tparam0(jl_typetagof(m.mem)) == (jl_value_t*)jl_atomic_sym));
414+
const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(m.mem))->layout;
415+
assert(isatomic == (layout->flags.arrayelem_isatomic || layout->flags.arrayelem_islocked));
415416
jl_value_t *eltype = jl_tparam1(jl_typetagof(m.mem));
416417
if (eltype != (jl_value_t*)jl_any_type && !jl_typeis(rhs, eltype)) {
417418
JL_GC_PUSH1(&rhs);
418419
if (!jl_isa(rhs, eltype))
419420
jl_type_error("memoryrefset!", eltype, rhs);
420421
JL_GC_POP();
421422
}
422-
const jl_datatype_layout_t *layout = ((jl_datatype_t*)jl_typetagof(m.mem))->layout;
423423
if (layout->flags.arrayelem_isboxed) {
424424
assert((char*)m.ptr_or_offset - (char*)m.mem->ptr < sizeof(jl_value_t*) * m.mem->length);
425425
if (isatomic)
@@ -449,7 +449,7 @@ JL_DLLEXPORT void jl_memoryrefset(jl_genericmemoryref_t m JL_ROOTING_ARGUMENT, j
449449
}
450450
if (layout->size != 0) {
451451
assert(data - (char*)m.mem->ptr < layout->size * m.mem->length);
452-
int needlock = isatomic && layout->size > MAX_ATOMIC_SIZE;
452+
int needlock = layout->flags.arrayelem_islocked;
453453
size_t fsz = jl_datatype_size((jl_datatype_t*)jl_typeof(rhs)); // need to shrink-wrap the final copy
454454
if (isatomic && !needlock) {
455455
jl_atomic_store_bits(data, rhs, fsz);

‎src/julia.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,10 +574,12 @@ typedef struct {
574574
// metadata bit only for GenericMemory eltype layout
575575
uint16_t arrayelem_isboxed : 1;
576576
uint16_t arrayelem_isunion : 1;
577+
uint16_t arrayelem_isatomic : 1;
578+
uint16_t arrayelem_islocked : 1;
577579
// If set, this type's egality can be determined entirely by comparing
578580
// the non-padding bits of this datatype.
579581
uint16_t isbitsegal : 1;
580-
uint16_t padding : 10;
582+
uint16_t padding : 8;
581583
} flags;
582584
// union {
583585
// jl_fielddesc8_t field8[nfields];
@@ -1667,6 +1669,8 @@ static inline int jl_field_isconst(jl_datatype_t *st, int i) JL_NOTSAFEPOINT
16671669
#define jl_is_addrspacecore(v) jl_typetagis(v,jl_addrspacecore_type)
16681670
#define jl_is_abioverride(v) jl_typetagis(v,jl_abioverride_type)
16691671
#define jl_genericmemory_isbitsunion(a) (((jl_datatype_t*)jl_typetagof(a))->layout->flags.arrayelem_isunion)
1672+
#define jl_genericmemory_isatomic(a) (((jl_datatype_t*)jl_typetagof(a))->layout->flags.arrayelem_isatomic)
1673+
#define jl_genericmemory_islocked(a) (((jl_datatype_t*)jl_typetagof(a))->layout->flags.arrayelem_islocked)
16701674
#define jl_is_array_any(v) jl_typetagis(v,jl_array_any_type)
16711675

16721676
JL_DLLEXPORT int jl_subtype(jl_value_t *a, jl_value_t *b);

‎src/rtutils.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,10 +1290,20 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt
12901290
for (size_t j = 0; j < tlen; j++) {
12911291
if (layout->flags.arrayelem_isboxed) {
12921292
jl_value_t **ptr = ((jl_value_t**)m->ptr) + j;
1293+
if (layout->flags.arrayelem_islocked) {
1294+
// Skip the lock at the beginning for locked arrays
1295+
size_t lock_size = sizeof(jl_mutex_t);
1296+
ptr = (jl_value_t**)((char*)ptr + lock_size);
1297+
}
12931298
n += jl_static_show_x(out, *ptr, depth, ctx);
12941299
}
12951300
else {
12961301
char *ptr = ((char*)m->ptr) + j * layout->size;
1302+
if (layout->flags.arrayelem_islocked) {
1303+
// Skip the lock at the beginning for locked arrays
1304+
size_t lock_size = sizeof(jl_mutex_t);
1305+
ptr += lock_size;
1306+
}
12971307
n += jl_static_show_x_(out, (jl_value_t*)ptr,
12981308
(jl_datatype_t*)(typetagdata ? jl_nth_union_component(el_type, typetagdata[j]) : el_type),
12991309
depth, ctx);

0 commit comments

Comments
 (0)