Skip to content

[release-1.9] Atomically order specsigflags, specptr, and invoke (#47832) #48874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[release-1.9] Atomically order specsigflags, specptr, and invoke (#47832) #48874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
912cda1
Merge pull request #48756 from JuliaLang/avi/backport
KristofferC Feb 22, 2023
26b1181
inference: fix the correctness of inference bail out interface (#48826)
aviatesk Mar 1, 2023
7b92f6d
effects: taint `nonoverlayed` when bailing out inference (#48838)
aviatesk Mar 1, 2023
1cf35bc
Atomically order specsigflags, specptr, and invoke (#47832)
pchintalapudi Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions base/compiler/abstractinterpretation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,9 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
match = applicable[i]::MethodMatch
method = match.method
sig = match.spec_types
if bail_out_toplevel_call(interp, sig, sv)
if bail_out_toplevel_call(interp, InferenceLoopState(sig, rettype, all_effects), sv)
# only infer concrete call sites in top-level expressions
add_remark!(interp, sv, "Refusing to infer non-concrete call site in top-level expression")
rettype = Any
break
end
this_rt = Bottom
Expand Down Expand Up @@ -191,7 +190,8 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
conditionals[2][i] = tmerge(conditionals[2][i], cnd.elsetype)
end
end
if bail_out_call(interp, rettype, sv)
if bail_out_call(interp, InferenceLoopState(sig, rettype, all_effects), sv)
add_remark!(interp, sv, "Call inference reached maximally imprecise information. Bailing on.")
break
end
end
Expand All @@ -201,10 +201,10 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
info = ConstCallInfo(info, const_results)
end

if seen != napplicable
# there may be unanalyzed effects within unseen dispatch candidate,
# but we can still ignore nonoverlayed effect here since we already accounted for it
all_effects = merge_effects(all_effects, EFFECTS_UNKNOWN)
if seen ≠ napplicable
# there is unanalyzed candidate, widen type and effects to the top
rettype = Any
all_effects = Effects()
elseif isa(matches, MethodMatches) ? (!matches.fullmatch || any_ambig(matches)) :
(!all(matches.fullmatches) || any_ambig(matches))
# Account for the fact that we may encounter a MethodError with a non-covered or ambiguous signature.
Expand Down Expand Up @@ -1529,7 +1529,9 @@ function abstract_apply(interp::AbstractInterpreter, argtypes::Vector{Any}, si::
end
retinfos = ApplyCallInfo[]
retinfo = UnionSplitApplyCallInfo(retinfos)
for i = 1:length(ctypes)
napplicable = length(ctypes)
seen = 0
for i = 1:napplicable
ct = ctypes[i]
arginfo = infos[i]
lct = length(ct)
Expand All @@ -1543,17 +1545,21 @@ function abstract_apply(interp::AbstractInterpreter, argtypes::Vector{Any}, si::
end
end
call = abstract_call(interp, ArgInfo(nothing, ct), si, sv, max_methods)
seen += 1
push!(retinfos, ApplyCallInfo(call.info, arginfo))
res = tmerge(res, call.rt)
effects = merge_effects(effects, call.effects)
if bail_out_apply(interp, res, sv)
if i != length(ctypes)
# No point carrying forward the info, we're not gonna inline it anyway
retinfo = NoCallInfo()
end
if bail_out_apply(interp, InferenceLoopState(ct, res, effects), sv)
add_remark!(interp, sv, "_apply_iterate inference reached maximally imprecise information. Bailing on.")
break
end
end
if seen ≠ napplicable
# there is unanalyzed candidate, widen type and effects to the top
res = Any
effects = Effects()
retinfo = NoCallInfo() # NOTE this is necessary to prevent the inlining processing
end
# TODO: Add a special info type to capture all the iteration info.
# For now, only propagate info if we don't also union-split the iteration
return CallMeta(res, effects, retinfo)
Expand Down
21 changes: 15 additions & 6 deletions base/compiler/inferencestate.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,23 @@ is_effect_overridden(override::EffectsOverride, effect::Symbol) = getfield(overr

add_remark!(::AbstractInterpreter, sv::Union{InferenceState, IRCode}, remark) = return

function bail_out_toplevel_call(::AbstractInterpreter, @nospecialize(callsig), sv::Union{InferenceState, IRCode})
return isa(sv, InferenceState) && sv.restrict_abstract_call_sites && !isdispatchtuple(callsig)
struct InferenceLoopState
sig
rt
effects::Effects
function InferenceLoopState(@nospecialize(sig), @nospecialize(rt), effects::Effects)
new(sig, rt, effects)
end
end

function bail_out_toplevel_call(::AbstractInterpreter, state::InferenceLoopState, sv::Union{InferenceState, IRCode})
return isa(sv, InferenceState) && sv.restrict_abstract_call_sites && !isdispatchtuple(state.sig)
end
function bail_out_call(::AbstractInterpreter, @nospecialize(rt), sv::Union{InferenceState, IRCode})
return rt === Any
function bail_out_call(::AbstractInterpreter, state::InferenceLoopState, sv::Union{InferenceState, IRCode})
return state.rt === Any
end
function bail_out_apply(::AbstractInterpreter, @nospecialize(rt), sv::Union{InferenceState, IRCode})
return rt === Any
function bail_out_apply(::AbstractInterpreter, state::InferenceLoopState, sv::Union{InferenceState, IRCode})
return state.rt === Any
end

was_reached(sv::InferenceState, pc::Int) = sv.ssavaluetypes[pc] !== NOT_FOUND
Expand Down
40 changes: 30 additions & 10 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4215,7 +4215,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, const
jl_value_t *ci = ctx.params->lookup(mi, ctx.world, ctx.world); // TODO: need to use the right pair world here
jl_code_instance_t *codeinst = (jl_code_instance_t*)ci;
if (ci != jl_nothing) {
auto invoke = jl_atomic_load_relaxed(&codeinst->invoke);
auto invoke = jl_atomic_load_acquire(&codeinst->invoke);
// check if we know how to handle this specptr
if (invoke == jl_fptr_const_return_addr) {
result = mark_julia_const(ctx, codeinst->rettype_const);
Expand All @@ -4240,10 +4240,14 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, const
if (cache_valid) {
// optimization: emit the correct name immediately, if we know it
// TODO: use `emitted` map here too to try to consolidate names?
auto invoke = jl_atomic_load_relaxed(&codeinst->invoke);
// 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 fptr = jl_atomic_load_relaxed(&codeinst->specptr.fptr);
if (fptr) {
if (specsig ? codeinst->isspecsig : invoke == jl_fptr_args_addr) {
while (!(jl_atomic_load_acquire(&codeinst->specsigflags) & 0b10)) {
jl_cpu_pause();
}
invoke = jl_atomic_load_relaxed(&codeinst->invoke);
if (specsig ? jl_atomic_load_relaxed(&codeinst->specsigflags) & 0b1 : invoke == jl_fptr_args_addr) {
protoname = jl_ExecutionEngine->getFunctionAtAddress((uintptr_t)fptr, codeinst);
need_to_emit = false;
}
Expand Down Expand Up @@ -5763,18 +5767,27 @@ 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_acquire(&codeinst->invoke);
auto fptr = jl_atomic_load_relaxed(&codeinst->specptr.fptr);
assert(invoke);
if (fptr) {
while (!(jl_atomic_load_acquire(&codeinst->specsigflags) & 0b10)) {
jl_cpu_pause();
}
invoke = jl_atomic_load_relaxed(&codeinst->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) {
// don't need the fptr
callptr = (void*)codeinst->rettype_const;
calltype = 2;
}
else if (codeinst->isspecsig) {
callptr = codeinst->specptr.fptr;
else if (jl_atomic_load_relaxed(&codeinst->specsigflags) & 0b1) {
callptr = fptr;
calltype = 3;
}
astrt = codeinst->rettype;
Expand Down Expand Up @@ -8500,18 +8513,25 @@ void jl_compile_workqueue(
"invalid world for code-instance");
StringRef preal_decl = "";
bool preal_specsig = false;
auto invoke = jl_atomic_load_relaxed(&codeinst->invoke);
auto invoke = jl_atomic_load_acquire(&codeinst->invoke);
bool cache_valid = params.cache;
if (params.external_linkage) {
cache_valid = 0 && jl_object_in_image((jl_value_t*)codeinst);
}
// 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 (cache_valid && invoke != NULL) {
auto fptr = jl_atomic_load_relaxed(&codeinst->specptr.fptr);
if (fptr) {
while (!(jl_atomic_load_acquire(&codeinst->specsigflags) & 0b10)) {
jl_cpu_pause();
}
// in case we are racing with another thread that is emitting this function
invoke = jl_atomic_load_relaxed(&codeinst->invoke);
}
if (invoke == jl_fptr_args_addr) {
preal_decl = jl_ExecutionEngine->getFunctionAtAddress((uintptr_t)fptr, codeinst);
}
else if (codeinst->isspecsig) {
else if (jl_atomic_load_relaxed(&codeinst->specsigflags) & 0b1) {
preal_decl = jl_ExecutionEngine->getFunctionAtAddress((uintptr_t)fptr, codeinst);
preal_specsig = true;
}
Expand Down
Loading