Skip to content

Commit bd5e6da

Browse files
vtjnashvtjnash
authored
fix atomic intrinsics implementation issues (#49967)
* jltypes: add missing GC root for cmpswap_type Tuple. This is called with a fieldtype, which might not even be a DataType. * support Ptr{Union{}} and Ptr{Cvoid} better
1 parent f6f637a commit bd5e6da

File tree

4 files changed

+55
-15
lines changed

4 files changed

+55
-15
lines changed

‎src/intrinsics.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,11 @@ static jl_cgval_t emit_atomic_pointerop(jl_codectx_t &ctx, intrinsic f, const jl
926926
bool isboxed;
927927
Type *ptrty = julia_type_to_llvm(ctx, ety, &isboxed);
928928
assert(!isboxed);
929-
Value *thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ);
929+
Value *thePtr;
930+
if (!type_is_ghost(ptrty))
931+
thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ);
932+
else
933+
thePtr = nullptr; // could use any value here, since typed_store will not use it
930934
jl_cgval_t ret = typed_store(ctx, thePtr, nullptr, x, y, ety, ctx.tbaa().tbaa_data, nullptr, nullptr, isboxed,
931935
llvm_order, llvm_failorder, nb, false, issetfield, isreplacefield, isswapfield, ismodifyfield, false, modifyop, "atomic_pointermodify");
932936
if (issetfield)

‎src/jltypes.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,7 +1411,7 @@ jl_datatype_t *jl_apply_modify_type(jl_value_t *dt)
14111411
return rettyp;
14121412
}
14131413

1414-
jl_datatype_t *jl_apply_cmpswap_type(jl_value_t *dt)
1414+
jl_datatype_t *jl_apply_cmpswap_type(jl_value_t *ty)
14151415
{
14161416
jl_value_t *params[2];
14171417
jl_value_t *names = jl_atomic_load_relaxed(&cmpswap_names);
@@ -1422,12 +1422,12 @@ jl_datatype_t *jl_apply_cmpswap_type(jl_value_t *dt)
14221422
if (jl_atomic_cmpswap(&cmpswap_names, &names, lnames))
14231423
names = jl_atomic_load_relaxed(&cmpswap_names); // == lnames
14241424
}
1425-
params[0] = dt;
1425+
params[0] = ty;
14261426
params[1] = (jl_value_t*)jl_bool_type;
1427-
jl_datatype_t *tuptyp = (jl_datatype_t*)jl_apply_tuple_type_v(params, 2);
1428-
JL_GC_PROMISE_ROOTED(tuptyp); // (JL_ALWAYS_LEAFTYPE)
1429-
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2((jl_value_t*)jl_namedtuple_type, names, (jl_value_t*)tuptyp);
1430-
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
1427+
jl_value_t *tuptyp = jl_apply_tuple_type_v(params, 2);
1428+
JL_GC_PUSH1(&tuptyp);
1429+
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2((jl_value_t*)jl_namedtuple_type, names, tuptyp);
1430+
JL_GC_POP();
14311431
return rettyp;
14321432
}
14331433

‎src/runtime_intrinsics.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,8 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointerreplace(jl_value_t *p, jl_value_t *exp
429429
jl_atomic_error("atomic_pointerreplace: invalid atomic ordering");
430430
// TODO: filter other invalid orderings
431431
jl_value_t *ety = jl_tparam0(jl_typeof(p));
432+
if (!is_valid_intrinsic_elptr(ety))
433+
jl_error("atomic_pointerreplace: invalid pointer");
432434
char *pp = (char*)jl_unbox_long(p);
433435
jl_datatype_t *rettyp = jl_apply_cmpswap_type(ety);
434436
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
@@ -447,8 +449,6 @@ JL_DLLEXPORT jl_value_t *jl_atomic_pointerreplace(jl_value_t *p, jl_value_t *exp
447449
return result;
448450
}
449451
else {
450-
if (!is_valid_intrinsic_elptr(ety))
451-
jl_error("atomic_pointerreplace: invalid pointer");
452452
if (jl_typeof(x) != ety)
453453
jl_type_error("atomic_pointerreplace", ety, x);
454454
size_t nb = jl_datatype_size(ety);

‎test/intrinsics.jl

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,14 @@ swap(i, j) = j
214214
for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Complex{Int512}, Any)
215215
r = Ref{TT}(10)
216216
GC.@preserve r begin
217-
(function (::Type{TT}) where TT
217+
(@noinline function (::Type{TT}) where TT
218218
p = Base.unsafe_convert(Ptr{TT}, r)
219219
T(x) = convert(TT, x)
220220
S = UInt32
221221
if TT !== Any
222222
@test_throws TypeError Core.Intrinsics.atomic_pointerset(p, S(1), :sequentially_consistent)
223-
@test_throws TypeError Core.Intrinsics.atomic_pointerswap(p, S(100), :sequentially_consistent)
224-
@test_throws TypeError Core.Intrinsics.atomic_pointerreplace(p, T(100), S(2), :sequentially_consistent, :sequentially_consistent)
223+
@test_throws TypeError Core.Intrinsics.atomic_pointerswap(p, S(2), :sequentially_consistent)
224+
@test_throws TypeError Core.Intrinsics.atomic_pointerreplace(p, T(10), S(3), :sequentially_consistent, :sequentially_consistent)
225225
end
226226
@test Core.Intrinsics.pointerref(p, 1, 1) === T(10) === r[]
227227
if sizeof(r) > 8
@@ -234,7 +234,10 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co
234234
@test_throws ErrorException("atomic_pointerreplace: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerreplace(p, S(100), T(2), :sequentially_consistent, :sequentially_consistent)
235235
@test Core.Intrinsics.pointerref(p, 1, 1) === T(10) === r[]
236236
else
237-
TT !== Any && @test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, swap, S(1), :sequentially_consistent)
237+
if TT !== Any
238+
@test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, swap, S(4), :sequentially_consistent)
239+
@test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, Returns(S(5)), T(10), :sequentially_consistent)
240+
end
238241
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(10)
239242
@test Core.Intrinsics.atomic_pointerset(p, T(1), :sequentially_consistent) === p
240243
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(1)
@@ -248,10 +251,12 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co
248251
@test Core.Intrinsics.atomic_pointerswap(p, T(103), :sequentially_consistent) === T(102)
249252
@test Core.Intrinsics.atomic_pointerreplace(p, S(100), T(2), :sequentially_consistent, :sequentially_consistent) === ReplaceType{TT}((T(103), false))
250253
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(103)
254+
@test Core.Intrinsics.atomic_pointermodify(p, Returns(T(105)), nothing, :sequentially_consistent) === Pair{TT,TT}(T(103), T(105))
255+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(105)
251256
end
252257
if TT === Any
253-
@test Core.Intrinsics.atomic_pointermodify(p, swap, S(103), :sequentially_consistent) === Pair{TT,TT}(T(103), S(103))
254-
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === S(103)
258+
@test Core.Intrinsics.atomic_pointermodify(p, swap, S(105), :sequentially_consistent) === Pair{TT,TT}(T(105), S(105))
259+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === S(105)
255260
@test Core.Intrinsics.atomic_pointerset(p, S(1), :sequentially_consistent) === p
256261
@test Core.Intrinsics.atomic_pointerswap(p, S(100), :sequentially_consistent) === S(1)
257262
@test Core.Intrinsics.atomic_pointerreplace(p, T(100), S(2), :sequentially_consistent, :sequentially_consistent) === ReplaceType{TT}((S(100), false))
@@ -262,6 +267,37 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co
262267
end
263268
end
264269

270+
for TT in (Ptr{Nothing}, Ptr)
271+
r = Ref(nothing)
272+
GC.@preserve r begin
273+
p = Ref{TT}(Base.unsafe_convert(Ptr{Nothing}, r))
274+
(@noinline function (p::Ref)
275+
p = p[]
276+
S = UInt32
277+
@test_throws TypeError Core.Intrinsics.atomic_pointerset(p, S(1), :sequentially_consistent)
278+
@test_throws TypeError Core.Intrinsics.atomic_pointerswap(p, S(100), :sequentially_consistent)
279+
@test_throws TypeError Core.Intrinsics.atomic_pointerreplace(p, nothing, S(2), :sequentially_consistent, :sequentially_consistent)
280+
@test Core.Intrinsics.pointerref(p, 1, 1) === nothing === r[]
281+
@test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, swap, S(1), :sequentially_consistent)
282+
@test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, Returns(S(1)), nothing, :sequentially_consistent)
283+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === nothing
284+
@test Core.Intrinsics.atomic_pointerset(p, nothing, :sequentially_consistent) === p
285+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === nothing
286+
@test Core.Intrinsics.atomic_pointerreplace(p, nothing, nothing, :sequentially_consistent, :sequentially_consistent) === ReplaceType{Nothing}((nothing, true))
287+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === nothing
288+
@test Core.Intrinsics.atomic_pointerreplace(p, S(1), nothing, :sequentially_consistent, :sequentially_consistent) === ReplaceType{Nothing}((nothing, false))
289+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === nothing
290+
@test Core.Intrinsics.atomic_pointermodify(p, Returns(nothing), nothing, :sequentially_consistent) === Pair{Nothing,Nothing}(nothing, nothing)
291+
@test Core.Intrinsics.atomic_pointermodify(p, Returns(nothing), S(1), :sequentially_consistent) === Pair{Nothing,Nothing}(nothing, nothing)
292+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === nothing
293+
@test Core.Intrinsics.atomic_pointerswap(p, nothing, :sequentially_consistent) === nothing
294+
@test Core.Intrinsics.atomic_pointerreplace(p, S(100), nothing, :sequentially_consistent, :sequentially_consistent) === ReplaceType{Nothing}((nothing, false))
295+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === nothing
296+
end)(p,)
297+
end
298+
end
299+
300+
265301
mutable struct IntWrap <: Signed
266302
x::Int
267303
end

0 commit comments

Comments
 (0)