Skip to content

Commit 5e39fc6

Browse files
aviateskKristofferC
authored andcommitted
lattice: fix minor lattice issues (#47570)
I found some lattice issues when implementing `MustAlias` under the new extendable lattice system in another PR. (cherry picked from commit ee0f3fc)
1 parent f7c4f59 commit 5e39fc6

File tree

2 files changed

+130
-71
lines changed

2 files changed

+130
-71
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 127 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ end
5353
function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
5454
arginfo::ArgInfo, si::StmtInfo, @nospecialize(atype),
5555
sv::InferenceState, max_methods::Int)
56-
= (typeinf_lattice(interp))
56+
= (ipo_lattice(interp))
5757
if !should_infer_this_call(sv)
5858
add_remark!(interp, sv, "Skipped call in throw block")
5959
nonoverlayed = false
@@ -133,7 +133,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
133133
result, f, this_arginfo, si, match, sv)
134134
const_result = nothing
135135
if const_call_result !== nothing
136-
if const_call_result.rt rt
136+
if const_call_result.rt rt
137137
rt = const_call_result.rt
138138
(; effects, const_result, edge) = const_call_result
139139
end
@@ -166,7 +166,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
166166
this_const_rt = widenwrappedconditional(const_call_result.rt)
167167
# return type of const-prop' inference can be wider than that of non const-prop' inference
168168
# e.g. in cases when there are cycles but cached result is still accurate
169-
if this_const_rt this_rt
169+
if this_const_rt this_rt
170170
this_conditional = this_const_conditional
171171
this_rt = this_const_rt
172172
(; effects, const_result, edge) = const_call_result
@@ -2422,19 +2422,52 @@ function abstract_eval_ssavalue(s::SSAValue, ssavaluetypes::Vector{Any})
24222422
return typ
24232423
end
24242424

2425-
function widenreturn(ipo_lattice::AbstractLattice, @nospecialize(rt), @nospecialize(bestguess), nargs::Int, slottypes::Vector{Any}, changes::VarTable)
2426-
= (ipo_lattice)
2427-
inner_lattice = widenlattice(ipo_lattice)
2428-
= (inner_lattice)
2429-
if !(bestguess ₚ Bool) || bestguess === Bool
2425+
struct BestguessInfo{Interp<:AbstractInterpreter}
2426+
interp::Interp
2427+
bestguess
2428+
nargs::Int
2429+
slottypes::Vector{Any}
2430+
changes::VarTable
2431+
function BestguessInfo(interp::Interp, @nospecialize(bestguess), nargs::Int,
2432+
slottypes::Vector{Any}, changes::VarTable) where Interp<:AbstractInterpreter
2433+
new{Interp}(interp, bestguess, nargs, slottypes, changes)
2434+
end
2435+
end
2436+
2437+
"""
2438+
widenreturn(@nospecialize(rt), info::BestguessInfo) -> new_bestguess
2439+
2440+
Appropriately converts inferred type of a return value `rt` to such a type
2441+
that we know we can store in the cache and is valid and good inter-procedurally,
2442+
E.g. if `rt isa Conditional` then `rt` should be converted to `InterConditional`
2443+
or the other cachable lattice element.
2444+
2445+
External lattice `𝕃ₑ::ExternalLattice` may overload:
2446+
- `widenreturn(𝕃ₑ::ExternalLattice, @nospecialize(rt), info::BestguessInfo)`
2447+
- `widenreturn_noslotwrapper(𝕃ₑ::ExternalLattice, @nospecialize(rt), info::BestguessInfo)`
2448+
"""
2449+
function widenreturn(@nospecialize(rt), info::BestguessInfo)
2450+
return widenreturn(typeinf_lattice(info.interp), rt, info)
2451+
end
2452+
2453+
function widenreturn(𝕃ᵢ::AbstractLattice, @nospecialize(rt), info::BestguessInfo)
2454+
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
2455+
end
2456+
function widenreturn_noslotwrapper(𝕃ᵢ::AbstractLattice, @nospecialize(rt), info::BestguessInfo)
2457+
return widenreturn_noslotwrapper(widenlattice(𝕃ᵢ), rt, info)
2458+
end
2459+
2460+
function widenreturn(𝕃ᵢ::ConditionalsLattice, @nospecialize(rt), info::BestguessInfo)
2461+
= (𝕃ᵢ)
2462+
if !((ipo_lattice(info.interp), info.bestguess, Bool)) || info.bestguess === Bool
24302463
# give up inter-procedural constraint back-propagation
24312464
# when tmerge would widen the result anyways (as an optimization)
24322465
rt = widenconditional(rt)
24332466
else
24342467
if isa(rt, Conditional)
24352468
id = rt.slot
2436-
if 1 id nargs
2437-
old_id_type = widenconditional(slottypes[id]) # same as `(states[1]::VarTable)[id].typ`
2469+
if 1 id info.nargs
2470+
old_id_type = widenconditional(info.slottypes[id]) # same as `(states[1]::VarTable)[id].typ`
24382471
if (!(rt.thentype ᵢ old_id_type) || old_id_type ᵢ rt.thentype) &&
24392472
(!(rt.elsetype ᵢ old_id_type) || old_id_type ᵢ rt.elsetype)
24402473
# discard this `Conditional` since it imposes
@@ -2451,44 +2484,69 @@ function widenreturn(ipo_lattice::AbstractLattice, @nospecialize(rt), @nospecial
24512484
end
24522485
if isa(rt, Conditional)
24532486
rt = InterConditional(rt.slot, rt.thentype, rt.elsetype)
2454-
elseif is_lattice_bool(ipo_lattice, rt)
2455-
if isa(bestguess, InterConditional)
2456-
# if the bestguess so far is already `Conditional`, try to convert
2457-
# this `rt` into `Conditional` on the slot to avoid overapproximation
2458-
# due to conflict of different slots
2459-
rt = bool_rt_to_conditional(rt, slottypes, changes, bestguess.slot)
2460-
else
2461-
# pick up the first "interesting" slot, convert `rt` to its `Conditional`
2462-
# TODO: ideally we want `Conditional` and `InterConditional` to convey
2463-
# constraints on multiple slots
2464-
for slot_id in 1:nargs
2465-
rt = bool_rt_to_conditional(rt, slottypes, changes, slot_id)
2466-
rt isa InterConditional && break
2467-
end
2468-
end
2487+
elseif is_lattice_bool(𝕃ᵢ, rt)
2488+
rt = bool_rt_to_conditional(rt, info)
24692489
end
24702490
end
2471-
2472-
# only propagate information we know we can store
2473-
# and is valid and good inter-procedurally
24742491
isa(rt, Conditional) && return InterConditional(rt)
24752492
isa(rt, InterConditional) && return rt
2476-
return widenreturn_noconditional(widenlattice(ipo_lattice), rt)
2493+
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
2494+
end
2495+
function bool_rt_to_conditional(@nospecialize(rt), info::BestguessInfo)
2496+
bestguess = info.bestguess
2497+
if isa(bestguess, InterConditional)
2498+
# if the bestguess so far is already `Conditional`, try to convert
2499+
# this `rt` into `Conditional` on the slot to avoid overapproximation
2500+
# due to conflict of different slots
2501+
rt = bool_rt_to_conditional(rt, bestguess.slot, info)
2502+
else
2503+
# pick up the first "interesting" slot, convert `rt` to its `Conditional`
2504+
# TODO: ideally we want `Conditional` and `InterConditional` to convey
2505+
# constraints on multiple slots
2506+
for slot_id = 1:info.nargs
2507+
rt = bool_rt_to_conditional(rt, slot_id, info)
2508+
rt isa InterConditional && break
2509+
end
2510+
end
2511+
return rt
2512+
end
2513+
function bool_rt_to_conditional(@nospecialize(rt), slot_id::Int, info::BestguessInfo)
2514+
= (typeinf_lattice(info.interp))
2515+
old = info.slottypes[slot_id]
2516+
new = widenconditional(info.changes[slot_id].typ) # avoid nested conditional
2517+
if new ᵢ old && !(old ᵢ new)
2518+
if isa(rt, Const)
2519+
val = rt.val
2520+
if val === true
2521+
return InterConditional(slot_id, new, Bottom)
2522+
elseif val === false
2523+
return InterConditional(slot_id, Bottom, new)
2524+
end
2525+
elseif rt === Bool
2526+
return InterConditional(slot_id, new, new)
2527+
end
2528+
end
2529+
return rt
24772530
end
24782531

2479-
function widenreturn_noconditional(inner_lattice::AbstractLattice, @nospecialize(rt))
2480-
isa(rt, Const) && return rt
2481-
isa(rt, Type) && return rt
2532+
function widenreturn(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
2533+
return widenreturn_partials(𝕃ᵢ, rt, info)
2534+
end
2535+
function widenreturn_noslotwrapper(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
2536+
return widenreturn_partials(𝕃ᵢ, rt, info)
2537+
end
2538+
function widenreturn_partials(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
24822539
if isa(rt, PartialStruct)
24832540
fields = copy(rt.fields)
24842541
local anyrefine = false
2542+
𝕃 = typeinf_lattice(info.interp)
24852543
for i in 1:length(fields)
24862544
a = fields[i]
2487-
a = isvarargtype(a) ? a : widenreturn_noconditional(inner_lattice, a)
2545+
a = isvarargtype(a) ? a : widenreturn_noslotwrapper(𝕃, a, info)
24882546
if !anyrefine
24892547
# TODO: consider adding && const_prop_profitable(a) here?
24902548
anyrefine = has_const_info(a) ||
2491-
(inner_lattice, a, fieldtype(rt.typ, i))
2549+
(𝕃, a, fieldtype(rt.typ, i))
24922550
end
24932551
fields[i] = a
24942552
end
@@ -2497,6 +2555,24 @@ function widenreturn_noconditional(inner_lattice::AbstractLattice, @nospecialize
24972555
if isa(rt, PartialOpaque)
24982556
return rt # XXX: this case was missed in #39512
24992557
end
2558+
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
2559+
end
2560+
2561+
function widenreturn(::ConstsLattice, @nospecialize(rt), ::BestguessInfo)
2562+
return widenreturn_consts(rt)
2563+
end
2564+
function widenreturn_noslotwrapper(::ConstsLattice, @nospecialize(rt), ::BestguessInfo)
2565+
return widenreturn_consts(rt)
2566+
end
2567+
function widenreturn_consts(@nospecialize(rt))
2568+
isa(rt, Const) && return rt
2569+
return widenconst(rt)
2570+
end
2571+
2572+
function widenreturn(::JLTypeLattice, @nospecialize(rt), ::BestguessInfo)
2573+
return widenconst(rt)
2574+
end
2575+
function widenreturn_noslotwrapper(::JLTypeLattice, @nospecialize(rt), ::BestguessInfo)
25002576
return widenconst(rt)
25012577
end
25022578

@@ -2560,15 +2636,15 @@ end
25602636
end
25612637
end
25622638

2563-
function update_bbstate!(lattice::AbstractLattice, frame::InferenceState, bb::Int, vartable::VarTable)
2639+
function update_bbstate!(𝕃ᵢ::AbstractLattice, frame::InferenceState, bb::Int, vartable::VarTable)
25642640
bbtable = frame.bb_vartables[bb]
25652641
if bbtable === nothing
25662642
# if a basic block hasn't been analyzed yet,
25672643
# we can update its state a bit more aggressively
25682644
frame.bb_vartables[bb] = copy(vartable)
25692645
return true
25702646
else
2571-
return stupdate!(lattice, bbtable, vartable)
2647+
return stupdate!(𝕃ᵢ, bbtable, vartable)
25722648
end
25732649
end
25742650

@@ -2590,6 +2666,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
25902666
ssavaluetypes = frame.ssavaluetypes
25912667
bbs = frame.cfg.blocks
25922668
nbbs = length(bbs)
2669+
𝕃ₚ, 𝕃ᵢ = ipo_lattice(interp), typeinf_lattice(interp)
25932670

25942671
currbb = frame.currbb
25952672
if currbb != 1
@@ -2659,19 +2736,19 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
26592736
# We continue with the true branch, but process the false
26602737
# branch here.
26612738
if isa(condt, Conditional)
2662-
else_change = conditional_change(currstate, condt.elsetype, condt.slot)
2739+
else_change = conditional_change(𝕃ᵢ, currstate, condt.elsetype, condt.slot)
26632740
if else_change !== nothing
26642741
false_vartable = stoverwrite1!(copy(currstate), else_change)
26652742
else
26662743
false_vartable = currstate
26672744
end
2668-
changed = update_bbstate!(typeinf_lattice(interp), frame, falsebb, false_vartable)
2669-
then_change = conditional_change(currstate, condt.thentype, condt.slot)
2745+
changed = update_bbstate!(𝕃ᵢ, frame, falsebb, false_vartable)
2746+
then_change = conditional_change(𝕃ᵢ, currstate, condt.thentype, condt.slot)
26702747
if then_change !== nothing
26712748
stoverwrite1!(currstate, then_change)
26722749
end
26732750
else
2674-
changed = update_bbstate!(typeinf_lattice(interp), frame, falsebb, currstate)
2751+
changed = update_bbstate!(𝕃ᵢ, frame, falsebb, currstate)
26752752
end
26762753
if changed
26772754
handle_control_backedge!(interp, frame, currpc, stmt.dest)
@@ -2683,7 +2760,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
26832760
elseif isa(stmt, ReturnNode)
26842761
bestguess = frame.bestguess
26852762
rt = abstract_eval_value(interp, stmt.val, currstate, frame)
2686-
rt = widenreturn(ipo_lattice(interp), rt, bestguess, nargs, slottypes, currstate)
2763+
rt = widenreturn(rt, BestguessInfo(interp, bestguess, nargs, slottypes, currstate))
26872764
# narrow representation of bestguess slightly to prepare for tmerge with rt
26882765
if rt isa InterConditional && bestguess isa Const
26892766
let slot_id = rt.slot
@@ -2703,9 +2780,9 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27032780
if !isempty(frame.limitations)
27042781
rt = LimitedAccuracy(rt, copy(frame.limitations))
27052782
end
2706-
if tchanged(ipo_lattice(interp), rt, bestguess)
2783+
if tchanged(𝕃ₚ, rt, bestguess)
27072784
# new (wider) return type for frame
2708-
bestguess = tmerge(ipo_lattice(interp), bestguess, rt)
2785+
bestguess = tmerge(𝕃ₚ, bestguess, rt)
27092786
# TODO: if bestguess isa InterConditional && !interesting(bestguess); bestguess = widenconditional(bestguess); end
27102787
frame.bestguess = bestguess
27112788
for (caller, caller_pc) in frame.cycle_backedges
@@ -2721,7 +2798,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27212798
# Propagate entry info to exception handler
27222799
l = stmt.args[1]::Int
27232800
catchbb = block_for_inst(frame.cfg, l)
2724-
if update_bbstate!(typeinf_lattice(interp), frame, catchbb, currstate)
2801+
if update_bbstate!(𝕃ᵢ, frame, catchbb, currstate)
27252802
push!(W, catchbb)
27262803
end
27272804
ssavaluetypes[currpc] = Any
@@ -2746,7 +2823,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27462823
# propagate new type info to exception handler
27472824
# the handling for Expr(:enter) propagates all changes from before the try/catch
27482825
# so this only needs to propagate any changes
2749-
if stupdate1!(typeinf_lattice(interp), states[exceptbb]::VarTable, changes)
2826+
if stupdate1!(𝕃ᵢ, states[exceptbb]::VarTable, changes)
27502827
push!(W, exceptbb)
27512828
end
27522829
cur_hand = frame.handler_at[cur_hand]
@@ -2758,7 +2835,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27582835
continue
27592836
end
27602837
if !isempty(frame.ssavalue_uses[currpc])
2761-
record_ssa_assign!(currpc, type, frame)
2838+
record_ssa_assign!(𝕃ᵢ, currpc, type, frame)
27622839
else
27632840
ssavaluetypes[currpc] = type
27642841
end
@@ -2771,7 +2848,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27712848

27722849
# Case 2: Directly branch to a different BB
27732850
begin @label branch
2774-
if update_bbstate!(typeinf_lattice(interp), frame, nextbb, currstate)
2851+
if update_bbstate!(𝕃ᵢ, frame, nextbb, currstate)
27752852
push!(W, nextbb)
27762853
end
27772854
end
@@ -2795,13 +2872,13 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27952872
nothing
27962873
end
27972874

2798-
function conditional_change(state::VarTable, @nospecialize(typ), slot::Int)
2875+
function conditional_change(𝕃ᵢ::AbstractLattice, state::VarTable, @nospecialize(typ), slot::Int)
27992876
vtype = state[slot]
28002877
oldtyp = vtype.typ
28012878
if iskindtype(typ)
28022879
# this code path corresponds to the special handling for `isa(x, iskindtype)` check
28032880
# implemented within `abstract_call_builtin`
2804-
elseif ignorelimited(typ) ignorelimited(oldtyp)
2881+
elseif (𝕃ᵢ, ignorelimited(typ), ignorelimited(oldtyp))
28052882
# approximate test for `typ ∩ oldtyp` being better than `oldtyp`
28062883
# since we probably formed these types with `typesubstract`,
28072884
# the comparison is likely simple
@@ -2811,29 +2888,11 @@ function conditional_change(state::VarTable, @nospecialize(typ), slot::Int)
28112888
if oldtyp isa LimitedAccuracy
28122889
# typ is better unlimited, but we may still need to compute the tmeet with the limit
28132890
# "causes" since we ignored those in the comparison
2814-
typ = tmerge(typ, LimitedAccuracy(Bottom, oldtyp.causes))
2891+
typ = tmerge(𝕃ᵢ, typ, LimitedAccuracy(Bottom, oldtyp.causes))
28152892
end
28162893
return StateUpdate(SlotNumber(slot), VarState(typ, vtype.undef), state, true)
28172894
end
28182895

2819-
function bool_rt_to_conditional(@nospecialize(rt), slottypes::Vector{Any}, state::VarTable, slot_id::Int)
2820-
old = slottypes[slot_id]
2821-
new = widenconditional(state[slot_id].typ) # avoid nested conditional
2822-
if new old && !(old new)
2823-
if isa(rt, Const)
2824-
val = rt.val
2825-
if val === true
2826-
return InterConditional(slot_id, new, Bottom)
2827-
elseif val === false
2828-
return InterConditional(slot_id, Bottom, new)
2829-
end
2830-
elseif rt === Bool
2831-
return InterConditional(slot_id, new, new)
2832-
end
2833-
end
2834-
return rt
2835-
end
2836-
28372896
# make as much progress on `frame` as possible (by handling cycles)
28382897
function typeinf_nocycle(interp::AbstractInterpreter, frame::InferenceState)
28392898
typeinf_local(interp, frame)

base/compiler/inferencestate.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -429,14 +429,14 @@ end
429429

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

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

0 commit comments

Comments
 (0)