Skip to content

Commit

Permalink
lattice: fix minor lattice issues (#47570)
Browse files Browse the repository at this point in the history
I found some lattice issues when implementing `MustAlias` under the new
extendable lattice system in another PR.
  • Loading branch information
aviatesk committed Nov 16, 2022
1 parent e805a18 commit ee0f3fc
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 71 deletions.
195 changes: 127 additions & 68 deletions base/compiler/abstractinterpretation.jl
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
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

0 comments on commit ee0f3fc

Please sign in to comment.