Skip to content
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.

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

fix #44732, propagate effects of a thrown concrete call correctly #44762

Merged
merged 3 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
23 changes: 10 additions & 13 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end
tristate_merge!(sv, effects)
push!(const_results, const_result)
if const_result !== nothing
any_const_result = true
end
any_const_result |= const_result !== nothing
this_rt = tmerge(this_rt, rt)
if bail_out_call(interp, this_rt, sv)
break
Expand Down Expand Up @@ -187,9 +185,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end
tristate_merge!(sv, effects)
push!(const_results, const_result)
if const_result !== nothing
any_const_result = true
end
any_const_result |= const_result !== nothing
end
@assert !(this_conditional isa Conditional) "invalid lattice element returned from inter-procedural context"
seen += 1
Expand Down Expand Up @@ -743,17 +739,18 @@ function concrete_eval_call(interp::AbstractInterpreter,
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState)
concrete_eval_eligible(interp, f, result, arginfo, sv) || return nothing
args = collect_const_args(arginfo)
local value
try
value = Core._call_in_world_total(get_world_counter(interp), f, args...)
aviatesk marked this conversation as resolved.
Show resolved Hide resolved
if is_inlineable_constant(value) || call_result_unused(sv)
# If the constant is not inlineable, still do the const-prop, since the
# code that led to the creation of the Const may be inlineable in the same
# circumstance and may be optimizable.
return ConstCallResults(Const(value), ConstResult(result.edge, value), EFFECTS_TOTAL)
end
catch
# The evaulation threw. By :consistent-cy, we're guaranteed this would have happened at runtime
return ConstCallResults(Union{}, ConstResult(result.edge), result.edge_effects)
return ConstCallResults(Union{}, ConstResult(result.edge, result.edge_effects), result.edge_effects)
end
if is_inlineable_constant(value) || call_result_unused(sv)
# If the constant is not inlineable, still do the const-prop, since the
# code that led to the creation of the Const may be inlineable in the same
# circumstance and may be optimizable.
return ConstCallResults(Const(value), ConstResult(result.edge, EFFECTS_TOTAL, value), EFFECTS_TOTAL)
end
return nothing
end
Expand Down
6 changes: 3 additions & 3 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1330,10 +1330,10 @@ end

function const_result_item(result::ConstResult, state::InliningState)
if !isdefined(result, :result) || !is_inlineable_constant(result.result)
return compileable_specialization(state.et, result.mi, EFFECTS_TOTAL)
else
return ConstantCase(quoted(result.result))
return compileable_specialization(state.et, result.mi, result.effects)
end
@assert result.effects === EFFECTS_TOTAL
return ConstantCase(quoted(result.result))
end

function handle_cases!(ir::IRCode, idx::Int, stmt::Expr, @nospecialize(atype),
Expand Down
5 changes: 3 additions & 2 deletions base/compiler/stmtinfo.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ end

struct ConstResult
mi::MethodInstance
effects::Effects
result
ConstResult(mi::MethodInstance) = new(mi)
ConstResult(mi::MethodInstance, @nospecialize val) = new(mi, val)
ConstResult(mi::MethodInstance, effects::Effects) = new(mi, effects)
ConstResult(mi::MethodInstance, effects::Effects, @nospecialize val) = new(mi, effects, val)
end

"""
Expand Down
22 changes: 22 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,28 @@ Base.@assume_effects :consistent :effect_free :terminates_globally consteval(
consteval(getindex, ___CONST_DICT___, :a)
end

# https://github.com/JuliaLang/julia/issues/44732
struct Component44732
v
end
struct Container44732
x::Union{Nothing,Component44732}
end

# NOTE make sure to prevent inference bail out
validate44732(::Component44732) = nothing
validate44732(::Any) = error("don't erase this error!")

function issue44732(c::Container44732)
validate44732(c.x)
return nothing
end

let src = code_typed1(issue44732, (Container44732,))
@test any(isinvoke(:validate44732), src.code)
end
@test_throws ErrorException issue44732(Container44732(nothing))
aviatesk marked this conversation as resolved.
Show resolved Hide resolved

global x44200::Int = 0
function f44200()
global x = 0
Expand Down