From e6d9514c63f76d8a8fd494056ebc5500946275a7 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Tue, 15 Nov 2022 11:32:16 +0900 Subject: [PATCH] lattice: fix minor lattice issues I found some lattice issues when implementing `MustAlias` under the new extendable lattice system in another PR. --- base/compiler/abstractinterpretation.jl | 195 +++++++++++++++--------- base/compiler/inferencestate.jl | 6 +- 2 files changed, 130 insertions(+), 71 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 653e1168ea145..c676c967309da 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -2537,7 +2613,7 @@ 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, @@ -2545,7 +2621,7 @@ function update_bbstate!(lattice::AbstractLattice, frame::InferenceState, bb::In frame.bb_vartables[bb] = copy(vartable) return true else - return stupdate!(lattice, bbtable, vartable) + return stupdate!(𝕃ᵢ, bbtable, vartable) end end @@ -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 @@ -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) @@ -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 @@ -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 @@ -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 @@ -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] @@ -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 @@ -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 @@ -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 @@ -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) diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl index 53b7c635797a2..49315fd67834c 100644 --- a/base/compiler/inferencestate.jl +++ b/base/compiler/inferencestate.jl @@ -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)