Skip to content

Commit 7b587ac

Browse files
authored
irinterp: fix semi-concrete evaluation of overlay methods (#50739)
This commit introduces several fixes related to the overlay method and its semi-concrete evaluation. When `f` is being overlayed, semi-concrete interpretation is performed on `f`. While this isn't problematic in itself, the issue arises since there is no overlay check within concrete evaluation in the semi-concrete interpretation (`concrete_eval_invoke`), potentially leading to mis-compilation. This commit makes adjustments to allow semi-concrete interpretation for overlay methods, while preventing the concretization of overlay methods within `concrete_eval_invoke`. Note: Confirmed GPUCompiler test suite passes with this patch after JuliaGPU/GPUCompiler.jl#488.
1 parent 9808035 commit 7b587ac

File tree

4 files changed

+63
-27
lines changed

4 files changed

+63
-27
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
2828
return CallMeta(Any, Effects(), NoCallInfo())
2929
end
3030

31-
(; valid_worlds, applicable, info) = matches
31+
(; valid_worlds, applicable, info, nonoverlayed) = matches
3232
update_valid_age!(sv, valid_worlds)
3333
napplicable = length(applicable)
3434
rettype = Bottom
@@ -39,13 +39,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
3939
const_results = Union{Nothing,ConstResult}[]
4040
multiple_matches = napplicable > 1
4141
fargs = arginfo.fargs
42-
all_effects = EFFECTS_TOTAL
43-
if !matches.nonoverlayed
44-
# currently we don't have a good way to execute the overlayed method definition,
45-
# so we should give up concrete eval when any of the matched methods is overlayed
46-
f = nothing
47-
all_effects = Effects(all_effects; nonoverlayed=false)
48-
end
42+
all_effects = Effects(EFFECTS_TOTAL; nonoverlayed)
4943

5044
𝕃ₚ = ipo_lattice(interp)
5145
for i in 1:napplicable
@@ -792,7 +786,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter,
792786
end
793787
eligibility = concrete_eval_eligible(interp, f, result, arginfo, sv)
794788
if eligibility === :concrete_eval
795-
return concrete_eval_call(interp, f, result, arginfo, sv, invokecall)
789+
return concrete_eval_call(interp, f, result, arginfo, sv; invokecall)
796790
end
797791
mi = maybe_get_const_prop_profitable(interp, result, f, arginfo, si, match, sv)
798792
mi === nothing && return nothing
@@ -848,16 +842,17 @@ function concrete_eval_eligible(interp::AbstractInterpreter,
848842
add_remark!(interp, sv, "[constprop] Concrete evel disabled for inbounds")
849843
return :none
850844
end
851-
if isoverlayed(method_table(interp)) && !is_nonoverlayed(effects)
852-
# disable concrete-evaluation if this function call is tainted by some overlayed
853-
# method since currently there is no direct way to execute overlayed methods
854-
add_remark!(interp, sv, "[constprop] Concrete evel disabled for overlayed methods")
855-
return :none
856-
end
857-
if result.edge !== nothing && is_foldable(effects)
845+
mi = result.edge
846+
if mi !== nothing && is_foldable(effects)
858847
if f !== nothing && is_all_const_arg(arginfo, #=start=#2)
859-
return :concrete_eval
860-
elseif !any_conditional(arginfo)
848+
if is_nonoverlayed(mi.def::Method) && (!isoverlayed(method_table(interp)) || is_nonoverlayed(effects))
849+
return :concrete_eval
850+
end
851+
# disable concrete-evaluation if this function call is tainted by some overlayed
852+
# method since currently there is no easy way to execute overlayed methods
853+
add_remark!(interp, sv, "[constprop] Concrete evel disabled for overlayed methods")
854+
end
855+
if !any_conditional(arginfo)
861856
return :semi_concrete_eval
862857
end
863858
end
@@ -886,8 +881,8 @@ function collect_const_args(argtypes::Vector{Any}, start::Int)
886881
end
887882

888883
function concrete_eval_call(interp::AbstractInterpreter,
889-
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo,
890-
sv::AbsIntState, invokecall::Union{InvokeCall,Nothing})
884+
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::AbsIntState;
885+
invokecall::Union{InvokeCall,Nothing}=nothing)
891886
args = collect_const_args(arginfo, #=start=#2)
892887
if invokecall !== nothing
893888
# this call should be `invoke`d, rewrite `args` back now
@@ -1923,7 +1918,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
19231918
# argtypes′[i] = t ⊑ a ? t : a
19241919
# end
19251920
𝕃ₚ = ipo_lattice(interp)
1926-
f = overlayed ? nothing : singleton_type(ft′)
1921+
f = singleton_type(ft′)
19271922
invokecall = InvokeCall(types, lookupsig)
19281923
const_call_result = abstract_call_method_with_const_args(interp,
19291924
result, f, arginfo, si, match, sv, invokecall)
@@ -1934,7 +1929,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
19341929
end
19351930
end
19361931
rt = from_interprocedural!(interp, rt, sv, arginfo, sig)
1937-
effects = Effects(effects; nonoverlayed=!overlayed)
1932+
effects = Effects(effects; nonoverlayed = !overlayed)
19381933
info = InvokeCallInfo(match, const_result)
19391934
edge !== nothing && add_invoke_backedge!(sv, lookupsig, edge)
19401935
return CallMeta(rt, effects, info)

base/compiler/methodtable.jl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ end
167167
# This query is not cached
168168
findsup(@nospecialize(sig::Type), table::CachedMethodTable) = findsup(sig, table.table)
169169

170-
isoverlayed(::MethodTableView) = error("unsatisfied MethodTableView interface")
170+
isoverlayed(::MethodTableView) = error("unsatisfied MethodTableView interface")
171171
isoverlayed(::InternalMethodTable) = false
172-
isoverlayed(::OverlayMethodTable) = true
172+
isoverlayed(::OverlayMethodTable) = true
173173
isoverlayed(mt::CachedMethodTable) = isoverlayed(mt.table)
174+
isoverlayed(m::Method) = isdefined(m, :external_mt)
175+
is_nonoverlayed(m::Method) = !isoverlayed(m)

base/compiler/ssair/irinterp.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ function concrete_eval_invoke(interp::AbstractInterpreter,
1414
argtypes = collect_argtypes(interp, inst.args[2:end], nothing, irsv)
1515
argtypes === nothing && return Pair{Any,Bool}(Bottom, false)
1616
effects = decode_effects(code.ipo_purity_bits)
17-
if is_foldable(effects) && is_all_const_arg(argtypes, #=start=#1)
17+
if (is_foldable(effects) && is_all_const_arg(argtypes, #=start=#1) &&
18+
is_nonoverlayed(effects) && is_nonoverlayed(mi.def::Method))
1819
args = collect_const_args(argtypes, #=start=#1)
1920
value = let world = get_world_counter(interp)
2021
try

test/compiler/AbstractInterpreter.jl

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ include("newinterp.jl")
99
# OverlayMethodTable
1010
# ==================
1111

12-
import Base.Experimental: @MethodTable, @overlay
12+
using Base.Experimental: @MethodTable, @overlay
1313

1414
@newinterp MTOverlayInterp
15-
@MethodTable(OverlayedMT)
15+
@MethodTable OverlayedMT
1616
CC.method_table(interp::MTOverlayInterp) = CC.OverlayMethodTable(CC.get_world_counter(interp), OverlayedMT)
1717

1818
function CC.add_remark!(interp::MTOverlayInterp, ::CC.InferenceState, remark)
@@ -113,12 +113,50 @@ end |> only === Nothing
113113
@MethodTable Issue48097MT
114114
CC.method_table(interp::Issue48097Interp) = CC.OverlayMethodTable(CC.get_world_counter(interp), Issue48097MT)
115115
CC.InferenceParams(::Issue48097Interp) = CC.InferenceParams(; unoptimize_throw_blocks=false)
116+
function CC.concrete_eval_eligible(interp::Issue48097Interp,
117+
@nospecialize(f), result::CC.MethodCallResult, arginfo::CC.ArgInfo, sv::CC.AbsIntState)
118+
ret = @invoke CC.concrete_eval_eligible(interp::CC.AbstractInterpreter,
119+
f::Any, result::CC.MethodCallResult, arginfo::CC.ArgInfo, sv::CC.AbsIntState)
120+
if ret === :semi_concrete_eval
121+
# disable semi-concrete interpretation
122+
return :none
123+
end
124+
return ret
125+
end
116126
@overlay Issue48097MT @noinline Core.throw_inexacterror(f::Symbol, ::Type{T}, val) where {T} = return
117127
issue48097(; kwargs...) = return 42
118128
@test fully_eliminated(; interp=Issue48097Interp(), retval=42) do
119129
issue48097(; a=1f0, b=1.0)
120130
end
121131

132+
# Should not concrete-eval overlayed methods in semi-concrete interpretation
133+
@newinterp OverlaySinInterp
134+
@MethodTable OverlaySinMT
135+
CC.method_table(interp::OverlaySinInterp) = CC.OverlayMethodTable(CC.get_world_counter(interp), OverlaySinMT)
136+
overlay_sin1(x) = error("Not supposed to be called.")
137+
@overlay OverlaySinMT overlay_sin1(x) = cos(x)
138+
@overlay OverlaySinMT Base.sin(x::Union{Float32,Float64}) = overlay_sin1(x)
139+
let oc = Base.code_ircode(; interp=OverlaySinInterp()) do
140+
sin(0.)
141+
end |> only |> first |> Core.OpaqueClosure
142+
@test oc() == cos(0.)
143+
end
144+
@overlay OverlaySinMT Base.sin(x::Union{Float32,Float64}) = @noinline overlay_sin1(x)
145+
let oc = Base.code_ircode(; interp=OverlaySinInterp()) do
146+
sin(0.)
147+
end |> only |> first |> Core.OpaqueClosure
148+
@test oc() == cos(0.)
149+
end
150+
_overlay_sin2(x) = error("Not supposed to be called.")
151+
@overlay OverlaySinMT _overlay_sin2(x) = cos(x)
152+
overlay_sin2(x) = _overlay_sin2(x)
153+
@overlay OverlaySinMT Base.sin(x::Union{Float32,Float64}) = @noinline overlay_sin2(x)
154+
let oc = Base.code_ircode(; interp=OverlaySinInterp()) do
155+
sin(0.)
156+
end |> only |> first |> Core.OpaqueClosure
157+
@test oc() == cos(0.)
158+
end
159+
122160
# AbstractLattice
123161
# ===============
124162

0 commit comments

Comments
 (0)