Skip to content

Commit cff1610

Browse files
committed
atomics: switch to using Pair for return pairs
This makes many more functions type-stable, by directly preserving the element type when making the copy, and prints as `old => new`, like the argument to atomic replace.
1 parent e196b85 commit cff1610

File tree

17 files changed

+162
-125
lines changed

17 files changed

+162
-125
lines changed

base/Base.jl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ include("options.jl")
107107
include("promotion.jl")
108108
include("tuple.jl")
109109
include("expr.jl")
110+
Pair{A, B}(@nospecialize(a), @nospecialize(b)) where {A, B} = (@_inline_meta; Pair{A, B}(convert(A, a)::A, convert(B, b)::B))
111+
#Pair{Any, B}(@nospecialize(a::Any), b) where {B} = (@_inline_meta; Pair{Any, B}(a, Base.convert(B, b)::B))
112+
#Pair{A, Any}(a, @nospecialize(b::Any)) where {A} = (@_inline_meta; Pair{A, Any}(Base.convert(A, a)::A, b))
110113
include("pair.jl")
111114
include("traits.jl")
112115
include("range.jl")

base/boot.jl

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export
171171
# key types
172172
Any, DataType, Vararg, NTuple,
173173
Tuple, Type, UnionAll, TypeVar, Union, Nothing, Cvoid,
174-
AbstractArray, DenseArray, NamedTuple,
174+
AbstractArray, DenseArray, NamedTuple, Pair,
175175
# special objects
176176
Function, Method,
177177
Module, Symbol, Task, Array, UndefInitializer, undef, WeakRef, VecElement,
@@ -813,4 +813,16 @@ _parse = nothing
813813
# support for deprecated uses of internal _apply function
814814
_apply(x...) = Core._apply_iterate(Main.Base.iterate, x...)
815815

816+
struct Pair{A, B}
817+
first::A
818+
second::B
819+
# if we didn't inline this, it's probably because the callsite was actually dynamic
820+
# to avoid potentially compiling many copies of this, we mark the arguments with `@nospecialize`
821+
# but also mark the whole function with `@inline` to ensure we will inline it whenever possible
822+
# (even if `convert(::Type{A}, a::A)` for some reason was expensive)
823+
Pair(a, b) = new{typeof(a), typeof(b)}(a, b)
824+
Pair{A, B}(a::A, b::B) where {A, B} = new(a, b)
825+
Pair{Any, Any}(@nospecialize(a::Any), @nospecialize(b::Any)) = new(a, b)
826+
end
827+
816828
ccall(:jl_set_istopmod, Cvoid, (Any, Bool), Core, true)

base/compiler/tfuncs.jl

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -466,29 +466,51 @@ add_tfunc(Core._typevar, 3, 3, typevar_tfunc, 100)
466466
add_tfunc(applicable, 1, INT_INF, (@nospecialize(f), args...)->Bool, 100)
467467
add_tfunc(Core.Intrinsics.arraylen, 1, 1, @nospecialize(x)->Int, 4)
468468
add_tfunc(arraysize, 2, 2, (@nospecialize(a), @nospecialize(d))->Int, 4)
469+
469470
function pointer_eltype(@nospecialize(ptr))
470471
a = widenconst(ptr)
471-
if a <: Ptr
472-
if isa(a, DataType) && isa(a.parameters[1], Type)
473-
return a.parameters[1]
474-
elseif isa(a, UnionAll) && !has_free_typevars(a)
475-
unw = unwrap_unionall(a)
476-
if isa(unw, DataType)
477-
return rewrap_unionall(unw.parameters[1], a)
478-
end
472+
if !has_free_typevars(a)
473+
unw = unwrap_unionall(a)
474+
if isa(unw, DataType) && unw.name === Ptr.body.name
475+
T = unw.parameters[1]
476+
T isa Type && return rewrap_unionall(T, a)
479477
end
480478
end
481479
return Any
482480
end
481+
function atomic_pointermodify_tfunc(ptr, op, v, order)
482+
@nospecialize
483+
a = widenconst(ptr)
484+
if !has_free_typevars(a)
485+
unw = unwrap_unionall(a)
486+
if isa(unw, DataType) && unw.name === Ptr.body.name
487+
T = unw.parameters[1]
488+
# note: we could sometimes refine this to a PartialStruct if we analyzed `op(T, T)::T`
489+
T isa Type && return rewrap_unionall(Pair{T, T}, a)
490+
end
491+
end
492+
return Pair
493+
end
494+
function atomic_pointerreplace_tfunc(ptr, x, v, success_order, failure_order)
495+
@nospecialize
496+
a = widenconst(ptr)
497+
if !has_free_typevars(a)
498+
unw = unwrap_unionall(a)
499+
if isa(unw, DataType) && unw.name === Ptr.body.name
500+
T = unw.parameters[1]
501+
T isa Type && return rewrap_unionall(Pair{T, Bool}, a)
502+
end
503+
end
504+
return Pair{T,Bool} where T
505+
end
483506
add_tfunc(pointerref, 3, 3, (a, i, align) -> (@nospecialize; pointer_eltype(a)), 4)
484507
add_tfunc(pointerset, 4, 4, (a, v, i, align) -> (@nospecialize; a), 5)
485-
486508
add_tfunc(atomic_fence, 1, 1, (order) -> (@nospecialize; Nothing), 4)
487509
add_tfunc(atomic_pointerref, 2, 2, (a, order) -> (@nospecialize; pointer_eltype(a)), 4)
488510
add_tfunc(atomic_pointerset, 3, 3, (a, v, order) -> (@nospecialize; a), 5)
489511
add_tfunc(atomic_pointerswap, 3, 3, (a, v, order) -> (@nospecialize; pointer_eltype(a)), 5)
490-
add_tfunc(atomic_pointermodify, 4, 4, (a, op, v, order) -> (@nospecialize; T = pointer_eltype(a); Tuple{T, T}), 5)
491-
add_tfunc(atomic_pointerreplace, 5, 5, (a, x, v, success_order, failure_order) -> (@nospecialize; Tuple{pointer_eltype(a), Bool}), 5)
512+
add_tfunc(atomic_pointermodify, 4, 4, atomic_pointermodify_tfunc, 5)
513+
add_tfunc(atomic_pointerreplace, 5, 5, atomic_pointerreplace_tfunc, 5)
492514

493515
# more accurate typeof_tfunc for vararg tuples abstract only in length
494516
function typeof_concrete_vararg(t::DataType)
@@ -911,11 +933,23 @@ setfield!_tfunc(o, f, v) = (@nospecialize; v)
911933

912934
swapfield!_tfunc(o, f, v, order) = (@nospecialize; getfield_tfunc(o, f))
913935
swapfield!_tfunc(o, f, v) = (@nospecialize; getfield_tfunc(o, f))
914-
modifyfield!_tfunc(o, f, op, v, order) = (@nospecialize; T = getfield_tfunc(o, f); T === Bottom ? T : Tuple{T, T})
915-
modifyfield!_tfunc(o, f, op, v) = (@nospecialize; T = getfield_tfunc(o, f); T === Bottom ? T : Tuple{T, T}) # TODO: also model op(o.f, v) call
936+
modifyfield!_tfunc(o, f, op, v, order) = (@nospecialize; modifyfield!_tfunc(o, f, op, v))
937+
function modifyfield!_tfunc(o, f, op, v)
938+
@nospecialize
939+
T = _fieldtype_tfunc(o, isconcretetype(o), f)
940+
T === Bottom && return Bottom
941+
# note: we could sometimes refine this to a PartialStruct if we analyzed `op(o.f, v)::T`
942+
return instanceof_tfunc(apply_type_tfunc(Const(Pair), T, T))[1]
943+
end
916944
replacefield!_tfunc(o, f, x, v, success_order, failure_order) = (@nospecialize; replacefield!_tfunc(o, f, x, v))
917945
replacefield!_tfunc(o, f, x, v, success_order) = (@nospecialize; replacefield!_tfunc(o, f, x, v))
918-
replacefield!_tfunc(o, f, x, v) = (@nospecialize; T = getfield_tfunc(o, f); T === Bottom ? T : Tuple{widenconst(T), Bool})
946+
function replacefield!_tfunc(o, f, x, v)
947+
@nospecialize
948+
T = _fieldtype_tfunc(o, isconcretetype(o), f)
949+
T === Bottom && return Bottom
950+
return instanceof_tfunc(apply_type_tfunc(Const(Pair), T, Const(Bool)))[1]
951+
end
952+
919953
# we could use tuple_tfunc instead of widenconst, but `o` is mutable, so that is unlikely to be beneficial
920954

921955
add_tfunc(getfield, 2, 4, getfield_tfunc, 1)

base/pair.jl

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,5 @@
11
# This file is a part of Julia. License is MIT: https://julialang.org/license
22

3-
struct Pair{A, B}
4-
first::A
5-
second::B
6-
function Pair{A, B}(@nospecialize(a), @nospecialize(b)) where {A, B}
7-
@_inline_meta
8-
# if we didn't inline this, it's probably because the callsite was actually dynamic
9-
# to avoid potentially compiling many copies of this, we mark the arguments with `@nospecialize`
10-
# but also mark the whole function with `@inline` to ensure we will inline it whenever possible
11-
# (even if `convert(::Type{A}, a::A)` for some reason was expensive)
12-
return new(a, b)
13-
end
14-
end
15-
Pair(a, b) = Pair{typeof(a), typeof(b)}(a, b)
163
const => = Pair
174

185
"""

src/cgutils.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,9 +1560,8 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
15601560
Value *Success = emit_f_is(ctx, cmp, ghostValue(jltype));
15611561
Success = ctx.builder.CreateZExt(Success, T_int8);
15621562
jl_cgval_t argv[2] = {ghostValue(jltype), mark_julia_type(ctx, Success, false, jl_bool_type)};
1563-
// TODO: do better here
1564-
Value *instr = emit_jlcall(ctx, jltuple_func, V_rnull, argv, 2, JLCALL_F_CC);
1565-
return mark_julia_type(ctx, instr, true, jl_any_type);
1563+
jl_value_t *rettyp = jl_apply_type2(jl_pair_type, jltype, (jl_value_t*)jl_bool_type);
1564+
return emit_new_struct(ctx, rettyp, 2, argv);
15661565
}
15671566
else {
15681567
return ghostValue(jltype);
@@ -1803,10 +1802,10 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
18031802
}
18041803
oldval = mark_julia_type(ctx, instr, isboxed, jltype);
18051804
if (isreplacefield) {
1806-
// TODO: do better here
1805+
Success = ctx.builder.CreateZExt(Success, T_int8);
18071806
jl_cgval_t argv[2] = {oldval, mark_julia_type(ctx, Success, false, jl_bool_type)};
1808-
instr = emit_jlcall(ctx, jltuple_func, V_rnull, argv, 2, JLCALL_F_CC);
1809-
oldval = mark_julia_type(ctx, instr, true, jl_any_type);
1807+
jl_value_t *rettyp = jl_apply_type2(jl_pair_type, jltype, (jl_value_t*)jl_bool_type);
1808+
oldval = emit_new_struct(ctx, rettyp, 2, argv);
18101809
}
18111810
}
18121811
return oldval;
@@ -3247,10 +3246,10 @@ static jl_cgval_t emit_setfield(jl_codectx_t &ctx,
32473246
if (needlock)
32483247
emit_lockstate_value(ctx, strct, false);
32493248
if (isreplacefield) {
3249+
Success = ctx.builder.CreateZExt(Success, T_int8);
32503250
jl_cgval_t argv[2] = {oldval, mark_julia_type(ctx, Success, false, jl_bool_type)};
3251-
// TODO: do better here
3252-
Value *instr = emit_jlcall(ctx, jltuple_func, V_rnull, argv, 2, JLCALL_F_CC);
3253-
oldval = mark_julia_type(ctx, instr, true, jl_any_type);
3251+
jl_value_t *rettyp = jl_apply_type2(jl_pair_type, jfty, (jl_value_t*)jl_bool_type);
3252+
oldval = emit_new_struct(ctx, rettyp, 2, argv);
32543253
}
32553254
return oldval;
32563255
}

src/codegen.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,6 +1164,7 @@ static CallInst *emit_jlcall(jl_codectx_t &ctx, JuliaFunction *theFptr, Value *t
11641164
jl_cgval_t *args, size_t nargs, CallingConv::ID cc);
11651165
static Value *emit_f_is(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgval_t &arg2,
11661166
Value *nullcheck1 = nullptr, Value *nullcheck2 = nullptr);
1167+
static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t nargs, const jl_cgval_t *argv);
11671168

11681169
static Value *literal_pointer_val(jl_codectx_t &ctx, jl_value_t *p);
11691170
static GlobalVariable *prepare_global_in(Module *M, GlobalVariable *G);

src/datatype.c

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -957,14 +957,11 @@ JL_DLLEXPORT jl_value_t *jl_atomic_cmpswap_bits(jl_datatype_t *dt, char *dst, co
957957
{
958958
// dst must have the required alignment for an atomic of the given size
959959
// n.b.: this does not spuriously fail if there are padding bits
960-
jl_value_t *params[2];
961-
params[0] = (jl_value_t*)dt;
962-
params[1] = (jl_value_t*)jl_bool_type;
963-
jl_datatype_t *tuptyp = jl_apply_tuple_type_v(params, 2);
964-
JL_GC_PROMISE_ROOTED(tuptyp); // (JL_ALWAYS_LEAFTYPE)
965-
int isptr = jl_field_isptr(tuptyp, 0);
960+
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2(jl_pair_type, (jl_value_t*)dt, (jl_value_t*)jl_bool_type);
961+
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
966962
jl_task_t *ct = jl_current_task;
967-
jl_value_t *y = jl_gc_alloc(ct->ptls, isptr ? nb : tuptyp->size, isptr ? dt : tuptyp);
963+
int isptr = jl_field_isptr(rettyp, 0);
964+
jl_value_t *y = jl_gc_alloc(ct->ptls, isptr ? nb : rettyp->size, isptr ? dt : rettyp);
968965
int success;
969966
jl_datatype_t *et = (jl_datatype_t*)jl_typeof(expected);
970967
if (nb == 0) {
@@ -1053,7 +1050,7 @@ JL_DLLEXPORT jl_value_t *jl_atomic_cmpswap_bits(jl_datatype_t *dt, char *dst, co
10531050
}
10541051
if (isptr) {
10551052
JL_GC_PUSH1(&y);
1056-
jl_value_t *z = jl_gc_alloc(ct->ptls, tuptyp->size, tuptyp);
1053+
jl_value_t *z = jl_gc_alloc(ct->ptls, rettyp->size, rettyp);
10571054
*(jl_value_t**)z = y;
10581055
JL_GC_POP();
10591056
y = z;
@@ -1658,8 +1655,11 @@ jl_value_t *modify_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_valu
16581655
args[0] = r;
16591656
jl_gc_safepoint();
16601657
}
1661-
// args[0] == r (old); args[1] == y (new)
1662-
args[0] = jl_f_tuple(NULL, args, 2);
1658+
// args[0] == r (old)
1659+
// args[1] == y (new)
1660+
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2(jl_pair_type, ty, ty);
1661+
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
1662+
args[0] = jl_new_struct(rettyp, args[0], args[1]);
16631663
JL_GC_POP();
16641664
return args[0];
16651665
}
@@ -1683,19 +1683,18 @@ jl_value_t *replace_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_val
16831683
if (success || !jl_egal(r, expected))
16841684
break;
16851685
}
1686-
jl_value_t **args;
1687-
JL_GC_PUSHARGS(args, 2);
1688-
args[0] = r;
1689-
args[1] = success ? jl_true : jl_false;
1690-
r = jl_f_tuple(NULL, args, 2);
1686+
JL_GC_PUSH1(&r);
1687+
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2(jl_pair_type, ty, (jl_value_t*)jl_bool_type);
1688+
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
1689+
r = jl_new_struct(rettyp, r, success ? jl_true : jl_false);
16911690
JL_GC_POP();
16921691
}
16931692
else {
16941693
int hasptr;
16951694
int isunion = jl_is_uniontype(ty);
16961695
int needlock;
16971696
jl_value_t *rty = ty;
1698-
size_t fsz;
1697+
size_t fsz = jl_field_size(st, i);
16991698
if (isunion) {
17001699
assert(!isatomic);
17011700
hasptr = 0;
@@ -1708,7 +1707,7 @@ jl_value_t *replace_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_val
17081707
needlock = (isatomic && fsz > MAX_ATOMIC_SIZE);
17091708
}
17101709
if (isatomic && !needlock) {
1711-
r = jl_atomic_cmpswap_bits((jl_datatype_t*)rty, (char*)v + offs, r, rhs, fsz);
1710+
r = jl_atomic_cmpswap_bits((jl_datatype_t*)ty, (char*)v + offs, r, rhs, fsz);
17121711
int success = *((uint8_t*)r + fsz);
17131712
if (success && hasptr)
17141713
jl_gc_multi_wb(v, rhs); // rhs is immutable
@@ -1717,23 +1716,19 @@ jl_value_t *replace_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_val
17171716
jl_task_t *ct = jl_current_task;
17181717
uint8_t *psel;
17191718
if (isunion) {
1720-
size_t fsz = jl_field_size(st, i);
17211719
psel = &((uint8_t*)v)[offs + fsz - 1];
17221720
rty = jl_nth_union_component(rty, *psel);
17231721
}
1724-
jl_value_t *params[2];
1725-
params[0] = rty;
1726-
params[1] = (jl_value_t*)jl_bool_type;
1727-
jl_datatype_t *tuptyp = jl_apply_tuple_type_v(params, 2);
1728-
JL_GC_PROMISE_ROOTED(tuptyp); // (JL_ALWAYS_LEAFTYPE)
1729-
assert(!jl_field_isptr(tuptyp, 0));
1730-
r = jl_gc_alloc(ct->ptls, tuptyp->size, (jl_value_t*)tuptyp);
1722+
jl_datatype_t *rettyp = (jl_datatype_t*)jl_apply_type2(jl_pair_type, ty, (jl_value_t*)jl_bool_type);
1723+
JL_GC_PROMISE_ROOTED(rettyp); // (JL_ALWAYS_LEAFTYPE)
1724+
assert(!jl_field_isptr(rettyp, 0));
1725+
r = jl_gc_alloc(ct->ptls, rettyp->size, (jl_value_t*)rettyp);
17311726
int success = (rty == jl_typeof(expected));
17321727
if (needlock)
17331728
jl_lock_value(v);
1734-
size_t fsz = jl_datatype_size((jl_datatype_t*)rty); // need to shrink-wrap the final copy
1735-
memcpy((char*)r, (char*)v + offs, fsz);
1729+
memcpy((char*)r, (char*)v + offs, fsz); // copy field, including union bits
17361730
if (success) {
1731+
size_t fsz = jl_datatype_size((jl_datatype_t*)rty); // need to shrink-wrap the final copy
17371732
if (((jl_datatype_t*)rty)->layout->haspadding)
17381733
success = jl_egal__bits(r, expected, (jl_datatype_t*)rty);
17391734
else

src/init.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,7 @@ static void post_boot_hooks(void)
840840
jl_methoderror_type = (jl_datatype_t*)core("MethodError");
841841
jl_loaderror_type = (jl_datatype_t*)core("LoadError");
842842
jl_initerror_type = (jl_datatype_t*)core("InitError");
843+
jl_pair_type = core("Pair");
843844

844845
jl_weakref_type = (jl_datatype_t*)core("WeakRef");
845846
jl_vecelement_typename = ((jl_datatype_t*)jl_unwrap_unionall(core("VecElement")))->name;

src/jl_exported_data.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
XX(jl_number_type) \
7575
XX(jl_opaque_closure_type) \
7676
XX(jl_opaque_closure_typename) \
77+
XX(jl_pair_type) \
7778
XX(jl_partial_opaque_type) \
7879
XX(jl_partial_struct_type) \
7980
XX(jl_phicnode_type) \

src/jl_exported_funcs.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,3 +543,4 @@
543543
XX(jl_vprintf) \
544544
XX(jl_wakeup_thread) \
545545
XX(jl_yield) \
546+

0 commit comments

Comments
 (0)