From 16d4956c1853a053ac6c83c1648e2971487bf0b3 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Sun, 27 Mar 2022 13:22:44 +0900 Subject: [PATCH 1/3] fix #44732, propagate the effects of a thrown concrete call correctly --- base/compiler/abstractinterpretation.jl | 23 ++++++++++------------- base/compiler/ssair/inlining.jl | 6 +++--- base/compiler/stmtinfo.jl | 5 +++-- test/compiler/inline.jl | 22 ++++++++++++++++++++++ 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 3e34528aa8284..3a93526293c2d 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -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 @@ -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 @@ -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...) - 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 diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 225bd46c6f262..c06ddfbbcca8d 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -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), diff --git a/base/compiler/stmtinfo.jl b/base/compiler/stmtinfo.jl index e3f69b2c43e54..3eeff0c2c86a8 100644 --- a/base/compiler/stmtinfo.jl +++ b/base/compiler/stmtinfo.jl @@ -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 """ diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl index fa4425893767c..f586bd6435feb 100644 --- a/test/compiler/inline.jl +++ b/test/compiler/inline.jl @@ -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)) + global x44200::Int = 0 function f44200() global x = 0 From bbd498384e67ae1a40c7ac3e9d3c8cf0dc2f17ff Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Tue, 29 Mar 2022 10:15:46 +0900 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Jameson Nash --- base/compiler/abstractinterpretation.jl | 5 ++--- test/compiler/inline.jl | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 3a93526293c2d..c2927519a2eb0 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -739,9 +739,8 @@ 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...) + value = try + Core._call_in_world_total(get_world_counter(interp), f, args...) 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), result.edge_effects) diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl index f586bd6435feb..cc94ace0026df 100644 --- a/test/compiler/inline.jl +++ b/test/compiler/inline.jl @@ -1115,7 +1115,7 @@ end let src = code_typed1(issue44732, (Container44732,)) @test any(isinvoke(:validate44732), src.code) end -@test_throws ErrorException issue44732(Container44732(nothing)) +@test_throws ErrorException("don't erase this error!") issue44732(Container44732(nothing)) global x44200::Int = 0 function f44200() From 8fef8f7c6132c68eecdadaaa1047c68e8133530b Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Tue, 29 Mar 2022 10:17:58 +0900 Subject: [PATCH 3/3] limit pure/concrete-eval error handlings --- base/compiler/abstractinterpretation.jl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index c2927519a2eb0..5329df21b60aa 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -700,12 +700,12 @@ function pure_eval_call(interp::AbstractInterpreter, end function _pure_eval_call(@nospecialize(f), arginfo::ArgInfo) args = collect_const_args(arginfo) - try - value = Core._apply_pure(f, args) - return Const(value) + value = try + Core._apply_pure(f, args) catch return nothing end + return Const(value) end function concrete_eval_eligible(interp::AbstractInterpreter, @@ -739,8 +739,9 @@ 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) + world = get_world_counter(interp) value = try - Core._call_in_world_total(get_world_counter(interp), f, args...) + Core._call_in_world_total(world, f, args...) 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), result.edge_effects)