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.

Sign up for GitHub

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

Jump to bottom

lattice: fix minor lattice issues #47570

Merged
merged 1 commit into from
Nov 16, 2022
Merged

lattice: fix minor lattice issues #47570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
195 changes: 127 additions & 68 deletions base/compiler/abstractinterpretation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ end
function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
arginfo::ArgInfo, si::StmtInfo, @nospecialize(atype),
sv::InferenceState, max_methods::Int)
⊑ᵢ = ⊑(typeinf_lattice(interp))
⊑ₚ = ⊑(ipo_lattice(interp))
if !should_infer_this_call(sv)
add_remark!(interp, sv, "Skipped call in throw block")
nonoverlayed = false
Expand Down Expand Up @@ -134,7 +134,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
result, f, this_arginfo, si, match, sv)
const_result = nothing
if const_call_result !== nothing
if const_call_result.rt ⊑ᵢ rt
if const_call_result.rt ⊑ₚ rt
rt = const_call_result.rt
(; effects, const_result, edge) = const_call_result
end
Expand Down Expand Up @@ -167,7 +167,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
this_const_rt = widenwrappedconditional(const_call_result.rt)
# return type of const-prop' inference can be wider than that of non const-prop' inference
# e.g. in cases when there are cycles but cached result is still accurate
if this_const_rt ⊑ᵢ this_rt
if this_const_rt ⊑ₚ this_rt
this_conditional = this_const_conditional
this_rt = this_const_rt
(; effects, const_result, edge) = const_call_result
Expand Down Expand Up @@ -2399,19 +2399,52 @@ function abstract_eval_ssavalue(s::SSAValue, ssavaluetypes::Vector{Any})
return typ
end

function widenreturn(ipo_lattice::AbstractLattice, @nospecialize(rt), @nospecialize(bestguess), nargs::Int, slottypes::Vector{Any}, changes::VarTable)
⊑ₚ = ⊑(ipo_lattice)
inner_lattice = widenlattice(ipo_lattice)
⊑ᵢ = ⊑(inner_lattice)
if !(bestguess ⊑ₚ Bool) || bestguess === Bool
struct BestguessInfo{Interp<:AbstractInterpreter}
interp::Interp
bestguess
nargs::Int
slottypes::Vector{Any}
changes::VarTable
function BestguessInfo(interp::Interp, @nospecialize(bestguess), nargs::Int,
slottypes::Vector{Any}, changes::VarTable) where Interp<:AbstractInterpreter
new{Interp}(interp, bestguess, nargs, slottypes, changes)
end
end

"""
widenreturn(@nospecialize(rt), info::BestguessInfo) -> new_bestguess

Appropriately converts inferred type of a return value `rt` to such a type
that we know we can store in the cache and is valid and good inter-procedurally,
E.g. if `rt isa Conditional` then `rt` should be converted to `InterConditional`
or the other cachable lattice element.

External lattice `𝕃ₑ::ExternalLattice` may overload:
- `widenreturn(𝕃ₑ::ExternalLattice, @nospecialize(rt), info::BestguessInfo)`
- `widenreturn_noslotwrapper(𝕃ₑ::ExternalLattice, @nospecialize(rt), info::BestguessInfo)`
"""
function widenreturn(@nospecialize(rt), info::BestguessInfo)
return widenreturn(typeinf_lattice(info.interp), rt, info)
end

function widenreturn(𝕃ᵢ::AbstractLattice, @nospecialize(rt), info::BestguessInfo)
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
end
function widenreturn_noslotwrapper(𝕃ᵢ::AbstractLattice, @nospecialize(rt), info::BestguessInfo)
return widenreturn_noslotwrapper(widenlattice(𝕃ᵢ), rt, info)
end

function widenreturn(𝕃ᵢ::ConditionalsLattice, @nospecialize(rt), info::BestguessInfo)
⊑ᵢ = ⊑(𝕃ᵢ)
if !(⊑(ipo_lattice(info.interp), info.bestguess, Bool)) || info.bestguess === Bool
# give up inter-procedural constraint back-propagation
# when tmerge would widen the result anyways (as an optimization)
rt = widenconditional(rt)
else
if isa(rt, Conditional)
id = rt.slot
if 1 ≤ id ≤ nargs
old_id_type = widenconditional(slottypes[id]) # same as `(states[1]::VarTable)[id].typ`
if 1 ≤ id ≤ info.nargs
old_id_type = widenconditional(info.slottypes[id]) # same as `(states[1]::VarTable)[id].typ`
if (!(rt.thentype ⊑ᵢ old_id_type) || old_id_type ⊑ᵢ rt.thentype) &&
(!(rt.elsetype ⊑ᵢ old_id_type) || old_id_type ⊑ᵢ rt.elsetype)
# discard this `Conditional` since it imposes
Expand All @@ -2428,44 +2461,69 @@ function widenreturn(ipo_lattice::AbstractLattice, @nospecialize(rt), @nospecial
end
if isa(rt, Conditional)
rt = InterConditional(rt.slot, rt.thentype, rt.elsetype)
elseif is_lattice_bool(ipo_lattice, rt)
if isa(bestguess, InterConditional)
# if the bestguess so far is already `Conditional`, try to convert
# this `rt` into `Conditional` on the slot to avoid overapproximation
# due to conflict of different slots
rt = bool_rt_to_conditional(rt, slottypes, changes, bestguess.slot)
else
# pick up the first "interesting" slot, convert `rt` to its `Conditional`
# TODO: ideally we want `Conditional` and `InterConditional` to convey
# constraints on multiple slots
for slot_id in 1:nargs
rt = bool_rt_to_conditional(rt, slottypes, changes, slot_id)
rt isa InterConditional && break
end
end
elseif is_lattice_bool(𝕃ᵢ, rt)
rt = bool_rt_to_conditional(rt, info)
end
end

# only propagate information we know we can store
# and is valid and good inter-procedurally
isa(rt, Conditional) && return InterConditional(rt)
isa(rt, InterConditional) && return rt
return widenreturn_noconditional(widenlattice(ipo_lattice), rt)
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
end
function bool_rt_to_conditional(@nospecialize(rt), info::BestguessInfo)
bestguess = info.bestguess
if isa(bestguess, InterConditional)
# if the bestguess so far is already `Conditional`, try to convert
# this `rt` into `Conditional` on the slot to avoid overapproximation
# due to conflict of different slots
rt = bool_rt_to_conditional(rt, bestguess.slot, info)
else
# pick up the first "interesting" slot, convert `rt` to its `Conditional`
# TODO: ideally we want `Conditional` and `InterConditional` to convey
# constraints on multiple slots
for slot_id = 1:info.nargs
rt = bool_rt_to_conditional(rt, slot_id, info)
rt isa InterConditional && break
end
end
return rt
end
function bool_rt_to_conditional(@nospecialize(rt), slot_id::Int, info::BestguessInfo)
⊑ᵢ = ⊑(typeinf_lattice(info.interp))
old = info.slottypes[slot_id]
new = widenconditional(info.changes[slot_id].typ) # avoid nested conditional
if new ⊑ᵢ old && !(old ⊑ᵢ new)
if isa(rt, Const)
val = rt.val
if val === true
return InterConditional(slot_id, new, Bottom)
elseif val === false
return InterConditional(slot_id, Bottom, new)
end
elseif rt === Bool
return InterConditional(slot_id, new, new)
end
end
return rt
end

function widenreturn_noconditional(inner_lattice::AbstractLattice, @nospecialize(rt))
isa(rt, Const) && return rt
isa(rt, Type) && return rt
function widenreturn(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
return widenreturn_partials(𝕃ᵢ, rt, info)
end
function widenreturn_noslotwrapper(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
return widenreturn_partials(𝕃ᵢ, rt, info)
end
function widenreturn_partials(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
if isa(rt, PartialStruct)
fields = copy(rt.fields)
local anyrefine = false
𝕃 = typeinf_lattice(info.interp)
for i in 1:length(fields)
a = fields[i]
a = isvarargtype(a) ? a : widenreturn_noconditional(inner_lattice, a)
a = isvarargtype(a) ? a : widenreturn_noslotwrapper(𝕃, a, info)
if !anyrefine
# TODO: consider adding && const_prop_profitable(a) here?
anyrefine = has_const_info(a) ||
⊏(inner_lattice, a, fieldtype(rt.typ, i))
⊏(𝕃, a, fieldtype(rt.typ, i))
end
fields[i] = a
end
Expand All @@ -2474,6 +2532,24 @@ function widenreturn_noconditional(inner_lattice::AbstractLattice, @nospecialize
if isa(rt, PartialOpaque)
return rt # XXX: this case was missed in #39512
end
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
end

function widenreturn(::ConstsLattice, @nospecialize(rt), ::BestguessInfo)
return widenreturn_consts(rt)
end
function widenreturn_noslotwrapper(::ConstsLattice, @nospecialize(rt), ::BestguessInfo)
return widenreturn_consts(rt)
end
function widenreturn_consts(@nospecialize(rt))
isa(rt, Const) && return rt
return widenconst(rt)
end

function widenreturn(::JLTypeLattice, @nospecialize(rt), ::BestguessInfo)
return widenconst(rt)
end
function widenreturn_noslotwrapper(::JLTypeLattice, @nospecialize(rt), ::BestguessInfo)
return widenconst(rt)
end

Expand Down Expand Up @@ -2537,15 +2613,15 @@ end
end
end

function update_bbstate!(lattice::AbstractLattice, frame::InferenceState, bb::Int, vartable::VarTable)
function update_bbstate!(𝕃ᵢ::AbstractLattice, frame::InferenceState, bb::Int, vartable::VarTable)
bbtable = frame.bb_vartables[bb]
if bbtable === nothing
# if a basic block hasn't been analyzed yet,
# we can update its state a bit more aggressively
frame.bb_vartables[bb] = copy(vartable)
return true
else
return stupdate!(lattice, bbtable, vartable)
return stupdate!(𝕃ᵢ, bbtable, vartable)
end
end

Expand All @@ -2567,6 +2643,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
ssavaluetypes = frame.ssavaluetypes
bbs = frame.cfg.blocks
nbbs = length(bbs)
𝕃ₚ, 𝕃ᵢ = ipo_lattice(interp), typeinf_lattice(interp)

currbb = frame.currbb
if currbb != 1
Expand Down Expand Up @@ -2636,19 +2713,19 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
# We continue with the true branch, but process the false
# branch here.
if isa(condt, Conditional)
else_change = conditional_change(currstate, condt.elsetype, condt.slot)
else_change = conditional_change(𝕃ᵢ, currstate, condt.elsetype, condt.slot)
if else_change !== nothing
false_vartable = stoverwrite1!(copy(currstate), else_change)
else
false_vartable = currstate
end
changed = update_bbstate!(typeinf_lattice(interp), frame, falsebb, false_vartable)
then_change = conditional_change(currstate, condt.thentype, condt.slot)
changed = update_bbstate!(𝕃ᵢ, frame, falsebb, false_vartable)
then_change = conditional_change(𝕃ᵢ, currstate, condt.thentype, condt.slot)
if then_change !== nothing
stoverwrite1!(currstate, then_change)
end
else
changed = update_bbstate!(typeinf_lattice(interp), frame, falsebb, currstate)
changed = update_bbstate!(𝕃ᵢ, frame, falsebb, currstate)
end
if changed
handle_control_backedge!(interp, frame, currpc, stmt.dest)
Expand All @@ -2660,7 +2737,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
elseif isa(stmt, ReturnNode)
bestguess = frame.bestguess
rt = abstract_eval_value(interp, stmt.val, currstate, frame)
rt = widenreturn(ipo_lattice(interp), rt, bestguess, nargs, slottypes, currstate)
rt = widenreturn(rt, BestguessInfo(interp, bestguess, nargs, slottypes, currstate))
# narrow representation of bestguess slightly to prepare for tmerge with rt
if rt isa InterConditional && bestguess isa Const
let slot_id = rt.slot
Expand All @@ -2680,9 +2757,9 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
if !isempty(frame.limitations)
rt = LimitedAccuracy(rt, copy(frame.limitations))
end
if tchanged(ipo_lattice(interp), rt, bestguess)
if tchanged(𝕃ₚ, rt, bestguess)
# new (wider) return type for frame
bestguess = tmerge(ipo_lattice(interp), bestguess, rt)
bestguess = tmerge(𝕃ₚ, bestguess, rt)
# TODO: if bestguess isa InterConditional && !interesting(bestguess); bestguess = widenconditional(bestguess); end
frame.bestguess = bestguess
for (caller, caller_pc) in frame.cycle_backedges
Expand All @@ -2698,7 +2775,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
# Propagate entry info to exception handler
l = stmt.args[1]::Int
catchbb = block_for_inst(frame.cfg, l)
if update_bbstate!(typeinf_lattice(interp), frame, catchbb, currstate)
if update_bbstate!(𝕃ᵢ, frame, catchbb, currstate)
push!(W, catchbb)
end
ssavaluetypes[currpc] = Any
Expand All @@ -2723,7 +2800,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
# propagate new type info to exception handler
# the handling for Expr(:enter) propagates all changes from before the try/catch
# so this only needs to propagate any changes
if stupdate1!(typeinf_lattice(interp), states[exceptbb]::VarTable, changes)
if stupdate1!(𝕃ᵢ, states[exceptbb]::VarTable, changes)
push!(W, exceptbb)
end
cur_hand = frame.handler_at[cur_hand]
Expand All @@ -2735,7 +2812,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
continue
end
if !isempty(frame.ssavalue_uses[currpc])
record_ssa_assign!(currpc, type, frame)
record_ssa_assign!(𝕃ᵢ, currpc, type, frame)
else
ssavaluetypes[currpc] = type
end
Expand All @@ -2748,7 +2825,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)

# Case 2: Directly branch to a different BB
begin @label branch
if update_bbstate!(typeinf_lattice(interp), frame, nextbb, currstate)
if update_bbstate!(𝕃ᵢ, frame, nextbb, currstate)
push!(W, nextbb)
end
end
Expand All @@ -2772,13 +2849,13 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
nothing
end

function conditional_change(state::VarTable, @nospecialize(typ), slot::Int)
function conditional_change(𝕃ᵢ::AbstractLattice, state::VarTable, @nospecialize(typ), slot::Int)
vtype = state[slot]
oldtyp = vtype.typ
if iskindtype(typ)
# this code path corresponds to the special handling for `isa(x, iskindtype)` check
# implemented within `abstract_call_builtin`
elseif ignorelimited(typ) ⊑ ignorelimited(oldtyp)
elseif ⊑(𝕃ᵢ, ignorelimited(typ), ignorelimited(oldtyp))
# approximate test for `typ ∩ oldtyp` being better than `oldtyp`
# since we probably formed these types with `typesubstract`,
# the comparison is likely simple
Expand All @@ -2788,29 +2865,11 @@ function conditional_change(state::VarTable, @nospecialize(typ), slot::Int)
if oldtyp isa LimitedAccuracy
# typ is better unlimited, but we may still need to compute the tmeet with the limit
# "causes" since we ignored those in the comparison
typ = tmerge(typ, LimitedAccuracy(Bottom, oldtyp.causes))
typ = tmerge(𝕃ᵢ, typ, LimitedAccuracy(Bottom, oldtyp.causes))
end
return StateUpdate(SlotNumber(slot), VarState(typ, vtype.undef), state, true)
end

function bool_rt_to_conditional(@nospecialize(rt), slottypes::Vector{Any}, state::VarTable, slot_id::Int)
old = slottypes[slot_id]
new = widenconditional(state[slot_id].typ) # avoid nested conditional
if new ⊑ old && !(old ⊑ new)
if isa(rt, Const)
val = rt.val
if val === true
return InterConditional(slot_id, new, Bottom)
elseif val === false
return InterConditional(slot_id, Bottom, new)
end
elseif rt === Bool
return InterConditional(slot_id, new, new)
end
end
return rt
end

# make as much progress on `frame` as possible (by handling cycles)
function typeinf_nocycle(interp::AbstractInterpreter, frame::InferenceState)
typeinf_local(interp, frame)
Expand Down
6 changes: 3 additions & 3 deletions base/compiler/inferencestate.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,14 @@ end

update_valid_age!(edge::InferenceState, sv::InferenceState) = update_valid_age!(sv, edge.valid_worlds)

function record_ssa_assign!(ssa_id::Int, @nospecialize(new), frame::InferenceState)
function record_ssa_assign!(𝕃ᵢ::AbstractLattice, ssa_id::Int, @nospecialize(new), frame::InferenceState)
ssavaluetypes = frame.ssavaluetypes
old = ssavaluetypes[ssa_id]
if old === NOT_FOUND || !(new ⊑ old)
if old === NOT_FOUND || !⊑(𝕃ᵢ, new, old)
# typically, we expect that old ⊑ new (that output information only
# gets less precise with worse input information), but to actually
# guarantee convergence we need to use tmerge here to ensure that is true
ssavaluetypes[ssa_id] = old === NOT_FOUND ? new : tmerge(old, new)
ssavaluetypes[ssa_id] = old === NOT_FOUND ? new : tmerge(𝕃ᵢ, old, new)
W = frame.ip
for r in frame.ssavalue_uses[ssa_id]
if was_reached(frame, r)
Expand Down