Skip to content

Commit

Permalink
undo breaking change of removing parent field from CodeInfo (#53393)
Browse files Browse the repository at this point in the history
This drops the unnecessary breaking change from
#53219 by re-adding the optional
`parent` field to CodeInfo. A few months ago, I had actually already put
together a branch that also fixed Keno's complaints about how it was not
set reliably, so I copied that code here also, so that it should get set
more reliably whenever a CodeInfo is associated with a MethodInstance
(either because it called `retrieve_code_info` to get IR from the Method
or `uncompress_ir` to get it from the inference cache). This does not
entirely fix Cthulhu's test errors, since I don't see any particular
reason to re-introduce the other fields (min_world, max_world, inferred,
edges, method_for_inference_limit_heuristics) that got removed or are
now set incorrectly, and most errors appear to be instead related to the
`Expr(:boundscheck, true/false)` change. However, they could be
trivially re-added back as placeholders, if someone encounters a broken
package that still needs them.
  • Loading branch information
vtjnash authored Feb 21, 2024
2 parents 65f24da + 2126b67 commit 962d833
Show file tree
Hide file tree
Showing 21 changed files with 136 additions and 96 deletions.
2 changes: 1 addition & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,7 @@ function const_prop_methodinstance_heuristic(interp::AbstractInterpreter,
if isa(code, CodeInstance)
inferred = @atomic :monotonic code.inferred
# TODO propagate a specific `CallInfo` that conveys information about this call
if inlining_policy(interp, inferred, NoCallInfo(), IR_FLAG_NULL) !== nothing
if src_inlining_policy(interp, inferred, NoCallInfo(), IR_FLAG_NULL)
return true
end
end
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ function IRInterpretationState(interp::AbstractInterpreter,
@assert code.def === mi
src = @atomic :monotonic code.inferred
if isa(src, String)
src = _uncompressed_ir(mi.def, src)
src = _uncompressed_ir(code, src)
else
isa(src, CodeInfo) || return nothing
end
Expand Down
16 changes: 9 additions & 7 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -107,21 +107,23 @@ is_declared_noinline(@nospecialize src::MaybeCompressed) =
# OptimizationState #
#####################

function inlining_policy(interp::AbstractInterpreter,
# return whether this src should be inlined. If so, retrieve_ir_for_inlining must return an IRCode from it
function src_inlining_policy(interp::AbstractInterpreter,
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32)
if isa(src, MaybeCompressed)
src_inlineable = is_stmt_inline(stmt_flag) || is_inlineable(src)
return src_inlineable ? src : nothing
return src_inlineable
elseif isa(src, IRCode)
return src
return true
elseif isa(src, SemiConcreteResult)
return src
elseif isa(src, CodeInstance)
return inlining_policy(interp, src.inferred, info, stmt_flag)
return true
end
return nothing
@assert !isa(src, CodeInstance) # handled by caller
return false
end

function inlining_policy end # deprecated legacy name used by Cthulhu

struct InliningState{Interp<:AbstractInterpreter}
edges::Vector{Any}
world::UInt
Expand Down
65 changes: 40 additions & 25 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ function compileable_specialization(match::MethodMatch, effects::Effects,
end

struct InferredResult
src::Any
src::Any # CodeInfo or IRCode
effects::Effects
InferredResult(@nospecialize(src), effects::Effects) = new(src, effects)
end
Expand All @@ -849,11 +849,9 @@ end
# in this case function can be inlined to a constant
return ConstantCase(quoted(code.rettype_const))
end
src = @atomic :monotonic code.inferred
effects = decode_effects(code.ipo_purity_bits)
return InferredResult(src, effects)
return code
end
return InferredResult(nothing, Effects())
return nothing
end
@inline function get_local_result(inf_result::InferenceResult)
effects = inf_result.ipo_effects
Expand Down Expand Up @@ -887,7 +885,15 @@ function resolve_todo(mi::MethodInstance, result::Union{Nothing,InferenceResult,
add_inlining_backedge!(et, mi)
return inferred_result
end
(; src, effects) = inferred_result
if inferred_result isa InferredResult
(; src, effects) = inferred_result
elseif inferred_result isa CodeInstance
src = @atomic :monotonic inferred_result.inferred
effects = decode_effects(inferred_result.ipo_purity_bits)
else
src = nothing
effects = Effects()
end

# the duplicated check might have been done already within `analyze_method!`, but still
# we need it here too since we may come here directly using a constant-prop' result
Expand All @@ -896,12 +902,13 @@ function resolve_todo(mi::MethodInstance, result::Union{Nothing,InferenceResult,
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)
end

src = inlining_policy(state.interp, src, info, flag)
src === nothing && return compileable_specialization(mi, effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)
src_inlining_policy(state.interp, src, info, flag) ||
return compileable_specialization(mi, effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)

add_inlining_backedge!(et, mi)
ir = retrieve_ir_for_inlining(mi, src, preserve_local_sources)
ir = inferred_result isa CodeInstance ? retrieve_ir_for_inlining(inferred_result, src) :
retrieve_ir_for_inlining(mi, src, preserve_local_sources)
return InliningTodo(mi, ir, effects)
end

Expand All @@ -919,14 +926,22 @@ function resolve_todo(mi::MethodInstance, @nospecialize(info::CallInfo), flag::U
add_inlining_backedge!(et, mi)
return cached_result
end
(; src, effects) = cached_result

src = inlining_policy(state.interp, src, info, flag)

src === nothing && return nothing
if cached_result isa InferredResult
(; src, effects) = cached_result
elseif cached_result isa CodeInstance
src = @atomic :monotonic cached_result.inferred
effects = decode_effects(cached_result.ipo_purity_bits)
else
src = nothing
effects = Effects()
end

preserve_local_sources = true
src_inlining_policy(state.interp, src, info, flag) || return nothing
ir = cached_result isa CodeInstance ? retrieve_ir_for_inlining(cached_result, src) :
retrieve_ir_for_inlining(mi, src, preserve_local_sources)
add_inlining_backedge!(et, mi)
return InliningTodo(mi, retrieve_ir_for_inlining(mi, src), effects)
return InliningTodo(mi, ir, effects)
end

function validate_sparams(sparams::SimpleVector)
Expand Down Expand Up @@ -979,17 +994,17 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
return resolve_todo(mi, volatile_inf_result, info, flag, state; invokesig)
end

function retrieve_ir_for_inlining(mi::MethodInstance, src::String, ::Bool=true)
src = _uncompressed_ir(mi.def, src)
return inflate_ir!(src, mi)
function retrieve_ir_for_inlining(cached_result::CodeInstance, src::MaybeCompressed)
src = _uncompressed_ir(cached_result, src)::CodeInfo
return inflate_ir!(src, cached_result.def)
end
function retrieve_ir_for_inlining(mi::MethodInstance, src::CodeInfo, preserve_local_sources::Bool=true)
function retrieve_ir_for_inlining(mi::MethodInstance, src::CodeInfo, preserve_local_sources::Bool)
if preserve_local_sources
src = copy(src)
end
return inflate_ir!(src, mi)
end
function retrieve_ir_for_inlining(::MethodInstance, ir::IRCode, preserve_local_sources::Bool=true)
function retrieve_ir_for_inlining(mi::MethodInstance, ir::IRCode, preserve_local_sources::Bool)
if preserve_local_sources
ir = copy(ir)
end
Expand Down Expand Up @@ -1494,13 +1509,13 @@ function semiconcrete_result_item(result::SemiConcreteResult,
return compileable_specialization(mi, result.effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)
end
ir = inlining_policy(state.interp, result.ir, info, flag)
ir === nothing && return compileable_specialization(mi, result.effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)
src_inlining_policy(state.interp, result.ir, info, flag) ||
return compileable_specialization(mi, result.effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)

add_inlining_backedge!(et, mi)
preserve_local_sources = OptimizationParams(state.interp).preserve_local_sources
ir = retrieve_ir_for_inlining(mi, ir, preserve_local_sources)
ir = retrieve_ir_for_inlining(mi, result.ir, preserve_local_sources)
return InliningTodo(mi, ir, result.effects)
end

Expand Down
2 changes: 2 additions & 0 deletions base/compiler/ssair/legacy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ Mainly used for testing or interactive use.
inflate_ir(ci::CodeInfo, linfo::MethodInstance) = inflate_ir!(copy(ci), linfo)
inflate_ir(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{Any}) = inflate_ir!(copy(ci), sptypes, argtypes)
function inflate_ir(ci::CodeInfo)
parent = ci.parent
isa(parent, MethodInstance) && return inflate_ir(ci, parent)
# XXX the length of `ci.slotflags` may be different from the actual number of call
# arguments, but we really don't know that information in this case
argtypes = Any[ Any for i = 1:length(ci.slotflags) ]
Expand Down
7 changes: 3 additions & 4 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1492,12 +1492,11 @@ function try_inline_finalizer!(ir::IRCode, argexprs::Vector{Any}, idx::Int,
end
src = @atomic :monotonic code.inferred
else
src = nothing
return false
end

src = inlining_policy(inlining.interp, src, info, IR_FLAG_NULL)
src === nothing && return false
src = retrieve_ir_for_inlining(mi, src)
src_inlining_policy(inlining.interp, src, info, IR_FLAG_NULL) || return false
src = retrieve_ir_for_inlining(code, src)

# For now: Require finalizer to only have one basic block
length(src.cfg.blocks) == 1 || return false
Expand Down
40 changes: 21 additions & 19 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,20 @@ function CodeInstance(interp::AbstractInterpreter, result::InferenceResult;
end
end
relocatability = 0x0
owner = cache_owner(interp)
if const_flags == 0x3 && can_discard_trees
inferred_result = nothing
relocatability = 0x1
else
inferred_result = transform_result_for_cache(interp, result.linfo, result.valid_worlds, result, can_discard_trees)
if inferred_result isa CodeInfo
uncompressed = inferred_result
inferred_result = maybe_compress_codeinfo(interp, result.linfo, inferred_result, can_discard_trees)
result.is_src_volatile |= uncompressed !== inferred_result
elseif owner === nothing
# The global cache can only handle objects that codegen understands
inferred_result = nothing
end
if isa(inferred_result, String)
t = @_gc_preserve_begin inferred_result
relocatability = unsafe_load(unsafe_convert(Ptr{UInt8}, inferred_result), Core.sizeof(inferred_result))
Expand All @@ -323,15 +332,21 @@ function CodeInstance(interp::AbstractInterpreter, result::InferenceResult;
relocatability = 0x1
end
end
# relocatability = isa(inferred_result, String) ? inferred_result[end] : UInt8(0)
return CodeInstance(result.linfo, cache_owner(interp),
# n.b. relocatability = (isa(inferred_result, String) && inferred_result[end]) || inferred_result === nothing
return CodeInstance(result.linfo, owner,
widenconst(result_type), widenconst(result.exc_result), rettype_const, inferred_result,
const_flags, first(result.valid_worlds), last(result.valid_worlds),
# TODO: Actually do something with non-IPO effects
encode_effects(result.ipo_effects), encode_effects(result.ipo_effects), result.analysis_results,
relocatability)
end

function transform_result_for_cache(interp::AbstractInterpreter,
linfo::MethodInstance, valid_worlds::WorldRange, result::InferenceResult,
can_discard_trees::Bool=may_discard_trees(interp))
return result.src
end

function maybe_compress_codeinfo(interp::AbstractInterpreter, linfo::MethodInstance, ci::CodeInfo,
can_discard_trees::Bool=may_discard_trees(interp))
def = linfo.def
Expand All @@ -354,22 +369,6 @@ function maybe_compress_codeinfo(interp::AbstractInterpreter, linfo::MethodInsta
end
end

function transform_result_for_cache(interp::AbstractInterpreter,
linfo::MethodInstance, valid_worlds::WorldRange, result::InferenceResult,
can_discard_trees::Bool=may_discard_trees(interp))
inferred_result = result.src
if inferred_result isa CodeInfo
uncompressed = inferred_result
inferred_result = maybe_compress_codeinfo(interp, linfo, inferred_result, can_discard_trees)
result.is_src_volatile |= uncompressed !== inferred_result
end
# The global cache can only handle objects that codegen understands
if !isa(inferred_result, MaybeCompressed)
inferred_result = nothing
end
return inferred_result
end

function cache_result!(interp::AbstractInterpreter, result::InferenceResult)
if last(result.valid_worlds) == get_world_counter()
# if we've successfully recorded all of the backedges in the global reverse-cache,
Expand Down Expand Up @@ -874,7 +873,7 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
# propagate newly inferred source to the inliner, allowing efficient inlining w/o deserialization:
# note that this result is cached globally exclusively, we can use this local result destructively
volatile_inf_result = isinferred && (force_inline || inlining_policy(interp, result.src, NoCallInfo(), IR_FLAG_NULL) !== nothing) ?
volatile_inf_result = isinferred && (force_inline || src_inlining_policy(interp, result.src, NoCallInfo(), IR_FLAG_NULL) !== nothing) ?
VolatileInferenceResult(result) : nothing
return EdgeCallResult(frame.bestguess, exc_bestguess, edge, effects, volatile_inf_result)
elseif frame === true
Expand Down Expand Up @@ -930,6 +929,7 @@ function codeinfo_for_const(interp::AbstractInterpreter, mi::MethodInstance, @no
tree.linetable = LineInfoNode[LineInfoNode(method.module, method.name, method.file, method.line, Int32(0))]
tree.ssaflags = UInt32[0]
set_inlineable!(tree, true)
tree.parent = mi
return tree
end

Expand Down Expand Up @@ -1091,6 +1091,8 @@ function ci_meets_requirement(code::CodeInstance, source_mode::UInt8, ci_is_cach
return false
end

_uncompressed_ir(ci::Core.CodeInstance, s::String) = ccall(:jl_uncompress_ir, Any, (Any, Any, Any), ci.def.def::Method, ci, s)::CodeInfo

# compute (and cache) an inferred AST and return type
function typeinf_ext(interp::AbstractInterpreter, mi::MethodInstance, source_mode::UInt8)
start_time = ccall(:jl_typeinf_timing_begin, UInt64, ())
Expand Down
3 changes: 2 additions & 1 deletion base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,13 @@ function retrieve_code_info(linfo::MethodInstance, world::UInt)
# can happen in images built with --strip-ir
return nothing
elseif isa(src, String)
c = _uncompressed_ir(def, src)
c = ccall(:jl_uncompress_ir, Any, (Any, Ptr{Cvoid}, Any), def, C_NULL, src)
else
c = copy(src::CodeInfo)
end
end
if c isa CodeInfo
c.parent = linfo
return c
end
return nothing
Expand Down
12 changes: 7 additions & 5 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1313,12 +1313,14 @@ function length(mt::Core.MethodTable)
end
isempty(mt::Core.MethodTable) = (mt.defs === nothing)

uncompressed_ir(m::Method) = isdefined(m, :source) ? _uncompressed_ir(m, m.source) :
uncompressed_ir(m::Method) = isdefined(m, :source) ? _uncompressed_ir(m) :
isdefined(m, :generator) ? error("Method is @generated; try `code_lowered` instead.") :
error("Code for this Method is not available.")
_uncompressed_ir(m::Method, s::CodeInfo) = copy(s)
_uncompressed_ir(m::Method, s::String) = ccall(:jl_uncompress_ir, Any, (Any, Any), m, s)::CodeInfo
_uncompressed_ir(ci::Core.CodeInstance, s::String) = ccall(:jl_uncompress_ir, Any, (Any, Any), ci.def.def::Method, s)::CodeInfo
function _uncompressed_ir(m::Method)
s = m.source
s isa String && (s = ccall(:jl_uncompress_ir, Any, (Any, Ptr{Cvoid}, Any), m, C_NULL, s))
return s::CodeInfo
end
# for backwards compat
const uncompressed_ast = uncompressed_ir
const _uncompressed_ast = _uncompressed_ir
Expand Down Expand Up @@ -1632,7 +1634,7 @@ function get_oc_code_rt(@nospecialize(oc::Core.OpaqueClosure))
ccall(:jl_is_in_pure_context, Bool, ()) && error("code reflection cannot be used from generated functions")
m = oc.source
if isa(m, Method)
code = _uncompressed_ir(m, m.source)
code = _uncompressed_ir(m)
return Pair{CodeInfo,Any}(code, typeof(oc).parameters[2])
else
error("encountered invalid Core.OpaqueClosure object")
Expand Down
6 changes: 3 additions & 3 deletions src/aotcompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ static void jl_ci_cache_lookup(const jl_cgparams_t &cgparams, jl_method_instance
if ((jl_value_t*)*src_out == jl_nothing)
*src_out = NULL;
if (*src_out && jl_is_method(def))
*src_out = jl_uncompress_ir(def, (jl_value_t*)*src_out);
*src_out = jl_uncompress_ir(def, codeinst, (jl_value_t*)*src_out);
}
if (*src_out == NULL || !jl_is_code_info(*src_out)) {
if (cgparams.lookup != jl_rettype_inferred_addr) {
Expand Down Expand Up @@ -1950,7 +1950,7 @@ extern "C" JL_DLLEXPORT_CODEGEN jl_code_info_t *jl_gdbdumpcode(jl_method_instanc
src = (jl_code_info_t*)jl_atomic_load_relaxed(&codeinst->inferred);
if ((jl_value_t*)src != jl_nothing && !jl_is_code_info(src) && jl_is_method(mi->def.method)) {
JL_GC_PUSH2(&codeinst, &src);
src = jl_uncompress_ir(mi->def.method, (jl_value_t*)src);
src = jl_uncompress_ir(mi->def.method, codeinst, (jl_value_t*)src);
JL_GC_POP();
}
}
Expand Down Expand Up @@ -1989,7 +1989,7 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz
}
if (src) {
if ((jl_value_t*)src != jl_nothing && !jl_is_code_info(src) && jl_is_method(mi->def.method))
src = jl_uncompress_ir(mi->def.method, (jl_value_t*)src);
src = jl_uncompress_ir(mi->def.method, codeinst, (jl_value_t*)src);
}

// emit this function into a new llvm module
Expand Down
4 changes: 2 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6015,7 +6015,7 @@ static std::pair<Function*, Function*> get_oc_function(jl_codectx_t &ctx, jl_met

if (it == ctx.emission_context.compiled_functions.end()) {
++EmittedOpaqueClosureFunctions;
jl_code_info_t *ir = jl_uncompress_ir(closure_method, (jl_value_t*)inferred);
jl_code_info_t *ir = jl_uncompress_ir(closure_method, ci, (jl_value_t*)inferred);
JL_GC_PUSH1(&ir);
// TODO: Emit this inline and outline it late using LLVM's coroutine support.
orc::ThreadSafeModule closure_m = jl_create_ts_module(
Expand Down Expand Up @@ -9570,7 +9570,7 @@ jl_llvm_functions_t jl_emit_codeinst(
return jl_emit_oc_wrapper(m, params, codeinst->def, codeinst->rettype);
}
if (src && (jl_value_t*)src != jl_nothing && jl_is_method(def))
src = jl_uncompress_ir(def, (jl_value_t*)src);
src = jl_uncompress_ir(def, codeinst, (jl_value_t*)src);
if (!src || !jl_is_code_info(src)) {
JL_GC_POP();
m = orc::ThreadSafeModule();
Expand Down
Loading

0 comments on commit 962d833

Please sign in to comment.