From c8662b593a245e3d1efa5b0d2b60175cfc23ebc7 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 7 Dec 2022 17:15:33 -0500 Subject: [PATCH] more field property annotations (#47677) Mostly looking at fixing more implicit data race with more atomics. Some of this (at the Julia level) is not strictly necessary since we already intend to start using implicit release-consume ordering for references. But since the user should not be changing these fields anyways, or risk severe breakage, this should be fine to mark explicitly. --- base/Base.jl | 2 +- base/compiler/tfuncs.jl | 58 ++++++++++------- base/operators.jl | 2 +- base/reflection.jl | 23 +++++++ doc/src/base/base.md | 1 + src/Makefile | 2 +- src/ccall.cpp | 10 +-- src/codegen.cpp | 76 +++++++++++++---------- src/datatype.c | 2 +- src/gf.c | 29 ++++----- src/init.c | 2 +- src/interpreter.c | 4 +- src/jitlayers.cpp | 2 +- src/jltypes.c | 26 ++++++-- src/julia.h | 54 +++++++--------- src/method.c | 24 ++++--- src/module.c | 69 +++++++++++--------- src/rtutils.c | 2 +- src/stackwalk.c | 2 +- src/staticdata.c | 4 +- src/toplevel.c | 2 +- stdlib/Serialization/src/Serialization.jl | 4 +- test/atomics.jl | 6 +- test/core.jl | 18 +++++- test/stacktraces.jl | 2 +- 25 files changed, 259 insertions(+), 167 deletions(-) diff --git a/base/Base.jl b/base/Base.jl index 29a6f9ed4366d..0c53a8bc9124b 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -112,7 +112,7 @@ function Core.kwcall(kwargs, ::typeof(invoke), f, T, args...) return invoke(Core.kwcall, T, kwargs, f, args...) end # invoke does not have its own call cache, but kwcall for invoke does -typeof(invoke).name.mt.max_args = 3 # invoke, f, T, args... +setfield!(typeof(invoke).name.mt, :max_args, 3, :monotonic) # invoke, f, T, args... # core operations & types include("promotion.jl") diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl index 50c2f1e15a089..82ed8f19a37b6 100644 --- a/base/compiler/tfuncs.jl +++ b/base/compiler/tfuncs.jl @@ -809,13 +809,14 @@ function try_compute_fieldidx(typ::DataType, @nospecialize(field)) return field end -function getfield_boundscheck(argtypes::Vector{Any}) # ::Union{Bool, Nothing, Type{Bool}} +function getfield_boundscheck(argtypes::Vector{Any}) # ::Union{Bool, Nothing} if length(argtypes) == 2 return true elseif length(argtypes) == 3 boundscheck = argtypes[3] - if boundscheck === Const(:not_atomic) # TODO: this is assuming not atomic - boundscheck = Bool + isvarargtype(boundscheck) && return nothing + if widenconst(boundscheck) === Symbol + return true end elseif length(argtypes) == 4 boundscheck = argtypes[4] @@ -823,22 +824,43 @@ function getfield_boundscheck(argtypes::Vector{Any}) # ::Union{Bool, Nothing, Ty return nothing end isvarargtype(boundscheck) && return nothing - widenconst(boundscheck) !== Bool && return nothing + widenconst(boundscheck) === Bool || return nothing boundscheck = widenconditional(boundscheck) if isa(boundscheck, Const) - return boundscheck.val + return boundscheck.val::Bool else - return Bool + return nothing end end -function getfield_nothrow(argtypes::Vector{Any}) - boundscheck = getfield_boundscheck(argtypes) +function getfield_nothrow(argtypes::Vector{Any}, boundscheck::Union{Bool,Nothing}=getfield_boundscheck(argtypes)) + @specialize boundscheck boundscheck === nothing && return false - return getfield_nothrow(argtypes[1], argtypes[2], !(boundscheck === false)) + ordering = Const(:not_atomic) + if length(argtypes) == 3 + isvarargtype(argtypes[3]) && return false + if widenconst(argtypes[3]) !== Bool + ordering = argtypes[3] + end + elseif length(argtypes) == 4 + ordering = argtypes[4] + elseif length(argtypes) != 2 + return false + end + isvarargtype(ordering) && return false + widenconst(ordering) === Symbol || return false + if isa(ordering, Const) + ordering = ordering.val::Symbol + if ordering !== :not_atomic # TODO: this is assuming not atomic + return false + end + return getfield_nothrow(argtypes[1], argtypes[2], !(boundscheck === false)) + else + return false + end end function getfield_nothrow(@nospecialize(s00), @nospecialize(name), boundscheck::Bool) - # If we don't have boundscheck and don't know the field, don't even bother + # If we don't have boundscheck off and don't know the field, don't even bother if boundscheck isa(name, Const) || return false end @@ -880,7 +902,6 @@ function getfield_nothrow(@nospecialize(s00), @nospecialize(name), boundscheck:: if isa(s, DataType) # Can't say anything about abstract types isabstracttype(s) && return false - s.name.atomicfields == C_NULL || return false # TODO: currently we're only testing for ordering === :not_atomic # If all fields are always initialized, and bounds check is disabled, we can assume # we don't throw if !boundscheck && s.name.n_uninitialized == 0 @@ -890,6 +911,7 @@ function getfield_nothrow(@nospecialize(s00), @nospecialize(name), boundscheck:: isa(name, Const) || return false field = try_compute_fieldidx(s, name.val) field === nothing && return false + isfieldatomic(s, field) && return false # TODO: currently we're only testing for ordering === :not_atomic field <= datatype_min_ninitialized(s) && return true # `try_compute_fieldidx` already check for field index bound. !isvatuple(s) && isbitstype(fieldtype(s0, field)) && return true @@ -1210,12 +1232,12 @@ function setfield!_nothrow(@specialize(𝕃::AbstractLattice), s00, name, v) # Can't say anything about abstract types isabstracttype(s) && return false ismutabletype(s) || return false - s.name.atomicfields == C_NULL || return false # TODO: currently we're only testing for ordering === :not_atomic isa(name, Const) || return false field = try_compute_fieldidx(s, name.val) field === nothing && return false # `try_compute_fieldidx` already check for field index bound. isconst(s, field) && return false + isfieldatomic(s, field) && return false # TODO: currently we're only testing for ordering === :not_atomic v_expected = fieldtype(s0, field) return v ⊑ v_expected end @@ -2074,20 +2096,14 @@ function getfield_effects(argtypes::Vector{Any}, @nospecialize(rt)) if !(length(argtypes) ≥ 2 && getfield_notundefined(widenconst(obj), argtypes[2])) consistent = ALWAYS_FALSE end - if getfield_boundscheck(argtypes) !== true + nothrow = getfield_nothrow(argtypes, true) + if !nothrow && getfield_boundscheck(argtypes) !== true # If we cannot independently prove inboundsness, taint consistency. # The inbounds-ness assertion requires dynamic reachability, while # :consistent needs to be true for all input values. # N.B. We do not taint for `--check-bounds=no` here -that happens in # InferenceState. - if length(argtypes) ≥ 2 && getfield_nothrow(argtypes[1], argtypes[2], true) - nothrow = true - else - consistent = ALWAYS_FALSE - nothrow = false - end - else - nothrow = getfield_nothrow(argtypes) + consistent = ALWAYS_FALSE end if hasintersect(widenconst(obj), Module) inaccessiblememonly = getglobal_effects(argtypes, rt).inaccessiblememonly diff --git a/base/operators.jl b/base/operators.jl index 4d3faec6f61cb..2542acbde27d4 100644 --- a/base/operators.jl +++ b/base/operators.jl @@ -568,7 +568,7 @@ function afoldl(op, a, bs...) end return y end -typeof(afoldl).name.mt.max_args = 34 +setfield!(typeof(afoldl).name.mt, :max_args, 34, :monotonic) for op in (:+, :*, :&, :|, :xor, :min, :max, :kron) @eval begin diff --git a/base/reflection.jl b/base/reflection.jl index 0157616a4b8e3..2b559b73261c6 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -294,6 +294,29 @@ function isconst(@nospecialize(t::Type), s::Int) return unsafe_load(Ptr{UInt32}(constfields), 1 + s÷32) & (1 << (s%32)) != 0 end +""" + isfieldatomic(t::DataType, s::Union{Int,Symbol}) -> Bool + +Determine whether a field `s` is declared `@atomic` in a given type `t`. +""" +function isfieldatomic(@nospecialize(t::Type), s::Symbol) + t = unwrap_unionall(t) + isa(t, DataType) || return false + return isfieldatomic(t, fieldindex(t, s, false)) +end +function isfieldatomic(@nospecialize(t::Type), s::Int) + t = unwrap_unionall(t) + # TODO: what to do for `Union`? + isa(t, DataType) || return false # uncertain + ismutabletype(t) || return false # immutable structs are never atomic + 1 <= s <= length(t.name.names) || return false # OOB reads are not atomic (they always throw) + atomicfields = t.name.atomicfields + atomicfields === C_NULL && return false + s -= 1 + return unsafe_load(Ptr{UInt32}(atomicfields), 1 + s÷32) & (1 << (s%32)) != 0 +end + + """ @locals() diff --git a/doc/src/base/base.md b/doc/src/base/base.md index 704a0ff9fe2f1..72a8ec4db613c 100644 --- a/doc/src/base/base.md +++ b/doc/src/base/base.md @@ -188,6 +188,7 @@ Base.fieldcount Base.hasfield Core.nfields Base.isconst +Base.isfieldatomic ``` ### Memory layout diff --git a/src/Makefile b/src/Makefile index 380d2687e75a1..11d5afa963a8c 100644 --- a/src/Makefile +++ b/src/Makefile @@ -455,7 +455,7 @@ SA_EXCEPTIONS-jloptions.c := -Xanalyzer -analyzer-config -Xana SA_EXCEPTIONS-subtype.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core.uninitialized.Assign;core.UndefinedBinaryOperatorResult" SA_EXCEPTIONS-codegen.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core" # these need to be annotated (and possibly fixed) -SKIP_IMPLICIT_ATOMICS := module.c staticdata.c codegen.cpp +SKIP_IMPLICIT_ATOMICS := staticdata.c # these need to be annotated (and possibly fixed) SKIP_GC_CHECK := codegen.cpp rtutils.c diff --git a/src/ccall.cpp b/src/ccall.cpp index b2e66a1345f96..f1fe94a0d0ede 100644 --- a/src/ccall.cpp +++ b/src/ccall.cpp @@ -85,7 +85,7 @@ static bool runtime_sym_gvs(jl_codectx_t &ctx, const char *f_lib, const char *f_ else { std::string name = "ccalllib_"; name += llvm::sys::path::filename(f_lib); - name += std::to_string(globalUniqueGeneratedNames++); + name += std::to_string(jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1)); runtime_lib = true; auto &libgv = ctx.emission_context.libMapGV[f_lib]; if (libgv.first == NULL) { @@ -105,7 +105,7 @@ static bool runtime_sym_gvs(jl_codectx_t &ctx, const char *f_lib, const char *f_ std::string name = "ccall_"; name += f_name; name += "_"; - name += std::to_string(globalUniqueGeneratedNames++); + name += std::to_string(jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1)); auto T_pvoidfunc = JuliaType::get_pvoidfunc_ty(M->getContext()); llvmgv = new GlobalVariable(*M, T_pvoidfunc, false, GlobalVariable::ExternalLinkage, @@ -215,7 +215,7 @@ static Value *runtime_sym_lookup( std::string gvname = "libname_"; gvname += f_name; gvname += "_"; - gvname += std::to_string(globalUniqueGeneratedNames++); + gvname += std::to_string(jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1)); llvmgv = new GlobalVariable(*jl_Module, T_pvoidfunc, false, GlobalVariable::ExternalLinkage, Constant::getNullValue(T_pvoidfunc), gvname); @@ -244,7 +244,7 @@ static GlobalVariable *emit_plt_thunk( libptrgv = prepare_global_in(M, libptrgv); llvmgv = prepare_global_in(M, llvmgv); std::string fname; - raw_string_ostream(fname) << "jlplt_" << f_name << "_" << globalUniqueGeneratedNames++; + raw_string_ostream(fname) << "jlplt_" << f_name << "_" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); Function *plt = Function::Create(functype, GlobalVariable::ExternalLinkage, fname, M); @@ -858,7 +858,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar // Make sure to find a unique name std::string ir_name; while (true) { - raw_string_ostream(ir_name) << (ctx.f->getName().str()) << "u" << globalUniqueGeneratedNames++; + raw_string_ostream(ir_name) << (ctx.f->getName().str()) << "u" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); if (jl_Module->getFunction(ir_name) == NULL) break; } diff --git a/src/codegen.cpp b/src/codegen.cpp index cdc5e00a26281..024f30ad576e9 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1198,7 +1198,7 @@ static const auto &builtin_func_map() { static const auto jl_new_opaque_closure_jlcall_func = new JuliaFunction{XSTR(jl_new_opaque_closure_jlcall), get_func_sig, get_func_attrs}; -static std::atomic globalUniqueGeneratedNames{0}; +static _Atomic(int) globalUniqueGeneratedNames{1}; // --- code generation --- extern "C" { @@ -2315,7 +2315,7 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex) if (b && b->constp) { if (b->deprecated) cg_bdw(ctx, b); - return b->value; + return jl_atomic_load_relaxed(&b->value); } return NULL; } @@ -2337,7 +2337,7 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex) if (b && b->constp) { if (b->deprecated) cg_bdw(ctx, b); - return b->value; + return jl_atomic_load_relaxed(&b->value); } } } @@ -2579,14 +2579,17 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t * if (bp == NULL) return jl_cgval_t(); bp = julia_binding_pvalue(ctx, bp); - if (bnd && bnd->value != NULL) { - if (bnd->constp) { - return mark_julia_const(ctx, bnd->value); + if (bnd) { + jl_value_t *v = jl_atomic_load_acquire(&bnd->value); // acquire value for ty + if (v != NULL) { + if (bnd->constp) + return mark_julia_const(ctx, v); + LoadInst *v = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, bp, Align(sizeof(void*))); + v->setOrdering(order); + tbaa_decorate(ctx.tbaa().tbaa_binding, v); + jl_value_t *ty = jl_atomic_load_relaxed(&bnd->ty); + return mark_julia_type(ctx, v, true, ty); } - LoadInst *v = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, bp, Align(sizeof(void*))); - v->setOrdering(order); - tbaa_decorate(ctx.tbaa().tbaa_binding, v); - return mark_julia_type(ctx, v, true, bnd->ty); } // todo: use type info to avoid undef check return emit_checked_var(ctx, bp, name, false, ctx.tbaa().tbaa_binding); @@ -2595,15 +2598,17 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t * static void emit_globalset(jl_codectx_t &ctx, jl_binding_t *bnd, Value *bp, const jl_cgval_t &rval_info, AtomicOrdering Order) { Value *rval = boxed(ctx, rval_info); - if (bnd && !bnd->constp && bnd->ty && jl_subtype(rval_info.typ, bnd->ty)) { - StoreInst *v = ctx.builder.CreateAlignedStore(rval, julia_binding_pvalue(ctx, bp), Align(sizeof(void*))); - v->setOrdering(Order); - tbaa_decorate(ctx.tbaa().tbaa_binding, v); - emit_write_barrier_binding(ctx, bp, rval); - } - else { - ctx.builder.CreateCall(prepare_call(jlcheckassign_func), { bp, mark_callee_rooted(ctx, rval) }); + if (bnd && !bnd->constp) { + jl_value_t *ty = jl_atomic_load_relaxed(&bnd->ty); + if (ty && jl_subtype(rval_info.typ, ty)) { // TODO: use typeassert here instead + StoreInst *v = ctx.builder.CreateAlignedStore(rval, julia_binding_pvalue(ctx, bp), Align(sizeof(void*))); + v->setOrdering(Order); + tbaa_decorate(ctx.tbaa().tbaa_binding, v); + emit_write_barrier_binding(ctx, bp, rval); + return; + } } + ctx.builder.CreateCall(prepare_call(jlcheckassign_func), { bp, mark_callee_rooted(ctx, rval) }); } static Value *emit_box_compare(jl_codectx_t &ctx, const jl_cgval_t &arg1, const jl_cgval_t &arg2, @@ -4028,6 +4033,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, const if (ctx.use_cache) { // optimization: emit the correct name immediately, if we know it // TODO: use `emitted` map here too to try to consolidate names? + // WARNING: isspecsig is protected by the codegen-lock. If that lock is removed, then the isspecsig load needs to be properly atomically sequenced with this. auto invoke = jl_atomic_load_relaxed(&codeinst->invoke); auto fptr = jl_atomic_load_relaxed(&codeinst->specptr.fptr); if (fptr) { @@ -4043,7 +4049,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, const need_to_emit = false; } if (need_to_emit) { - raw_string_ostream(name) << (specsig ? "j_" : "j1_") << name_from_method_instance(mi) << "_" << globalUniqueGeneratedNames++; + raw_string_ostream(name) << (specsig ? "j_" : "j1_") << name_from_method_instance(mi) << "_" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); protoname = StringRef(name); } jl_returninfo_t::CallingConv cc = jl_returninfo_t::CallingConv::Boxed; @@ -4324,7 +4330,7 @@ static jl_cgval_t emit_isdefined(jl_codectx_t &ctx, jl_value_t *sym) } jl_binding_t *bnd = jl_get_binding(modu, name); if (bnd) { - if (bnd->value != NULL) + if (jl_atomic_load_relaxed(&bnd->value) != NULL) return mark_julia_const(ctx, jl_true); Value *bp = julia_binding_gv(ctx, bnd); bp = julia_binding_pvalue(ctx, bp); @@ -5052,10 +5058,8 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_ } JL_CATCH { jl_value_t *e = jl_current_exception(); - // errors. boo. root it somehow :( - bnd = jl_get_binding_wr(ctx.module, (jl_sym_t*)jl_gensym(), 1); - bnd->value = e; - bnd->constp = 1; + // errors. boo. :( + e = jl_as_global_root(e); raise_exception(ctx, literal_pointer_val(ctx, e)); return ghostValue(ctx, jl_nothing_type); } @@ -5359,7 +5363,7 @@ static Function *emit_tojlinvoke(jl_code_instance_t *codeinst, Module *M, jl_cod ++EmittedToJLInvokes; jl_codectx_t ctx(M->getContext(), params); std::string name; - raw_string_ostream(name) << "tojlinvoke" << globalUniqueGeneratedNames++; + raw_string_ostream(name) << "tojlinvoke" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); Function *f = Function::Create(ctx.types().T_jlfunc, GlobalVariable::InternalLinkage, name, M); @@ -5530,18 +5534,21 @@ static Function* gen_cfun_wrapper( if (lam && params.cache) { // TODO: this isn't ideal to be unconditionally calling type inference (and compile) from here codeinst = jl_compile_method_internal(lam, world); - assert(codeinst->invoke); - if (codeinst->invoke == jl_fptr_args_addr) { - callptr = codeinst->specptr.fptr; + auto invoke = jl_atomic_load_relaxed(&codeinst->invoke); + auto fptr = jl_atomic_load_relaxed(&codeinst->specptr.fptr); + assert(invoke); + // WARNING: this invoke load is protected by the codegen-lock. If that lock is removed, then the isspecsig load needs to be properly atomically sequenced with this. + if (invoke == jl_fptr_args_addr) { + callptr = fptr; calltype = 1; } - else if (codeinst->invoke == jl_fptr_const_return_addr) { + else if (invoke == jl_fptr_const_return_addr) { // don't need the fptr callptr = (void*)codeinst->rettype_const; calltype = 2; } else if (codeinst->isspecsig) { - callptr = codeinst->specptr.fptr; + callptr = fptr; calltype = 3; } astrt = codeinst->rettype; @@ -5555,7 +5562,7 @@ static Function* gen_cfun_wrapper( } std::string funcName; - raw_string_ostream(funcName) << "jlcapi_" << name << "_" << globalUniqueGeneratedNames++; + raw_string_ostream(funcName) << "jlcapi_" << name << "_" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); Module *M = into; // Safe because ctx lock is held by params AttributeList attributes = sig.attributes; @@ -6748,7 +6755,7 @@ static jl_llvm_functions_t if (unadorned_name[0] == '@') unadorned_name++; #endif - funcName << unadorned_name << "_" << globalUniqueGeneratedNames++; + funcName << unadorned_name << "_" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); declarations.specFunctionObject = funcName.str(); // allocate Function declarations and wrapper objects @@ -6790,7 +6797,7 @@ static jl_llvm_functions_t }(); std::string wrapName; - raw_string_ostream(wrapName) << "jfptr_" << unadorned_name << "_" << globalUniqueGeneratedNames++; + raw_string_ostream(wrapName) << "jfptr_" << unadorned_name << "_" << jl_atomic_fetch_add(&globalUniqueGeneratedNames, 1); declarations.functionObject = wrapName; (void)gen_invoke_wrapper(lam, jlrettype, returninfo, retarg, declarations.functionObject, M, ctx.emission_context); // TODO: add attributes: maybe_mark_argument_dereferenceable(Arg, argType) @@ -8225,7 +8232,7 @@ jl_llvm_functions_t jl_emit_codeinst( if (// and there is something to delete (test this before calling jl_ir_inlining_cost) inferred != jl_nothing && // don't delete inlineable code, unless it is constant - (codeinst->invoke == jl_fptr_const_return_addr || + (jl_atomic_load_relaxed(&codeinst->invoke) == jl_fptr_const_return_addr || (jl_ir_inlining_cost((jl_array_t*)inferred) == UINT16_MAX)) && // don't delete code when generating a precompile file !(params.imaging || jl_options.incremental)) { @@ -8269,6 +8276,7 @@ void jl_compile_workqueue( // We need to emit a trampoline that loads the target address in an extern_module from a GV // Right now we will unecessarily emit a function we have already compiled in a native module // again in a calling module. + // WARNING: isspecsig is protected by the codegen-lock. If that lock is removed, then the isspecsig load needs to be properly atomically sequenced with this. if (params.cache && invoke != NULL) { auto fptr = jl_atomic_load_relaxed(&codeinst->specptr.fptr); if (invoke == jl_fptr_args_addr) { diff --git a/src/datatype.c b/src/datatype.c index 3fc37f199bfd8..920475af95529 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -51,7 +51,7 @@ JL_DLLEXPORT jl_methtable_t *jl_new_method_table(jl_sym_t *name, jl_module_t *mo jl_atomic_store_relaxed(&mt->defs, jl_nothing); jl_atomic_store_relaxed(&mt->leafcache, (jl_array_t*)jl_an_empty_vec_any); jl_atomic_store_relaxed(&mt->cache, jl_nothing); - mt->max_args = 0; + jl_atomic_store_relaxed(&mt->max_args, 0); mt->backedges = NULL; JL_MUTEX_INIT(&mt->writelock); mt->offs = 0; diff --git a/src/gf.c b/src/gf.c index 77cb7d236168d..df50b42216006 100644 --- a/src/gf.c +++ b/src/gf.c @@ -874,7 +874,7 @@ JL_DLLEXPORT int jl_isa_compileable_sig( if ((jl_value_t*)mt != jl_nothing) { // try to refine estimate of min and max if (kwmt && kwmt != jl_type_type_mt && kwmt != jl_nonfunction_mt && kwmt != jl_kwcall_mt) - nspec_min = kwmt->max_args + 2 + 2 * (mt == jl_kwcall_mt); + nspec_min = jl_atomic_load_relaxed(&kwmt->max_args) + 2 + 2 * (mt == jl_kwcall_mt); else nspec_max = nspec_min; } @@ -1081,7 +1081,7 @@ static jl_method_instance_t *cache_method( int cache_with_orig = 1; jl_tupletype_t *compilationsig = tt; jl_methtable_t *kwmt = mt == jl_kwcall_mt ? jl_kwmethod_table_for(definition->sig) : mt; - intptr_t nspec = (kwmt == NULL || kwmt == jl_type_type_mt || kwmt == jl_nonfunction_mt || kwmt == jl_kwcall_mt ? definition->nargs + 1 : kwmt->max_args + 2 + 2 * (mt == jl_kwcall_mt)); + intptr_t nspec = (kwmt == NULL || kwmt == jl_type_type_mt || kwmt == jl_nonfunction_mt || kwmt == jl_kwcall_mt ? definition->nargs + 1 : jl_atomic_load_relaxed(&kwmt->max_args) + 2 + 2 * (mt == jl_kwcall_mt)); jl_compilation_sig(tt, sparams, definition, nspec, &newparams); if (newparams) { compilationsig = jl_apply_tuple_type(newparams); @@ -1368,8 +1368,9 @@ static void update_max_args(jl_methtable_t *mt, jl_value_t *type) size_t na = jl_nparams(type); if (jl_va_tuple_kind((jl_datatype_t*)type) == JL_VARARG_UNBOUND) na--; - if (na > mt->max_args) - mt->max_args = na; + // update occurs inside mt->writelock + if (na > jl_atomic_load_relaxed(&mt->max_args)) + jl_atomic_store_relaxed(&mt->max_args, na); } jl_array_t *_jl_debug_method_invalidation JL_GLOBALLY_ROOTED = NULL; @@ -2160,8 +2161,8 @@ jl_code_instance_t *jl_compile_method_internal(jl_method_instance_t *mi, size_t jl_code_instance_t *ucache = jl_get_method_inferred(unspec, (jl_value_t*)jl_any_type, 1, ~(size_t)0); // ask codegen to make the fptr for unspec if (jl_atomic_load_acquire(&ucache->invoke) == NULL) { - if (def->source == jl_nothing && (ucache->def->uninferred == jl_nothing || - ucache->def->uninferred == NULL)) { + if (def->source == jl_nothing && (jl_atomic_load_relaxed(&ucache->def->uninferred) == jl_nothing || + jl_atomic_load_relaxed(&ucache->def->uninferred) == NULL)) { jl_printf(JL_STDERR, "source not available for "); jl_static_show(JL_STDERR, (jl_value_t*)mi); jl_printf(JL_STDERR, "\n"); @@ -2245,7 +2246,7 @@ JL_DLLEXPORT jl_value_t *jl_normalize_to_compilable_sig(jl_methtable_t *mt, jl_t jl_svec_t *newparams = NULL; JL_GC_PUSH2(&tt, &newparams); jl_methtable_t *kwmt = mt == jl_kwcall_mt ? jl_kwmethod_table_for(m->sig) : mt; - intptr_t nspec = (kwmt == NULL || kwmt == jl_type_type_mt || kwmt == jl_nonfunction_mt || kwmt == jl_kwcall_mt ? m->nargs + 1 : kwmt->max_args + 2 + 2 * (mt == jl_kwcall_mt)); + intptr_t nspec = (kwmt == NULL || kwmt == jl_type_type_mt || kwmt == jl_nonfunction_mt || kwmt == jl_kwcall_mt ? m->nargs + 1 : jl_atomic_load_relaxed(&kwmt->max_args) + 2 + 2 * (mt == jl_kwcall_mt)); jl_compilation_sig(ti, env, m, nspec, &newparams); tt = (newparams ? jl_apply_tuple_type(newparams) : ti); int is_compileable = ((jl_datatype_t*)ti)->isdispatchtuple || @@ -2408,7 +2409,7 @@ static void jl_compile_now(jl_method_instance_t *mi) JL_DLLEXPORT void jl_compile_method_instance(jl_method_instance_t *mi, jl_tupletype_t *types, size_t world) { size_t tworld = jl_typeinf_world; - mi->precompiled = 1; + jl_atomic_store_relaxed(&mi->precompiled, 1); if (jl_generating_output()) { jl_compile_now(mi); // In addition to full compilation of the compilation-signature, if `types` is more specific (e.g. due to nospecialize), @@ -2421,14 +2422,14 @@ JL_DLLEXPORT void jl_compile_method_instance(jl_method_instance_t *mi, jl_tuplet jl_value_t *types2 = NULL; JL_GC_PUSH2(&tpenv2, &types2); types2 = jl_type_intersection_env((jl_value_t*)types, (jl_value_t*)mi->def.method->sig, &tpenv2); - jl_method_instance_t *li2 = jl_specializations_get_linfo(mi->def.method, (jl_value_t*)types2, tpenv2); + jl_method_instance_t *mi2 = jl_specializations_get_linfo(mi->def.method, (jl_value_t*)types2, tpenv2); JL_GC_POP(); - li2->precompiled = 1; - if (jl_rettype_inferred(li2, world, world) == jl_nothing) - (void)jl_type_infer(li2, world, 1); + jl_atomic_store_relaxed(&mi2->precompiled, 1); + if (jl_rettype_inferred(mi2, world, world) == jl_nothing) + (void)jl_type_infer(mi2, world, 1); if (jl_typeinf_func && mi->def.method->primary_world <= tworld) { - if (jl_rettype_inferred(li2, tworld, tworld) == jl_nothing) - (void)jl_type_infer(li2, tworld, 1); + if (jl_rettype_inferred(mi2, tworld, tworld) == jl_nothing) + (void)jl_type_infer(mi2, tworld, 1); } } } diff --git a/src/init.c b/src/init.c index feee29d02ea70..19e4dafef44d3 100644 --- a/src/init.c +++ b/src/init.c @@ -902,7 +902,7 @@ static void post_boot_hooks(void) jl_pair_type = core("Pair"); jl_kwcall_func = core("kwcall"); jl_kwcall_mt = ((jl_datatype_t*)jl_typeof(jl_kwcall_func))->name->mt; - jl_kwcall_mt->max_args = 0; + jl_atomic_store_relaxed(&jl_kwcall_mt->max_args, 0); jl_weakref_type = (jl_datatype_t*)core("WeakRef"); jl_vecelement_typename = ((jl_datatype_t*)jl_unwrap_unionall(core("VecElement")))->name; diff --git a/src/interpreter.c b/src/interpreter.c index 37deb24188873..cf9ddd5b50630 100644 --- a/src/interpreter.c +++ b/src/interpreter.c @@ -632,7 +632,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip, jl_code_info_t *jl_code_for_interpreter(jl_method_instance_t *mi) { - jl_code_info_t *src = (jl_code_info_t*)mi->uninferred; + jl_code_info_t *src = (jl_code_info_t*)jl_atomic_load_relaxed(&mi->uninferred); if (jl_is_method(mi->def.value)) { if (!src || (jl_value_t*)src == jl_nothing) { if (mi->def.method->source) { @@ -646,7 +646,7 @@ jl_code_info_t *jl_code_for_interpreter(jl_method_instance_t *mi) if (src && (jl_value_t*)src != jl_nothing) { JL_GC_PUSH1(&src); src = jl_uncompress_ir(mi->def.method, NULL, (jl_array_t*)src); - mi->uninferred = (jl_value_t*)src; + jl_atomic_store_release(&mi->uninferred, (jl_value_t*)src); jl_gc_wb(mi, src); JL_GC_POP(); } diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp index b6a30d3380b27..f5a0623cd8df6 100644 --- a/src/jitlayers.cpp +++ b/src/jitlayers.cpp @@ -481,7 +481,7 @@ void jl_generate_fptr_for_unspecialized_impl(jl_code_instance_t *unspec) src = jl_uncompress_ir(def, NULL, (jl_array_t*)src); } else { - src = (jl_code_info_t*)unspec->def->uninferred; + src = (jl_code_info_t*)jl_atomic_load_relaxed(&unspec->def->uninferred); } assert(src && jl_is_code_info(src)); ++UnspecFPtrCount; diff --git a/src/jltypes.c b/src/jltypes.c index 0daeae58b6b64..9f93d60fb1acd 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -2050,7 +2050,9 @@ void jl_init_types(void) JL_GC_DISABLED jl_any_type /*jl_int32_type*/, jl_any_type /*jl_uint8_type*/); const static uint32_t datatype_constfields[1] = { 0x00000057 }; // (1<<0)|(1<<1)|(1<<2)|(1<<4)|(1<<6) + const static uint32_t datatype_atomicfields[1] = { 0x00000028 }; // (1<<3)|(1<<5) jl_datatype_type->name->constfields = datatype_constfields; + jl_datatype_type->name->atomicfields = datatype_atomicfields; jl_precompute_memoized_dt(jl_datatype_type, 1); jl_typename_type->name = jl_new_typename_in(jl_symbol("TypeName"), core, 0, 1); @@ -2074,7 +2076,9 @@ void jl_init_types(void) JL_GC_DISABLED jl_any_type /*jl_uint8_type*/, jl_any_type /*jl_uint8_type*/); const static uint32_t typename_constfields[1] = { 0x00003a3f }; // (1<<0)|(1<<1)|(1<<2)|(1<<3)|(1<<4)|(1<<5)|(1<<9)|(1<<11)|(1<<12)|(1<<13) + const static uint32_t typename_atomicfields[1] = { 0x00000180 }; // (1<<7)|(1<<8) jl_typename_type->name->constfields = typename_constfields; + jl_typename_type->name->atomicfields = typename_atomicfields; jl_precompute_memoized_dt(jl_typename_type, 1); jl_methtable_type->name = jl_new_typename_in(jl_symbol("MethodTable"), core, 0, 1); @@ -2093,7 +2097,9 @@ void jl_init_types(void) JL_GC_DISABLED jl_any_type/*voidpointer*/, jl_any_type/*int32*/, jl_any_type/*uint8*/, jl_any_type/*uint8*/); const static uint32_t methtable_constfields[1] = { 0x00000020 }; // (1<<5); + const static uint32_t methtable_atomicfields[1] = { 0x0000001e }; // (1<<1)|(1<<2)|(1<<3)|(1<<4); jl_methtable_type->name->constfields = methtable_constfields; + jl_methtable_type->name->atomicfields = methtable_atomicfields; jl_precompute_memoized_dt(jl_methtable_type, 1); jl_symbol_type->name = jl_new_typename_in(jl_symbol("Symbol"), core, 0, 1); @@ -2133,21 +2139,27 @@ void jl_init_types(void) JL_GC_DISABLED jl_perm_symsvec(2, "a", "b"), jl_svec(2, jl_any_type, jl_any_type), jl_emptysvec, 0, 0, 2); + // It seems like we probably usually end up needing the box for kinds (used in an Any context), so force it to exist + jl_uniontype_type->name->mayinlinealloc = 0; jl_tvar_type = jl_new_datatype(jl_symbol("TypeVar"), core, jl_any_type, jl_emptysvec, jl_perm_symsvec(3, "name", "lb", "ub"), jl_svec(3, jl_symbol_type, jl_any_type, jl_any_type), jl_emptysvec, 0, 1, 3); + const static uint32_t tvar_constfields[1] = { 0x00000007 }; // all fields are constant, even though TypeVar itself has identity + jl_tvar_type->name->constfields = tvar_constfields; jl_unionall_type = jl_new_datatype(jl_symbol("UnionAll"), core, type_type, jl_emptysvec, jl_perm_symsvec(2, "var", "body"), jl_svec(2, jl_tvar_type, jl_any_type), jl_emptysvec, 0, 0, 2); + jl_unionall_type->name->mayinlinealloc = 0; jl_vararg_type = jl_new_datatype(jl_symbol("TypeofVararg"), core, jl_any_type, jl_emptysvec, jl_perm_symsvec(2, "T", "N"), jl_svec(2, jl_any_type, jl_any_type), jl_emptysvec, 0, 0, 0); + jl_vararg_type->name->mayinlinealloc = 0; jl_svec_t *anytuple_params = jl_svec(1, jl_wrap_vararg((jl_value_t*)jl_any_type, (jl_value_t*)NULL)); jl_anytuple_type = jl_new_datatype(jl_symbol("Tuple"), core, jl_any_type, anytuple_params, @@ -2243,6 +2255,8 @@ void jl_init_types(void) JL_GC_DISABLED jl_any_type), jl_emptysvec, 0, 1, 6); + const static uint32_t typemap_level_atomicfields[1] = { 0x0000003f }; // (1<<0)|(1<<1)|(1<<2)|(1<<3)|(1<<4)|(1<<5) + jl_typemap_level_type->name->atomicfields = typemap_level_atomicfields; jl_typemap_entry_type = jl_new_datatype(jl_symbol("TypeMapEntry"), core, jl_any_type, jl_emptysvec, @@ -2270,8 +2284,10 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type), jl_emptysvec, 0, 1, 4); - const static uint32_t typemap_entry_constfields[1] = { 0x000003fe }; // (1<<1)|(1<<2)|(1<<3)|(1<<4)|(1<<5)|(1<<6)|(1<<7)|(1<<8)|(1<<9); + const static uint32_t typemap_entry_constfields[1] = { 0x000003fe }; // (1<<1)|(1<<2)|(1<<3)|(1<<4)|(1<<5)|(1<<6)|(1<<7)|(1<<8)|(1<<9) + const static uint32_t typemap_entry_atomicfields[1] = { 0x00000001 }; // (1<<0) jl_typemap_entry_type->name->constfields = typemap_entry_constfields; + jl_typemap_entry_type->name->atomicfields = typemap_entry_atomicfields; jl_function_type = jl_new_abstracttype((jl_value_t*)jl_symbol("Function"), core, jl_any_type, jl_emptysvec); jl_builtin_type = jl_new_abstracttype((jl_value_t*)jl_symbol("Builtin"), core, jl_function_type, jl_emptysvec); @@ -2529,8 +2545,12 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type), jl_emptysvec, 0, 1, 3); + // These fields should be constant, but Serialization wants to mutate them in initialization //const static uint32_t method_instance_constfields[1] = { 0x00000007 }; // (1<<0)|(1<<1)|(1<<2); + const static uint32_t method_instance_atomicfields[1] = { 0x00000148 }; // (1<<3)|(1<<6)|(1<<8); + //Fields 4 and 5 must be protected by method->write_lock, and thus all operations on jl_method_instance_t are threadsafe. TODO: except inInference //jl_method_instance_type->name->constfields = method_instance_constfields; + jl_method_instance_type->name->atomicfields = method_instance_atomicfields; jl_code_instance_type = jl_new_datatype(jl_symbol("CodeInstance"), core, @@ -2570,6 +2590,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_svecset(jl_code_instance_type->types, 1, jl_code_instance_type); const static uint32_t code_instance_constfields[1] = { 0b000001010110001 }; // Set fields 1, 5-6, 8, 10 as const const static uint32_t code_instance_atomicfields[1] = { 0b110100101000010 }; // Set fields 2, 7, 9, 12, 14-15 as atomic + //Fields 3-4 are only operated on by construction and deserialization, so are const at runtime //Fields 11 and 15 must be protected by locks, and thus all operations on jl_code_instance_t are threadsafe jl_code_instance_type->name->constfields = code_instance_constfields; jl_code_instance_type->name->atomicfields = code_instance_atomicfields; @@ -2742,9 +2763,6 @@ void jl_init_types(void) JL_GC_DISABLED // override the preferred layout for a couple types jl_lineinfonode_type->name->mayinlinealloc = 0; // FIXME: assumed to be a pointer by codegen - // It seems like we probably usually end up needing the box for kinds (used in an Any context)--but is that true? - jl_uniontype_type->name->mayinlinealloc = 0; - jl_unionall_type->name->mayinlinealloc = 0; } #ifdef __cplusplus diff --git a/src/julia.h b/src/julia.h index c40d3ce3b88e5..f7cf69710fa62 100644 --- a/src/julia.h +++ b/src/julia.h @@ -364,12 +364,12 @@ struct _jl_method_instance_t { } def; // pointer back to the context for this code jl_value_t *specTypes; // argument types this was specialized for jl_svec_t *sparam_vals; // static parameter values, indexed by def.method->sparam_syms - jl_value_t *uninferred; // cached uncompressed code, for generated functions, top-level thunks, or the interpreter + _Atomic(jl_value_t*) uninferred; // cached uncompressed code, for generated functions, top-level thunks, or the interpreter jl_array_t *backedges; // list of method-instances which call this method-instance; `invoke` records (invokesig, caller) pairs jl_array_t *callbacks; // list of callback functions to inform external caches about invalidations _Atomic(struct _jl_code_instance_t*) cache; uint8_t inInference; // flags to tell if inference is running on this object - uint8_t precompiled; // true if this instance was generated by an explicit `precompile(...)` call + _Atomic(uint8_t) precompiled; // true if this instance was generated by an explicit `precompile(...)` call }; // OpaqueClosure @@ -400,39 +400,29 @@ typedef struct _jl_code_instance_t { //TODO: uint8_t absolute_max; // whether true max world is unknown // purity results -#ifdef JL_USE_ANON_UNIONS_FOR_PURITY_FLAGS // see also encode_effects() and decode_effects() in `base/compiler/effects.jl`, - union { - uint32_t ipo_purity_bits; - struct { - uint8_t ipo_consistent : 2; - uint8_t ipo_effect_free : 2; - uint8_t ipo_nothrow : 2; - uint8_t ipo_terminates : 2; - uint8_t ipo_nonoverlayed : 1; - uint8_t ipo_notaskstate : 2; - uint8_t ipo_inaccessiblememonly : 2; - } ipo_purity_flags; - }; - union { - uint32_t purity_bits; - struct { - uint8_t consistent : 2; - uint8_t effect_free : 2; - uint8_t nothrow : 2; - uint8_t terminates : 2; - uint8_t nonoverlayed : 1; - uint8_t notaskstate : 2; - uint8_t inaccessiblememonly : 2; - } purity_flags; - }; -#else uint32_t ipo_purity_bits; + // ipo_purity_flags: + // uint8_t ipo_consistent : 2; + // uint8_t ipo_effect_free : 2; + // uint8_t ipo_nothrow : 2; + // uint8_t ipo_terminates : 2; + // uint8_t ipo_nonoverlayed : 1; + // uint8_t ipo_notaskstate : 2; + // uint8_t ipo_inaccessiblememonly : 2; _Atomic(uint32_t) purity_bits; -#endif + // purity_flags: + // uint8_t consistent : 2; + // uint8_t effect_free : 2; + // uint8_t nothrow : 2; + // uint8_t terminates : 2; + // uint8_t nonoverlayed : 1; + // uint8_t notaskstate : 2; + // uint8_t inaccessiblememonly : 2; jl_value_t *argescapes; // escape information of call arguments // compilation state cache + // WARNING: isspecsig is protected by the codegen-lock. If that lock is removed, then the isspecsig load needs to be properly atomically sequenced with the invoke pointers. uint8_t isspecsig; // if specptr is a specialized function signature for specTypes->rettype _Atomic(uint8_t) precompile; // if set, this will be added to the output system image uint8_t relocatability; // nonzero if all roots are built into sysimg or tagged by module key @@ -546,7 +536,7 @@ typedef struct _jl_datatype_t { jl_svec_t *types; jl_value_t *instance; // for singletons const jl_datatype_layout_t *layout; - // memoized properties + // memoized properties (set on construction) uint32_t hash; uint8_t hasfreetypevars:1; // majority part of isconcrete computation uint8_t isconcretetype:1; // whether this type can have instances @@ -660,7 +650,7 @@ typedef struct _jl_methtable_t { _Atomic(jl_typemap_t*) defs; _Atomic(jl_array_t*) leafcache; _Atomic(jl_typemap_t*) cache; - intptr_t max_args; // max # of non-vararg arguments in a signature + _Atomic(intptr_t) max_args; // max # of non-vararg arguments in a signature jl_module_t *module; // used for incremental serialization to locate original binding jl_array_t *backedges; // (sig, caller::MethodInstance) pairs jl_mutex_t writelock; @@ -1562,7 +1552,7 @@ JL_DLLEXPORT jl_value_t *jl_get_nth_field(jl_value_t *v, size_t i); // Like jl_get_nth_field above, but asserts if it needs to allocate JL_DLLEXPORT jl_value_t *jl_get_nth_field_noalloc(jl_value_t *v JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT; JL_DLLEXPORT jl_value_t *jl_get_nth_field_checked(jl_value_t *v, size_t i); -JL_DLLEXPORT void jl_set_nth_field(jl_value_t *v, size_t i, jl_value_t *rhs) JL_NOTSAFEPOINT; +JL_DLLEXPORT void jl_set_nth_field(jl_value_t *v, size_t i, jl_value_t *rhs); JL_DLLEXPORT int jl_field_isdefined(jl_value_t *v, size_t i) JL_NOTSAFEPOINT; JL_DLLEXPORT int jl_field_isdefined_checked(jl_value_t *v, size_t i); JL_DLLEXPORT jl_value_t *jl_get_field(jl_value_t *o, const char *fld); diff --git a/src/method.c b/src/method.c index 098c5df5aed98..3da88ac8211ac 100644 --- a/src/method.c +++ b/src/method.c @@ -442,12 +442,12 @@ JL_DLLEXPORT jl_method_instance_t *jl_new_method_instance_uninit(void) li->def.value = NULL; li->specTypes = NULL; li->sparam_vals = jl_emptysvec; - li->uninferred = NULL; + jl_atomic_store_relaxed(&li->uninferred, NULL); li->backedges = NULL; li->callbacks = NULL; jl_atomic_store_relaxed(&li->cache, NULL); li->inInference = 0; - li->precompiled = 0; + jl_atomic_store_relaxed(&li->precompiled, 0); return li; } @@ -555,8 +555,10 @@ JL_DLLEXPORT jl_code_info_t *jl_expand_and_resolve(jl_value_t *ex, jl_module_t * // effectively described by the tuple (specTypes, env, Method) inside linfo JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo) { - if (linfo->uninferred) { - return (jl_code_info_t*)jl_copy_ast((jl_value_t*)linfo->uninferred); + jl_value_t *uninferred = jl_atomic_load_relaxed(&linfo->uninferred); + if (uninferred) { + assert(jl_is_code_info(uninferred)); // make sure this did not get `nothing` put here + return (jl_code_info_t*)jl_copy_ast((jl_value_t*)uninferred); } JL_TIMING(STAGED_FUNCTION); @@ -599,13 +601,22 @@ JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo) } } + jl_add_function_name_to_lineinfo(func, (jl_value_t*)def->name); + // If this generated function has an opaque closure, cache it for // correctness of method identity for (int i = 0; i < jl_array_len(func->code); ++i) { jl_value_t *stmt = jl_array_ptr_ref(func->code, i); if (jl_is_expr(stmt) && ((jl_expr_t*)stmt)->head == jl_new_opaque_closure_sym) { - linfo->uninferred = jl_copy_ast((jl_value_t*)func); - jl_gc_wb(linfo, linfo->uninferred); + jl_value_t *uninferred = jl_copy_ast((jl_value_t*)func); + jl_value_t *old = NULL; + if (jl_atomic_cmpswap(&linfo->uninferred, &old, uninferred)) { + jl_gc_wb(linfo, uninferred); + } + else { + assert(jl_is_code_info(old)); + func = (jl_code_info_t*)old; + } break; } } @@ -613,7 +624,6 @@ JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo) ct->ptls->in_pure_callback = last_in; jl_lineno = last_lineno; ct->world_age = last_age; - jl_add_function_name_to_lineinfo(func, (jl_value_t*)def->name); } JL_CATCH { ct->ptls->in_pure_callback = last_in; diff --git a/src/module.c b/src/module.c index d507dc69ff7b2..ec62e6d83f2aa 100644 --- a/src/module.c +++ b/src/module.c @@ -28,7 +28,7 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui m->build_id.lo++; // build id 0 is invalid m->build_id.hi = ~(uint64_t)0; m->primary_world = 0; - m->counter = 1; + jl_atomic_store_relaxed(&m->counter, 1); m->nospecialize = 0; m->optlevel = -1; m->compile = -1; @@ -162,10 +162,10 @@ static jl_binding_t *new_binding(jl_sym_t *name) assert(jl_is_symbol(name)); jl_binding_t *b = (jl_binding_t*)jl_gc_alloc_buf(ct->ptls, sizeof(jl_binding_t)); b->name = name; - b->value = NULL; + jl_atomic_store_relaxed(&b->value, NULL); b->owner = NULL; - b->ty = NULL; - b->globalref = NULL; + jl_atomic_store_relaxed(&b->ty, NULL); + jl_atomic_store_relaxed(&b->globalref, NULL); b->constp = 0; b->exportp = 0; b->imported = 0; @@ -246,11 +246,11 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ else { JL_UNLOCK(&m->lock); jl_binding_t *b2 = jl_get_binding(b->owner, b->name); - if (b2 == NULL || b2->value == NULL) + if (b2 == NULL || jl_atomic_load_relaxed(&b2->value) == NULL) jl_errorf("invalid method definition: imported function %s.%s does not exist", jl_symbol_name(b->owner->name), jl_symbol_name(b->name)); // TODO: we might want to require explicitly importing types to add constructors - if (!b->imported && !jl_is_type(b2->value)) { + if (!b->imported && (!b2->constp || !jl_is_type(jl_atomic_load_relaxed(&b2->value)))) { jl_errorf("error in method definition: function %s.%s must be explicitly imported to be extended", jl_symbol_name(b->owner->name), jl_symbol_name(b->name)); } @@ -310,7 +310,7 @@ static jl_binding_t *using_resolve_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl continue; if (owner != NULL && tempb->owner != b->owner && !tempb->deprecated && !b->deprecated && - !(tempb->constp && tempb->value && b->constp && b->value == tempb->value)) { + !(tempb->constp && b->constp && jl_atomic_load_relaxed(&tempb->value) && b->constp && jl_atomic_load_relaxed(&b->value) == jl_atomic_load_relaxed(&tempb->value))) { if (warn) { // mark this binding resolved (by creating it or setting the owner), to avoid repeating the warning (void)jl_get_binding_wr(m, var, 1); @@ -460,9 +460,12 @@ JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var) static int eq_bindings(jl_binding_t *a, jl_binding_t *b) { - if (a==b) return 1; - if (a->name == b->name && a->owner == b->owner) return 1; - if (a->constp && a->value && b->constp && b->value == a->value) return 1; + if (a == b) + return 1; + if (a->name == b->name && a->owner == b->owner) + return 1; + if (a->constp && b->constp && jl_atomic_load_relaxed(&a->value) && jl_atomic_load_relaxed(&b->value) == jl_atomic_load_relaxed(&a->value)) + return 1; return 0; } @@ -487,7 +490,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_s } else { if (b->deprecated) { - if (b->value == jl_nothing) { + if (jl_atomic_load_relaxed(&b->value) == jl_nothing) { return; } else if (to != jl_main_module && to != jl_base_module && @@ -524,7 +527,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_s else if (bto->owner != to && bto->owner != NULL) { // already imported from somewhere else jl_binding_t *bval = jl_get_binding(to, asname); - if (bval->constp && bval->value && b->constp && b->value == bval->value) { + if (bval->constp && b->constp && jl_atomic_load_relaxed(&bval->value) && jl_atomic_load_relaxed(&b->value) == jl_atomic_load_relaxed(&bval->value)) { // equivalent binding bto->imported = (explici!=0); JL_UNLOCK(&to->lock); @@ -538,10 +541,10 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_s } return; } - else if (bto->constp || bto->value) { + else if (bto->constp || jl_atomic_load_relaxed(&bto->value)) { // conflict with name owned by destination module assert(bto->owner == to); - if (bto->constp && bto->value && b->constp && b->value == bto->value) { + if (bto->constp && b->constp && jl_atomic_load_relaxed(&bto->value) && jl_atomic_load_relaxed(&b->value) == jl_atomic_load_relaxed(&bto->value)) { // equivalent binding JL_UNLOCK(&to->lock); } @@ -654,7 +657,7 @@ JL_DLLEXPORT void jl_module_export(jl_module_t *from, jl_sym_t *s) JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var) { jl_binding_t *b = jl_get_binding(m, var); - return b && (b->value != NULL); + return b && (jl_atomic_load_relaxed(&b->value) != NULL); } JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) @@ -662,7 +665,7 @@ JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) JL_LOCK(&m->lock); jl_binding_t *b = (jl_binding_t*)ptrhash_get(&m->bindings, var); JL_UNLOCK(&m->lock); - return b != HT_NOTFOUND && (b->exportp || b->owner==m); + return b != HT_NOTFOUND && (b->exportp || b->owner == m); } JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var) @@ -711,8 +714,11 @@ JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *va JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT) { + // this function is mostly only used during initialization, so the data races here are not too important to us jl_binding_t *bp = jl_get_binding_wr(m, var, 1); - if (bp->value == NULL) { + if (jl_atomic_load_relaxed(&bp->value) == NULL) { + jl_value_t *old_ty = NULL; + jl_atomic_cmpswap_relaxed(&bp->ty, &old_ty, (jl_value_t*)jl_any_type); uint8_t constp = 0; // if (jl_atomic_cmpswap(&bp->constp, &constp, 1)) { if (constp = bp->constp, bp->constp = 1, constp == 0) { @@ -722,8 +728,6 @@ JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var return; } } - jl_value_t *old_ty = NULL; - jl_atomic_cmpswap_relaxed(&bp->ty, &old_ty, (jl_value_t*)jl_any_type); } jl_errorf("invalid redefinition of constant %s", jl_symbol_name(bp->name)); @@ -738,7 +742,7 @@ JL_DLLEXPORT int jl_binding_is_const(jl_binding_t *b) JL_DLLEXPORT int jl_binding_boundp(jl_binding_t *b) { assert(b); - return b->value != 0; + return jl_atomic_load_relaxed(&b->value) != NULL; } JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var) @@ -788,27 +792,30 @@ void jl_binding_deprecation_warning(jl_module_t *m, jl_binding_t *b) if (b->deprecated == 1 && jl_options.depwarn) { if (jl_options.depwarn != JL_OPTIONS_DEPWARN_ERROR) jl_printf(JL_STDERR, "WARNING: "); - jl_binding_t *dep_message_binding = NULL; + jl_value_t *dep_message = NULL; if (b->owner) { jl_printf(JL_STDERR, "%s.%s is deprecated", jl_symbol_name(b->owner->name), jl_symbol_name(b->name)); - dep_message_binding = jl_get_dep_message_binding(b->owner, b); + jl_binding_t *dep_message_binding = jl_get_dep_message_binding(b->owner, b); + if (dep_message_binding != NULL) + dep_message = jl_atomic_load_relaxed(&dep_message_binding->value); } else { jl_printf(JL_STDERR, "%s is deprecated", jl_symbol_name(b->name)); } - if (dep_message_binding && dep_message_binding->value) { - if (jl_isa(dep_message_binding->value, (jl_value_t*)jl_string_type)) { - jl_uv_puts(JL_STDERR, jl_string_data(dep_message_binding->value), - jl_string_len(dep_message_binding->value)); + JL_GC_PUSH1(&dep_message); + if (dep_message != NULL) { + if (jl_is_string(dep_message)) { + jl_uv_puts(JL_STDERR, jl_string_data(dep_message), jl_string_len(dep_message)); } else { - jl_static_show(JL_STDERR, dep_message_binding->value); + jl_static_show(JL_STDERR, dep_message); } } else { - jl_value_t *v = b->value; + jl_value_t *v = jl_atomic_load_relaxed(&b->value); + dep_message = v; if (v) { if (jl_is_type(v) || jl_is_module(v)) { jl_printf(JL_STDERR, ", use "); @@ -817,8 +824,7 @@ void jl_binding_deprecation_warning(jl_module_t *m, jl_binding_t *b) } else { jl_methtable_t *mt = jl_gf_mtable(v); - if (mt != NULL && (mt->defs != jl_nothing || - jl_isa(v, (jl_value_t*)jl_builtin_type))) { + if (mt != NULL) { jl_printf(JL_STDERR, ", use "); if (mt->module != jl_core_module) { jl_static_show(JL_STDERR, (jl_value_t*)mt->module); @@ -831,6 +837,7 @@ void jl_binding_deprecation_warning(jl_module_t *m, jl_binding_t *b) } } jl_printf(JL_STDERR, "\n"); + JL_GC_POP(); if (jl_options.depwarn != JL_OPTIONS_DEPWARN_ERROR) { if (jl_lineno == 0) { @@ -887,7 +894,7 @@ JL_DLLEXPORT void jl_checked_assignment(jl_binding_t *b, jl_value_t *rhs) JL_DLLEXPORT void jl_declare_constant(jl_binding_t *b) { - if (b->value != NULL && !b->constp) { + if (jl_atomic_load_relaxed(&b->value) != NULL && !b->constp) { jl_errorf("cannot declare %s constant; it already has a value", jl_symbol_name(b->name)); } diff --git a/src/rtutils.c b/src/rtutils.c index f34303b9aeea5..6ac35760a5fc6 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -748,7 +748,7 @@ static size_t jl_static_show_x_(JL_STREAM *out, jl_value_t *v, jl_datatype_t *vt else { n += jl_static_show_x(out, (jl_value_t*)li->def.module, depth); n += jl_printf(out, ". -> "); - n += jl_static_show_x(out, li->uninferred, depth); + n += jl_static_show_x(out, jl_atomic_load_relaxed(&li->uninferred), depth); } } else if (vt == jl_typename_type) { diff --git a/src/stackwalk.c b/src/stackwalk.c index e81a7cda8249b..481b0abf9d701 100644 --- a/src/stackwalk.c +++ b/src/stackwalk.c @@ -661,7 +661,7 @@ void jl_print_bt_entry_codeloc(jl_bt_element_t *bt_entry) JL_NOTSAFEPOINT jl_value_t *code = jl_bt_entry_jlvalue(bt_entry, 0); if (jl_is_method_instance(code)) { // When interpreting a method instance, need to unwrap to find the code info - code = ((jl_method_instance_t*)code)->uninferred; + code = jl_atomic_load_relaxed(&((jl_method_instance_t*)code)->uninferred); } if (jl_is_code_info(code)) { jl_code_info_t *src = (jl_code_info_t*)code; diff --git a/src/staticdata.c b/src/staticdata.c index 7b33db4eadccc..2098596b9b612 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -1344,7 +1344,7 @@ static void jl_write_values(jl_serializer_state *s) JL_GC_DISABLED } else if (jl_is_method_instance(v)) { jl_method_instance_t *newmi = (jl_method_instance_t*)&s->s->buf[reloc_offset]; - newmi->precompiled = 0; + jl_atomic_store_relaxed(&newmi->precompiled, 0); } else if (jl_is_code_instance(v)) { // Handle the native-code pointers @@ -2062,7 +2062,7 @@ static void strip_specializations_(jl_method_instance_t *mi) codeinst = jl_atomic_load_relaxed(&codeinst->next); } if (jl_options.strip_ir) { - record_field_change(&mi->uninferred, NULL); + record_field_change((jl_value_t**)&mi->uninferred, NULL); record_field_change((jl_value_t**)&mi->backedges, NULL); record_field_change((jl_value_t**)&mi->callbacks, NULL); } diff --git a/src/toplevel.c b/src/toplevel.c index baacb4235a838..0b0b819a723a2 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -579,7 +579,7 @@ int jl_needs_lowering(jl_value_t *e) JL_NOTSAFEPOINT static jl_method_instance_t *method_instance_for_thunk(jl_code_info_t *src, jl_module_t *module) { jl_method_instance_t *li = jl_new_method_instance_uninit(); - li->uninferred = (jl_value_t*)src; + jl_atomic_store_relaxed(&li->uninferred, (jl_value_t*)src); li->specTypes = (jl_value_t*)jl_emptytuple_type; li->def.module = module; return li; diff --git a/stdlib/Serialization/src/Serialization.jl b/stdlib/Serialization/src/Serialization.jl index ce9220f7864a7..e36b5c67e2282 100644 --- a/stdlib/Serialization/src/Serialization.jl +++ b/stdlib/Serialization/src/Serialization.jl @@ -1089,7 +1089,7 @@ function deserialize(s::AbstractSerializer, ::Type{Core.MethodInstance}) deserialize_cycle(s, linfo) tag = Int32(read(s.io, UInt8)::UInt8) if tag != UNDEFREF_TAG - linfo.uninferred = handle_deserialize(s, tag)::CodeInfo + setfield!(linfo, :uninferred, handle_deserialize(s, tag)::CodeInfo, :monotonic) end tag = Int32(read(s.io, UInt8)::UInt8) if tag != UNDEFREF_TAG @@ -1342,7 +1342,7 @@ function deserialize_typename(s::AbstractSerializer, number) mt.offs = 0 end mt.name = mtname - mt.max_args = maxa + setfield!(mt, :max_args, maxa, :monotonic) ccall(:jl_set_nth_field, Cvoid, (Any, Csize_t, Any), tn, Base.fieldindex(Core.TypeName, :mt)-1, mt) for def in defs if isdefined(def, :sig) diff --git a/test/atomics.jl b/test/atomics.jl index 15ffd84a2c0a2..e93afc3bff2c2 100644 --- a/test/atomics.jl +++ b/test/atomics.jl @@ -266,8 +266,10 @@ test_field_operators(ARefxy{Float64}(123_10, 123_20)) nothing end @noinline function test_field_orderings(r, x, y) - _test_field_orderings(Ref(copy(r)), x, y) - _test_field_orderings(Ref{Any}(copy(r)), x, y) + @testset "$r" begin + _test_field_orderings(Ref(copy(r)), x, y) + _test_field_orderings(Ref{Any}(copy(r)), x, y) + end nothing end @noinline test_field_orderings(x, y) = (@nospecialize; test_field_orderings(ARefxy(x, y), x, y)) diff --git a/test/core.jl b/test/core.jl index 4baf78ed2977c..116b514b2ed5f 100644 --- a/test/core.jl +++ b/test/core.jl @@ -16,15 +16,31 @@ for (T, c) in ( (Core.CodeInfo, []), (Core.CodeInstance, [:def, :rettype, :rettype_const, :ipo_purity_bits, :argescapes]), (Core.Method, [#=:name, :module, :file, :line, :primary_world, :sig, :slot_syms, :external_mt, :nargs, :called, :nospecialize, :nkw, :isva, :pure, :is_for_opaque_closure, :constprop=#]), - (Core.MethodInstance, [#=:def, :specTypes, :sparam_vals]=#]), + (Core.MethodInstance, [#=:def, :specTypes, :sparam_vals=#]), (Core.MethodTable, [:module]), (Core.TypeMapEntry, [:sig, :simplesig, :guardsigs, :min_world, :max_world, :func, :isleafsig, :issimplesig, :va]), (Core.TypeMapLevel, []), (Core.TypeName, [:name, :module, :names, :atomicfields, :constfields, :wrapper, :mt, :hash, :n_uninitialized, :flags]), (DataType, [:name, :super, :parameters, :instance, :hash]), + (TypeVar, [:name, :ub, :lb]), ) @test Set((fieldname(T, i) for i in 1:fieldcount(T) if isconst(T, i))) == Set(c) end +# +# sanity tests that our built-in types are marked correctly for atomic fields +for (T, c) in ( + (Core.CodeInfo, []), + (Core.CodeInstance, [:next, :inferred, :purity_bits, :invoke, :specptr, :precompile]), + (Core.Method, []), + (Core.MethodInstance, [:uninferred, :cache, :precompiled]), + (Core.MethodTable, [:defs, :leafcache, :cache, :max_args]), + (Core.TypeMapEntry, [:next]), + (Core.TypeMapLevel, [:arg1, :targ, :name1, :tname, :list, :any]), + (Core.TypeName, [:cache, :linearcache]), + (DataType, [:types, :layout]), + ) + @test Set((fieldname(T, i) for i in 1:fieldcount(T) if Base.isfieldatomic(T, i))) == Set(c) +end @test_throws(ErrorException("setfield!: const field .name of type DataType cannot be changed"), setfield!(Int, :name, Int.name)) diff --git a/test/stacktraces.jl b/test/stacktraces.jl index cbb07a60e456b..fb873c1a5cfb7 100644 --- a/test/stacktraces.jl +++ b/test/stacktraces.jl @@ -104,7 +104,7 @@ let src = Meta.lower(Main, quote let x = 1 end end).args[1]::Core.CodeInfo, li = ccall(:jl_new_method_instance_uninit, Ref{Core.MethodInstance}, ()), sf - li.uninferred = src + setfield!(li, :uninferred, src, :monotonic) li.specTypes = Tuple{} li.def = @__MODULE__ sf = StackFrame(:a, :b, 3, li, false, false, 0)