Skip to content

Commit

Permalink
effects: improve idempotency of effects derived by post-opt analysis (#…
Browse files Browse the repository at this point in the history
…52085)

Since now effects can be refined by post-opt analysis, `typeinf_edge`
should propagate `frame.result.ipo_effects` instead of
`frame.ipo_effects`.
  • Loading branch information
aviatesk authored Nov 9, 2023
2 parents 137783f + 358540c commit 529e4e7
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 23 deletions.
9 changes: 4 additions & 5 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1393,8 +1393,7 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt32, sig
cases = InliningCase[]
argtypes = sig.argtypes
local handled_all_cases::Bool = true
local revisit_idx = nothing
local only_method = nothing
local revisit_idx = local only_method = nothing
local meth::MethodLookupResult
local all_result_count = 0
local joint_effects::Effects = EFFECTS_TOTAL
Expand All @@ -1410,14 +1409,14 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt32, sig
handled_all_cases = false
continue
else
if length(meth) == 1 && only_method !== false
if length(meth) == 1 && only_method !== missing
if only_method === nothing
only_method = meth[1].method
elseif only_method !== meth[1].method
only_method = false
only_method = missing
end
else
only_method = false
only_method = missing
end
end
local split_fully_covered::Bool = false
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ function finish(me::InferenceState, interp::AbstractInterpreter)
end
me.result.valid_worlds = me.valid_worlds
me.result.result = bestguess
me.ipo_effects = me.result.ipo_effects = adjust_effects(me)
me.result.ipo_effects = me.ipo_effects = adjust_effects(me)

if limited_ret
# a parent may be cached still, but not this intermediate work:
Expand Down Expand Up @@ -856,7 +856,7 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
update_valid_age!(caller, frame.valid_worlds)
isinferred = is_inferred(frame)
edge = isinferred ? mi : nothing
effects = isinferred ? frame.ipo_effects : adjust_effects(Effects(), method) # effects are adjusted already within `finish` for ipo_effects
effects = isinferred ? frame.result.ipo_effects : adjust_effects(Effects(), method) # effects are adjusted already within `finish` for ipo_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 && let inferred_src = result.src
Expand Down
14 changes: 14 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1158,3 +1158,17 @@ end
issue51837(; openquotechar, newlinechar)
end |> !Core.Compiler.is_nothrow
@test_throws ArgumentError issue51837(; openquotechar='α', newlinechar='\n')

# idempotency of effects derived by post-opt analysis
callgetfield(x, f) = getfield(x, f, Base.@_boundscheck)
@test Base.infer_effects(callgetfield, (Some{Any},Symbol)).noub === Core.Compiler.NOUB_IF_NOINBOUNDS
callgetfield1(x, f) = getfield(x, f, Base.@_boundscheck)
callgetfield_simple(x, f) = callgetfield1(x, f)
@test Base.infer_effects(callgetfield_simple, (Some{Any},Symbol)).noub ===
Base.infer_effects(callgetfield_simple, (Some{Any},Symbol)).noub ===
Core.Compiler.ALWAYS_TRUE
callgetfield2(x, f) = getfield(x, f, Base.@_boundscheck)
callgetfield_inbounds(x, f) = @inbounds callgetfield2(x, f)
@test Base.infer_effects(callgetfield_inbounds, (Some{Any},Symbol)).noub ===
Base.infer_effects(callgetfield_inbounds, (Some{Any},Symbol)).noub ===
Core.Compiler.ALWAYS_FALSE
5 changes: 3 additions & 2 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ end
Core.Compiler.limit_type_size(Type{Any}, Type, Union{}, 0, 0) ==
Type{Any}

# issue #43296 #43296
# issue #43296
struct C43296{t,I} end
r43296(b) = r43296(typeof(b))
r43296(::Type) = nothing
Expand All @@ -149,7 +149,8 @@ f43296(g, :) = h
k43296(b, j, :) = l
k43296(b, j, ::Nothing) = b
i43296(b, j) = k43296(b, j, r43296(j))
@test only(Base.return_types(i43296, (Int, C43296{C43296{C43296{Val, Tuple}, Tuple}}))) == Int
@test only(Base.return_types(i43296, (Int, C43296{C43296{C43296{Val, Tuple}}}))) <: Int
@test only(Base.return_types(i43296, (Int, C43296{C43296{C43296{Val, <:Tuple}}}))) <: Int

abstract type e43296{a, j} <: AbstractArray{a, j} end
abstract type b43296{a, j, c, d} <: e43296{a, j} end
Expand Down
25 changes: 11 additions & 14 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1675,31 +1675,28 @@ function oc_capture_oc(z)
end
@test fully_eliminated(oc_capture_oc, (Int,))

# inlining with unmatched type parameters
@eval struct OldVal{T}
x::T
(OV::Type{OldVal{T}})() where T = $(Expr(:new, :OV))
end
with_unmatched_typeparam1(x::OldVal{i}) where {i} = i
with_unmatched_typeparam2() = [ Base.donotdelete(OldVal{i}()) for i in 1:10000 ]
function with_unmatched_typeparam3()
@test OldVal{0}() === OldVal{0}.instance
function with_unmatched_typeparam()
f(x::OldVal{i}) where {i} = i
r = 0
for i = 1:10000
r += f(OldVal{i}())
end
return r
end

@testset "Inlining with unmatched type parameters" begin
let src = code_typed1(with_unmatched_typeparam1, (Any,))
@test !any(@nospecialize(x) -> isexpr(x, :call) && length(x.args) == 1, src.code)
end
let src = code_typed1(with_unmatched_typeparam2)
@test !any(@nospecialize(x) -> isexpr(x, :call) && length(x.args) == 1, src.code)
end
let src = code_typed1(with_unmatched_typeparam3)
@test !any(@nospecialize(x) -> isexpr(x, :call) && length(x.args) == 1, src.code)
let src = code_typed1(with_unmatched_typeparam)
found = nothing
for x in src.code
if isexpr(x, :call) && length(x.args) == 1
found = x
break
end
end
@test isnothing(found) || (source=src, statement=found)
end

function twice_sitofp(x::Int, y::Int)
Expand Down

0 comments on commit 529e4e7

Please sign in to comment.