Skip to content

Commit fe4c222

Browse files
vtjnashvtjnash
committed
avoid some LLVM unhappiness, and fieldsize was the wrong predicate here
1 parent dd3fc0a commit fe4c222

7 files changed

+78
-76
lines changed

‎src/builtins.c

+17-17
Original file line numberDiff line numberDiff line change
@@ -783,29 +783,29 @@ JL_CALLABLE(jl_f_svec)
783783

784784
// struct operations ------------------------------------------------------------
785785

786-
enum jl_memory_order jl_get_atomic_order(jl_sym_t *order)
786+
enum jl_memory_order jl_get_atomic_order(jl_sym_t *order, char loading, char storing)
787787
{
788788
if (order == none_sym)
789789
return jl_memory_order_notatomic;
790-
if (order == unordered_sym)
790+
if (order == unordered_sym && (loading || storing))
791791
return jl_memory_order_unordered;
792-
if (order == monotonic_sym)
792+
if (order == monotonic_sym && (loading || storing))
793793
return jl_memory_order_monotonic;
794-
if (order == acquire_sym)
794+
if (order == acquire_sym && loading)
795795
return jl_memory_order_acquire;
796-
if (order == release_sym)
796+
if (order == release_sym && storing)
797797
return jl_memory_order_release;
798-
if (order == acquire_release_sym)
798+
if (order == acquire_release_sym && loading && storing)
799799
return jl_memory_order_acq_rel;
800800
if (order == sequentially_consistent_sym)
801801
return jl_memory_order_seq_cst;
802802
return jl_memory_order_invalid;
803803
}
804804

805-
enum jl_memory_order jl_get_atomic_order_checked(jl_sym_t *order)
805+
enum jl_memory_order jl_get_atomic_order_checked(jl_sym_t *order, char loading, char storing)
806806
{
807-
enum jl_memory_order mo = jl_get_atomic_order(order);
808-
if (mo < 0)
807+
enum jl_memory_order mo = jl_get_atomic_order(order, loading, storing);
808+
if (mo < 0) // notatomic or invalid
809809
jl_atomic_error("invalid atomic ordering");
810810
return mo;
811811
}
@@ -818,12 +818,12 @@ JL_CALLABLE(jl_f_getfield)
818818
JL_TYPECHK(getfield, symbol, args[3]);
819819
JL_TYPECHK(getfield, bool, args[4]);
820820
nargs -= 2;
821-
order = jl_get_atomic_order_checked((jl_sym_t*)args[3]);
821+
order = jl_get_atomic_order_checked((jl_sym_t*)args[3], 1, 0);
822822
}
823823
else if (nargs == 3) {
824824
if (!jl_is_bool(args[2])) {
825825
JL_TYPECHK(getfield, symbol, args[2]);
826-
order = jl_get_atomic_order_checked((jl_sym_t*)args[2]);
826+
order = jl_get_atomic_order_checked((jl_sym_t*)args[2], 1, 0);
827827
}
828828
nargs -= 1;
829829
}
@@ -866,7 +866,7 @@ JL_CALLABLE(jl_f_setfield)
866866
if (nargs == 4) {
867867
JL_TYPECHK(getfield, symbol, args[3]);
868868
nargs -= 1;
869-
order = jl_get_atomic_order_checked((jl_sym_t*)args[3]);
869+
order = jl_get_atomic_order_checked((jl_sym_t*)args[3], 0, 1);
870870
}
871871
JL_NARGS(setfield!, 3, 3);
872872
jl_value_t *v = args[0];
@@ -1002,7 +1002,7 @@ JL_CALLABLE(jl_f_isdefined)
10021002
enum jl_memory_order order = jl_memory_order_unspecified;
10031003
if (nargs == 3) {
10041004
JL_TYPECHK(isdefined, symbol, args[2]);
1005-
order = jl_get_atomic_order_checked((jl_sym_t*)args[2]);
1005+
order = jl_get_atomic_order_checked((jl_sym_t*)args[2], 1, 0);
10061006
}
10071007
if (jl_is_module(args[0])) {
10081008
JL_TYPECHK(isdefined, symbol, args[1]);
@@ -1017,7 +1017,7 @@ JL_CALLABLE(jl_f_isdefined)
10171017
idx = jl_unbox_long(args[1]) - 1;
10181018
if (idx >= jl_datatype_nfields(vt)) {
10191019
if (order != jl_memory_order_unspecified)
1020-
jl_atomic_error("isdefined atomic ordering cannot be specified for nonexistent field");
1020+
jl_atomic_error("isdefined: atomic ordering cannot be specified for nonexistent field");
10211021
return jl_false;
10221022
}
10231023
}
@@ -1026,15 +1026,15 @@ JL_CALLABLE(jl_f_isdefined)
10261026
idx = jl_field_index(vt, (jl_sym_t*)args[1], 0);
10271027
if ((int)idx == -1) {
10281028
if (order != jl_memory_order_unspecified)
1029-
jl_atomic_error("isdefined atomic ordering cannot be specified for nonexistent field");
1029+
jl_atomic_error("isdefined: atomic ordering cannot be specified for nonexistent field");
10301030
return jl_false;
10311031
}
10321032
}
10331033
int isatomic = jl_field_isatomic(vt, idx);
10341034
if (!isatomic && order != jl_memory_order_notatomic && order != jl_memory_order_unspecified)
1035-
jl_atomic_error("isdefined non-atomic field cannot be accessed atomically");
1035+
jl_atomic_error("isdefined: non-atomic field cannot be accessed atomically");
10361036
if (isatomic && order == jl_memory_order_notatomic)
1037-
jl_atomic_error("isdefined atomic field cannot be accessed non-atomically");
1037+
jl_atomic_error("isdefined: atomic field cannot be accessed non-atomically");
10381038
int v = jl_field_isdefined(args[0], idx);
10391039
if (v == 2) {
10401040
if (order > jl_memory_order_notatomic)

‎src/cgutils.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1798,7 +1798,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17981798
{
17991799
jl_value_t *jfty = jl_field_type(jt, idx);
18001800
bool isatomic = jl_field_isatomic(jt, idx);
1801-
bool needlock = isatomic && jl_field_size(jt, idx) > MAX_ATOMIC_SIZE;
1801+
bool needlock = isatomic && !jl_field_isptr(jt, idx) && jl_datatype_size(jfty) > MAX_ATOMIC_SIZE;
18021802
if (!isatomic && order != jl_memory_order_notatomic && order != jl_memory_order_unspecified) {
18031803
emit_atomic_error(ctx, "getfield non-atomic field cannot be accessed atomically");
18041804
return jl_cgval_t(); // unreachable

‎src/codegen.cpp

+13-12
Original file line numberDiff line numberDiff line change
@@ -2993,14 +2993,14 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
29932993
emit_typecheck(ctx, inb, (jl_value_t*)jl_bool_type, "getfield");
29942994
if (!ord.constant)
29952995
return false;
2996-
order = jl_get_atomic_order((jl_sym_t*)ord.constant);
2996+
order = jl_get_atomic_order((jl_sym_t*)ord.constant, true, false);
29972997
if (inb.constant == jl_false)
29982998
boundscheck = jl_false;
29992999
}
30003000
else if (nargs == 3) {
30013001
const jl_cgval_t &arg3 = argv[3];
30023002
if (arg3.typ == (jl_value_t*)jl_symbol_type && arg3.constant)
3003-
order = jl_get_atomic_order((jl_sym_t*)arg3.constant);
3003+
order = jl_get_atomic_order((jl_sym_t*)arg3.constant, true, false);
30043004
else if (arg3.constant == jl_false)
30053005
boundscheck = jl_false;
30063006
else if (arg3.typ != (jl_value_t*)jl_bool_type)
@@ -3076,7 +3076,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
30763076
// as long as we know that all elements are of the same (leaf) type.
30773077
if (obj.ispointer()) {
30783078
if (order != jl_memory_order_notatomic && order != jl_memory_order_unspecified) {
3079-
emit_atomic_error(ctx, "getfield non-atomic field cannot be accessed atomically");
3079+
emit_atomic_error(ctx, "getfield: non-atomic field cannot be accessed atomically");
30803080
*ret = jl_cgval_t(); // unreachable
30813081
return true;
30823082
}
@@ -3120,7 +3120,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
31203120
emit_typecheck(ctx, ord, (jl_value_t*)jl_symbol_type, "setfield!");
31213121
if (!ord.constant)
31223122
return false;
3123-
order = jl_get_atomic_order((jl_sym_t*)ord.constant);
3123+
order = jl_get_atomic_order((jl_sym_t*)ord.constant, false, true);
31243124
}
31253125
if (order == jl_memory_order_invalid) {
31263126
emit_atomic_error(ctx, "invalid atomic ordering");
@@ -3145,11 +3145,11 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
31453145
// TODO: attempt better codegen for approximate types
31463146
bool isboxed = jl_field_isptr(uty, idx);
31473147
bool isatomic = jl_field_isatomic(uty, idx);
3148-
bool needlock = isatomic && jl_field_size(uty, idx) > MAX_ATOMIC_SIZE;
3148+
bool needlock = isatomic && !isboxed && jl_datatype_size(jl_field_type(uty, idx)) > MAX_ATOMIC_SIZE;
31493149
if (isatomic == (order == jl_memory_order_notatomic)) {
31503150
emit_atomic_error(ctx,
3151-
isatomic ? "setfield! atomic field cannot be written non-atomically"
3152-
: "setfield! non-atomic field cannot be written atomically");
3151+
isatomic ? "setfield!: atomic field cannot be written non-atomically"
3152+
: "setfield!: non-atomic field cannot be written atomically");
31533153
*ret = jl_cgval_t();
31543154
return true;
31553155
}
@@ -3320,7 +3320,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
33203320
emit_typecheck(ctx, ord, (jl_value_t*)jl_symbol_type, "isdefined");
33213321
if (!ord.constant)
33223322
return false;
3323-
order = jl_get_atomic_order((jl_sym_t*)ord.constant);
3323+
order = jl_get_atomic_order((jl_sym_t*)ord.constant, true, false);
33243324
}
33253325
if (order == jl_memory_order_invalid) {
33263326
emit_atomic_error(ctx, "invalid atomic ordering");
@@ -3329,7 +3329,7 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
33293329
}
33303330
if (fieldidx < 0 || fieldidx >= jl_datatype_nfields(stt)) {
33313331
if (order != jl_memory_order_unspecified) {
3332-
emit_atomic_error(ctx, "isdefined atomic ordering cannot be specified for nonexistent field");
3332+
emit_atomic_error(ctx, "isdefined: atomic ordering cannot be specified for nonexistent field");
33333333
*ret = jl_cgval_t(); // unreachable
33343334
return true;
33353335
}
@@ -3338,12 +3338,12 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
33383338
}
33393339
bool isatomic = jl_field_isatomic(stt, fieldidx);
33403340
if (!isatomic && order != jl_memory_order_notatomic && order != jl_memory_order_unspecified) {
3341-
emit_atomic_error(ctx, "isdefined non-atomic field cannot be accessed atomically");
3341+
emit_atomic_error(ctx, "isdefined: non-atomic field cannot be accessed atomically");
33423342
*ret = jl_cgval_t(); // unreachable
33433343
return true;
33443344
}
33453345
if (isatomic && order == jl_memory_order_notatomic) {
3346-
emit_atomic_error(ctx, "isdefined atomic field cannot be accessed non-atomically");
3346+
emit_atomic_error(ctx, "isdefined: atomic field cannot be accessed non-atomically");
33473347
*ret = jl_cgval_t(); // unreachable
33483348
return true;
33493349
}
@@ -3379,7 +3379,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
33793379
else {
33803380
*ret = mark_julia_const(jl_true);
33813381
}
3382-
if (order > jl_memory_order_notatomic && ret->constant) {
3382+
if (order > jl_memory_order_monotonic && ret->constant) {
3383+
// fence instructions may only have acquire, release, acq_rel, or seq_cst ordering.
33833384
ctx.builder.CreateFence(get_llvm_atomic_order(order));
33843385
}
33853386
return true;

‎src/datatype.c

+6-7
Original file line numberDiff line numberDiff line change
@@ -1117,15 +1117,15 @@ JL_DLLEXPORT jl_value_t *jl_get_nth_field(jl_value_t *v, size_t i)
11171117
}
11181118
jl_value_t *ty = jl_field_type_concrete(st, i);
11191119
int isatomic = jl_field_isatomic(st, i);
1120-
size_t fsz = jl_field_size(st, i);
1121-
int needlock = (isatomic && fsz > MAX_ATOMIC_SIZE);
11221120
if (jl_is_uniontype(ty)) {
1121+
size_t fsz = jl_field_size(st, i);
11231122
uint8_t sel = ((uint8_t*)v)[offs + fsz - 1];
11241123
ty = jl_nth_union_component(ty, sel);
11251124
if (jl_is_datatype_singleton((jl_datatype_t*)ty))
11261125
return ((jl_datatype_t*)ty)->instance;
11271126
}
11281127
jl_value_t *r;
1128+
int needlock = (isatomic && jl_datatype_size(ty) > MAX_ATOMIC_SIZE);
11291129
if (isatomic && !needlock) {
11301130
r = jl_atomic_new_bits(ty, (char*)v + offs);
11311131
}
@@ -1159,7 +1159,8 @@ JL_DLLEXPORT jl_value_t *jl_get_nth_field_checked(jl_value_t *v, size_t i)
11591159
static inline void memassign_safe(int hasptr, jl_value_t *parent, char *dst, const jl_value_t *src, size_t nb) JL_NOTSAFEPOINT
11601160
{
11611161
if (hasptr) {
1162-
assert(nb >= jl_datatype_size(jl_typeof(src))); // dst might have some undefined bits, but the src heap box should be okay with that
1162+
// assert that although dst might have some undefined bits, the src heap box should be okay with that
1163+
assert(LLT_ALIGN(nb, sizeof(void*)) == LLT_ALIGN(jl_datatype_size(jl_typeof(src)), sizeof(void*)));
11631164
size_t nptr = nb / sizeof(void*);
11641165
memmove_refs((void**)dst, (void**)src, nptr);
11651166
jl_gc_multi_wb(parent, src);
@@ -1194,10 +1195,10 @@ void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs,
11941195
}
11951196
else {
11961197
jl_value_t *ty = jl_field_type_concrete(st, i);
1198+
jl_value_t *rty = jl_typeof(rhs);
11971199
size_t fsz = jl_field_size(st, i);
11981200
int hasptr;
11991201
if (jl_is_uniontype(ty)) {
1200-
jl_value_t *rty = jl_typeof(rhs);
12011202
uint8_t *psel = &((uint8_t*)v)[offs + fsz - 1];
12021203
unsigned nth = 0;
12031204
if (!jl_find_union_component(ty, rty, &nth))
@@ -1206,13 +1207,11 @@ void set_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rhs,
12061207
if (jl_is_datatype_singleton((jl_datatype_t*)rty))
12071208
return;
12081209
hasptr = 0;
1209-
fsz = jl_datatype_size((jl_datatype_t*)rty); // need to shrink-wrap the copy
12101210
}
12111211
else {
12121212
hasptr = ((jl_datatype_t*)ty)->layout->npointers > 0;
1213-
// should be safe enough to read fsz bytes from rhs
1214-
// due to gc alignment considerations
12151213
}
1214+
fsz = jl_datatype_size((jl_datatype_t*)rty); // need to shrink-wrap the final copy
12161215
int needlock = (isatomic && fsz > MAX_ATOMIC_SIZE);
12171216
if (isatomic && !needlock) {
12181217
jl_atomic_store_bits((char*)v + offs, rhs, fsz);

‎src/julia_internal.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1334,8 +1334,8 @@ extern jl_sym_t *acquire_sym;
13341334
extern jl_sym_t *release_sym;
13351335
extern jl_sym_t *acquire_release_sym;
13361336
extern jl_sym_t *sequentially_consistent_sym; // or strong_sym?
1337-
enum jl_memory_order jl_get_atomic_order(jl_sym_t *order);
1338-
enum jl_memory_order jl_get_atomic_order_checked(jl_sym_t *order);
1337+
enum jl_memory_order jl_get_atomic_order(jl_sym_t *order, char loading, char storing);
1338+
enum jl_memory_order jl_get_atomic_order_checked(jl_sym_t *order, char loading, char storing);
13391339

13401340
struct _jl_sysimg_fptrs_t;
13411341

‎src/runtime_intrinsics.c

+11-9
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointerref(jl_value_t *p, jl_value_t *order)
180180
{
181181
JL_TYPECHK(pointerref, pointer, p);
182182
JL_TYPECHK(pointerref, symbol, order)
183-
(void)jl_get_atomic_order_checked((jl_sym_t*)order);
183+
(void)jl_get_atomic_order_checked((jl_sym_t*)order, 1, 0);
184184
jl_value_t *ety = jl_tparam0(jl_typeof(p));
185185
if (ety == (jl_value_t*)jl_any_type) {
186186
jl_value_t **pp = (jl_value_t**)jl_unbox_long(p);
@@ -201,7 +201,7 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointerset(jl_value_t *p, jl_value_t *x, jl_v
201201
{
202202
JL_TYPECHK(pointerset, pointer, p);
203203
JL_TYPECHK(pointerset, symbol, order);
204-
(void)jl_get_atomic_order_checked((jl_sym_t*)order);
204+
(void)jl_get_atomic_order_checked((jl_sym_t*)order, 0, 1);
205205
jl_value_t *ety = jl_tparam0(jl_typeof(p));
206206
if (ety == (jl_value_t*)jl_any_type) {
207207
jl_value_t **pp = (jl_value_t**)jl_unbox_long(p);
@@ -225,7 +225,7 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointerswap(jl_value_t *p, jl_value_t *x, jl_
225225
{
226226
JL_TYPECHK(pointerswap, pointer, p);
227227
JL_TYPECHK(pointerswap, symbol, order);
228-
(void)jl_get_atomic_order_checked((jl_sym_t*)order);
228+
(void)jl_get_atomic_order_checked((jl_sym_t*)order, 1, 1);
229229
jl_value_t *ety = jl_tparam0(jl_typeof(p));
230230
jl_value_t *y;
231231
if (ety == (jl_value_t*)jl_any_type) {
@@ -264,13 +264,15 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointermodify(jl_value_t *p, jl_value_t *f, j
264264
}
265265
}
266266

267-
JL_DLLEXPORT jl_value_t *jl_atomic_pointercmpswap(jl_value_t *p, jl_value_t *x, jl_value_t *expected, jl_value_t *success_order, jl_value_t *failure_order)
267+
JL_DLLEXPORT jl_value_t *jl_atomic_pointercmpswap(jl_value_t *p, jl_value_t *x, jl_value_t *expected, jl_value_t *success_order_sym, jl_value_t *failure_order_sym)
268268
{
269269
JL_TYPECHK(pointercmpswap, pointer, p);
270-
JL_TYPECHK(pointercmpswap, symbol, success_order);
271-
JL_TYPECHK(pointercmpswap, symbol, failure_order);
272-
(void)jl_get_atomic_order_checked((jl_sym_t*)success_order);
273-
(void)jl_get_atomic_order_checked((jl_sym_t*)failure_order);
270+
JL_TYPECHK(pointercmpswap, symbol, success_order_sym);
271+
JL_TYPECHK(pointercmpswap, symbol, failure_order_sym);
272+
enum jl_memory_order success_order = jl_get_atomic_order_checked((jl_sym_t*)success_order_sym, 1, 1);
273+
enum jl_memory_order failure_order = jl_get_atomic_order_checked((jl_sym_t*)failure_order_sym, 1, 0);
274+
if (failure_order > success_order)
275+
jl_atomic_error("invalid atomic ordering");
274276
jl_value_t *ety = jl_tparam0(jl_typeof(p));
275277
int success;
276278
if (ety == (jl_value_t*)jl_any_type) {
@@ -307,7 +309,7 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointercmpswap(jl_value_t *p, jl_value_t *x,
307309
JL_DLLEXPORT jl_value_t *jl_atomic_fence(jl_value_t *order)
308310
{
309311
JL_TYPECHK(fence, symbol, order);
310-
(void)jl_get_atomic_order_checked((jl_sym_t*)order);
312+
(void)jl_get_atomic_order_checked((jl_sym_t*)order, 0, 0);
311313
jl_fence();
312314
return jl_nothing;
313315
}

0 commit comments

Comments
 (0)