Skip to content

Commit 50c8611

Browse files
vtjnashKristofferC
vtjnash
authored and
KristofferC
committed
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 (cherry picked from commit bd5e6da)
1 parent 0199dad commit 50c8611

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
@@ -932,7 +932,11 @@ static jl_cgval_t emit_atomic_pointerop(jl_codectx_t &ctx, intrinsic f, const jl
932932
bool isboxed;
933933
Type *ptrty = julia_type_to_llvm(ctx, ety, &isboxed);
934934
assert(!isboxed);
935-
Value *thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ);
935+
Value *thePtr;
936+
if (!type_is_ghost(ptrty))
937+
thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ);
938+
else
939+
thePtr = nullptr; // could use any value here, since typed_store will not use it
936940
jl_cgval_t ret = typed_store(ctx, thePtr, nullptr, x, y, ety, ctx.tbaa().tbaa_data, nullptr, nullptr, isboxed,
937941
llvm_order, llvm_failorder, nb, false, issetfield, isreplacefield, isswapfield, ismodifyfield, false, modifyop, "atomic_pointermodify");
938942
if (issetfield)

‎src/jltypes.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,7 @@ jl_datatype_t *jl_apply_modify_type(jl_value_t *dt)
11711171
return rettyp;
11721172
}
11731173

1174-
jl_datatype_t *jl_apply_cmpswap_type(jl_value_t *dt)
1174+
jl_datatype_t *jl_apply_cmpswap_type(jl_value_t *ty)
11751175
{
11761176
jl_value_t *params[2];
11771177
jl_value_t *names = jl_atomic_load_relaxed(&cmpswap_names);
@@ -1182,12 +1182,12 @@ jl_datatype_t *jl_apply_cmpswap_type(jl_value_t *dt)
11821182
if (jl_atomic_cmpswap(&cmpswap_names, &names, lnames))
11831183
names = jl_atomic_load_relaxed(&cmpswap_names); // == lnames
11841184
}
1185-
params[0] = dt;
1185+
params[0] = ty;
11861186
params[1] = (jl_value_t*)jl_bool_type;
1187-
jl_datatype_t *tuptyp = jl_apply_tuple_type_v(params, 2);
1188-
JL_GC_PROMISE_ROOTED(tuptyp); // (JL_ALWAYS_LEAFTYPE)
1189-
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2((jl_value_t*)jl_namedtuple_type, names, (jl_value_t*)tuptyp);
1190-
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
1187+
jl_value_t *tuptyp = (jl_value_t*)jl_apply_tuple_type_v(params, 2);
1188+
JL_GC_PUSH1(&tuptyp);
1189+
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2((jl_value_t*)jl_namedtuple_type, names, tuptyp);
1190+
JL_GC_POP();
11911191
return rettyp;
11921192
}
11931193

‎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
@@ -215,14 +215,14 @@ swap(i, j) = j
215215
for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Complex{Int512}, Any)
216216
r = Ref{TT}(10)
217217
GC.@preserve r begin
218-
(function (::Type{TT}) where TT
218+
(@noinline function (::Type{TT}) where TT
219219
p = Base.unsafe_convert(Ptr{TT}, r)
220220
T(x) = convert(TT, x)
221221
S = UInt32
222222
if TT !== Any
223223
@test_throws TypeError Core.Intrinsics.atomic_pointerset(p, S(1), :sequentially_consistent)
224-
@test_throws TypeError Core.Intrinsics.atomic_pointerswap(p, S(100), :sequentially_consistent)
225-
@test_throws TypeError Core.Intrinsics.atomic_pointerreplace(p, T(100), S(2), :sequentially_consistent, :sequentially_consistent)
224+
@test_throws TypeError Core.Intrinsics.atomic_pointerswap(p, S(2), :sequentially_consistent)
225+
@test_throws TypeError Core.Intrinsics.atomic_pointerreplace(p, T(10), S(3), :sequentially_consistent, :sequentially_consistent)
226226
end
227227
@test Core.Intrinsics.pointerref(p, 1, 1) === T(10) === r[]
228228
if sizeof(r) > 8
@@ -235,7 +235,10 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co
235235
@test_throws ErrorException("atomic_pointerreplace: invalid pointer for atomic operation") Core.Intrinsics.atomic_pointerreplace(p, S(100), T(2), :sequentially_consistent, :sequentially_consistent)
236236
@test Core.Intrinsics.pointerref(p, 1, 1) === T(10) === r[]
237237
else
238-
TT !== Any && @test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, swap, S(1), :sequentially_consistent)
238+
if TT !== Any
239+
@test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, swap, S(4), :sequentially_consistent)
240+
@test_throws TypeError Core.Intrinsics.atomic_pointermodify(p, Returns(S(5)), T(10), :sequentially_consistent)
241+
end
239242
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(10)
240243
@test Core.Intrinsics.atomic_pointerset(p, T(1), :sequentially_consistent) === p
241244
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(1)
@@ -249,10 +252,12 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co
249252
@test Core.Intrinsics.atomic_pointerswap(p, T(103), :sequentially_consistent) === T(102)
250253
@test Core.Intrinsics.atomic_pointerreplace(p, S(100), T(2), :sequentially_consistent, :sequentially_consistent) === ReplaceType{TT}((T(103), false))
251254
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(103)
255+
@test Core.Intrinsics.atomic_pointermodify(p, Returns(T(105)), nothing, :sequentially_consistent) === Pair{TT,TT}(T(103), T(105))
256+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === T(105)
252257
end
253258
if TT === Any
254-
@test Core.Intrinsics.atomic_pointermodify(p, swap, S(103), :sequentially_consistent) === Pair{TT,TT}(T(103), S(103))
255-
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === S(103)
259+
@test Core.Intrinsics.atomic_pointermodify(p, swap, S(105), :sequentially_consistent) === Pair{TT,TT}(T(105), S(105))
260+
@test Core.Intrinsics.atomic_pointerref(p, :sequentially_consistent) === S(105)
256261
@test Core.Intrinsics.atomic_pointerset(p, S(1), :sequentially_consistent) === p
257262
@test Core.Intrinsics.atomic_pointerswap(p, S(100), :sequentially_consistent) === S(1)
258263
@test Core.Intrinsics.atomic_pointerreplace(p, T(100), S(2), :sequentially_consistent, :sequentially_consistent) === ReplaceType{TT}((S(100), false))
@@ -263,6 +268,37 @@ for TT in (Int8, Int16, Int32, Int64, Int128, Int256, Int512, Complex{Int32}, Co
263268
end
264269
end
265270

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

0 commit comments

Comments
 (0)