Skip to content

Commit

Permalink
disable irinterp for interpreters with may_optimize(...)=false (Jul…
Browse files Browse the repository at this point in the history
…iaLang#53580)

As discussed at
<JuliaLang@b8a0a39#commitcomment-139076159>,
currently external abstract interpreter that configures `may_optimize`
to return `false` may end up with the internal error from irinterp since
it fundamentally required optimized IR but it currently assumes that all
sources from cached `CodeInstance`s are optimized.

This commit addresses the issue by incorporating a `may_optimize` check
in `concrete_eval_eligible`, which in turn automatically disables
irinterp for such interpreters. Although there were earlier discussions
suggesting the revival of `codeinfo.inferred::Bool`, this commit does
not need it, and I think this approach maintains the current state more
cleanly.

This should fix the error of `"inference"` benchmarks from
BaseBenchmarks.jl.
  • Loading branch information
aviatesk authored and mkitti committed Apr 13, 2024
1 parent 09b1efc commit bd55163
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
7 changes: 6 additions & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,12 @@ function concrete_eval_eligible(interp::AbstractInterpreter,
add_remark!(interp, sv, "[constprop] Concrete eval disabled for overlayed methods")
end
if !any_conditional(arginfo)
return :semi_concrete_eval
if may_optimize(interp)
return :semi_concrete_eval
else
# disable irinterp if optimization is disabled, since it requires optimized IR
add_remark!(interp, sv, "[constprop] Semi-concrete interpretation disabled for non-optimizing interpreter")
end
end
end
return :none
Expand Down
11 changes: 11 additions & 0 deletions test/compiler/AbstractInterpreter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ const CC = Core.Compiler
include("irutils.jl")
include("newinterp.jl")

# interpreter that performs abstract interpretation only
# (semi-concrete interpretation should be disabled automatically)
@newinterp AbsIntOnlyInterp1
CC.may_optimize(::AbsIntOnlyInterp1) = false
@test Base.infer_return_type(Base.init_stdio, (Ptr{Cvoid},); interp=AbsIntOnlyInterp1()) >: IO

# it should work even if the interpreter discards inferred source entirely
@newinterp AbsIntOnlyInterp2
CC.may_optimize(::AbsIntOnlyInterp2) = false
CC.transform_result_for_cache(::AbsIntOnlyInterp2, ::Core.MethodInstance, ::CC.WorldRange, ::CC.InferenceResult) = nothing
@test Base.infer_return_type(Base.init_stdio, (Ptr{Cvoid},); interp=AbsIntOnlyInterp2()) >: IO

# OverlayMethodTable
# ==================
Expand Down

0 comments on commit bd55163

Please sign in to comment.