Skip to content

Commit 6973e0f

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 0402eb7 commit 6973e0f

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
@@ -2447,19 +2447,52 @@ function abstract_eval_ssavalue(s::SSAValue, ssavaluetypes::Vector{Any})
24472447
return typ
24482448
end
24492449

2450-
function widenreturn(ipo_lattice::AbstractLattice, @nospecialize(rt), @nospecialize(bestguess), nargs::Int, slottypes::Vector{Any}, changes::VarTable)
2451-
= (ipo_lattice)
2452-
inner_lattice = widenlattice(ipo_lattice)
2453-
= (inner_lattice)
2454-
if !(bestguess ₚ Bool) || bestguess === Bool
2450+
struct BestguessInfo{Interp<:AbstractInterpreter}
2451+
interp::Interp
2452+
bestguess
2453+
nargs::Int
2454+
slottypes::Vector{Any}
2455+
changes::VarTable
2456+
function BestguessInfo(interp::Interp, @nospecialize(bestguess), nargs::Int,
2457+
slottypes::Vector{Any}, changes::VarTable) where Interp<:AbstractInterpreter
2458+
new{Interp}(interp, bestguess, nargs, slottypes, changes)
2459+
end
2460+
end
2461+
2462+
"""
2463+
widenreturn(@nospecialize(rt), info::BestguessInfo) -> new_bestguess
2464+
2465+
Appropriately converts inferred type of a return value `rt` to such a type
2466+
that we know we can store in the cache and is valid and good inter-procedurally,
2467+
E.g. if `rt isa Conditional` then `rt` should be converted to `InterConditional`
2468+
or the other cachable lattice element.
2469+
2470+
External lattice `𝕃ₑ::ExternalLattice` may overload:
2471+
- `widenreturn(𝕃ₑ::ExternalLattice, @nospecialize(rt), info::BestguessInfo)`
2472+
- `widenreturn_noslotwrapper(𝕃ₑ::ExternalLattice, @nospecialize(rt), info::BestguessInfo)`
2473+
"""
2474+
function widenreturn(@nospecialize(rt), info::BestguessInfo)
2475+
return widenreturn(typeinf_lattice(info.interp), rt, info)
2476+
end
2477+
2478+
function widenreturn(𝕃ᵢ::AbstractLattice, @nospecialize(rt), info::BestguessInfo)
2479+
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
2480+
end
2481+
function widenreturn_noslotwrapper(𝕃ᵢ::AbstractLattice, @nospecialize(rt), info::BestguessInfo)
2482+
return widenreturn_noslotwrapper(widenlattice(𝕃ᵢ), rt, info)
2483+
end
2484+
2485+
function widenreturn(𝕃ᵢ::ConditionalsLattice, @nospecialize(rt), info::BestguessInfo)
2486+
= (𝕃ᵢ)
2487+
if !((ipo_lattice(info.interp), info.bestguess, Bool)) || info.bestguess === Bool
24552488
# give up inter-procedural constraint back-propagation
24562489
# when tmerge would widen the result anyways (as an optimization)
24572490
rt = widenconditional(rt)
24582491
else
24592492
if isa(rt, Conditional)
24602493
id = rt.slot
2461-
if 1 id nargs
2462-
old_id_type = widenconditional(slottypes[id]) # same as `(states[1]::VarTable)[id].typ`
2494+
if 1 id info.nargs
2495+
old_id_type = widenconditional(info.slottypes[id]) # same as `(states[1]::VarTable)[id].typ`
24632496
if (!(rt.thentype ᵢ old_id_type) || old_id_type ᵢ rt.thentype) &&
24642497
(!(rt.elsetype ᵢ old_id_type) || old_id_type ᵢ rt.elsetype)
24652498
# discard this `Conditional` since it imposes
@@ -2476,44 +2509,69 @@ function widenreturn(ipo_lattice::AbstractLattice, @nospecialize(rt), @nospecial
24762509
end
24772510
if isa(rt, Conditional)
24782511
rt = InterConditional(rt.slot, rt.thentype, rt.elsetype)
2479-
elseif is_lattice_bool(ipo_lattice, rt)
2480-
if isa(bestguess, InterConditional)
2481-
# if the bestguess so far is already `Conditional`, try to convert
2482-
# this `rt` into `Conditional` on the slot to avoid overapproximation
2483-
# due to conflict of different slots
2484-
rt = bool_rt_to_conditional(rt, slottypes, changes, bestguess.slot)
2485-
else
2486-
# pick up the first "interesting" slot, convert `rt` to its `Conditional`
2487-
# TODO: ideally we want `Conditional` and `InterConditional` to convey
2488-
# constraints on multiple slots
2489-
for slot_id in 1:nargs
2490-
rt = bool_rt_to_conditional(rt, slottypes, changes, slot_id)
2491-
rt isa InterConditional && break
2492-
end
2493-
end
2512+
elseif is_lattice_bool(𝕃ᵢ, rt)
2513+
rt = bool_rt_to_conditional(rt, info)
24942514
end
24952515
end
2496-
2497-
# only propagate information we know we can store
2498-
# and is valid and good inter-procedurally
24992516
isa(rt, Conditional) && return InterConditional(rt)
25002517
isa(rt, InterConditional) && return rt
2501-
return widenreturn_noconditional(widenlattice(ipo_lattice), rt)
2518+
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
2519+
end
2520+
function bool_rt_to_conditional(@nospecialize(rt), info::BestguessInfo)
2521+
bestguess = info.bestguess
2522+
if isa(bestguess, InterConditional)
2523+
# if the bestguess so far is already `Conditional`, try to convert
2524+
# this `rt` into `Conditional` on the slot to avoid overapproximation
2525+
# due to conflict of different slots
2526+
rt = bool_rt_to_conditional(rt, bestguess.slot, info)
2527+
else
2528+
# pick up the first "interesting" slot, convert `rt` to its `Conditional`
2529+
# TODO: ideally we want `Conditional` and `InterConditional` to convey
2530+
# constraints on multiple slots
2531+
for slot_id = 1:info.nargs
2532+
rt = bool_rt_to_conditional(rt, slot_id, info)
2533+
rt isa InterConditional && break
2534+
end
2535+
end
2536+
return rt
2537+
end
2538+
function bool_rt_to_conditional(@nospecialize(rt), slot_id::Int, info::BestguessInfo)
2539+
= (typeinf_lattice(info.interp))
2540+
old = info.slottypes[slot_id]
2541+
new = widenconditional(info.changes[slot_id].typ) # avoid nested conditional
2542+
if new ᵢ old && !(old ᵢ new)
2543+
if isa(rt, Const)
2544+
val = rt.val
2545+
if val === true
2546+
return InterConditional(slot_id, new, Bottom)
2547+
elseif val === false
2548+
return InterConditional(slot_id, Bottom, new)
2549+
end
2550+
elseif rt === Bool
2551+
return InterConditional(slot_id, new, new)
2552+
end
2553+
end
2554+
return rt
25022555
end
25032556

2504-
function widenreturn_noconditional(inner_lattice::AbstractLattice, @nospecialize(rt))
2505-
isa(rt, Const) && return rt
2506-
isa(rt, Type) && return rt
2557+
function widenreturn(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
2558+
return widenreturn_partials(𝕃ᵢ, rt, info)
2559+
end
2560+
function widenreturn_noslotwrapper(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
2561+
return widenreturn_partials(𝕃ᵢ, rt, info)
2562+
end
2563+
function widenreturn_partials(𝕃ᵢ::PartialsLattice, @nospecialize(rt), info::BestguessInfo)
25072564
if isa(rt, PartialStruct)
25082565
fields = copy(rt.fields)
25092566
local anyrefine = false
2567+
𝕃 = typeinf_lattice(info.interp)
25102568
for i in 1:length(fields)
25112569
a = fields[i]
2512-
a = isvarargtype(a) ? a : widenreturn_noconditional(inner_lattice, a)
2570+
a = isvarargtype(a) ? a : widenreturn_noslotwrapper(𝕃, a, info)
25132571
if !anyrefine
25142572
# TODO: consider adding && const_prop_profitable(a) here?
25152573
anyrefine = has_const_info(a) ||
2516-
(inner_lattice, a, fieldtype(rt.typ, i))
2574+
(𝕃, a, fieldtype(rt.typ, i))
25172575
end
25182576
fields[i] = a
25192577
end
@@ -2522,6 +2580,24 @@ function widenreturn_noconditional(inner_lattice::AbstractLattice, @nospecialize
25222580
if isa(rt, PartialOpaque)
25232581
return rt # XXX: this case was missed in #39512
25242582
end
2583+
return widenreturn(widenlattice(𝕃ᵢ), rt, info)
2584+
end
2585+
2586+
function widenreturn(::ConstsLattice, @nospecialize(rt), ::BestguessInfo)
2587+
return widenreturn_consts(rt)
2588+
end
2589+
function widenreturn_noslotwrapper(::ConstsLattice, @nospecialize(rt), ::BestguessInfo)
2590+
return widenreturn_consts(rt)
2591+
end
2592+
function widenreturn_consts(@nospecialize(rt))
2593+
isa(rt, Const) && return rt
2594+
return widenconst(rt)
2595+
end
2596+
2597+
function widenreturn(::JLTypeLattice, @nospecialize(rt), ::BestguessInfo)
2598+
return widenconst(rt)
2599+
end
2600+
function widenreturn_noslotwrapper(::JLTypeLattice, @nospecialize(rt), ::BestguessInfo)
25252601
return widenconst(rt)
25262602
end
25272603

@@ -2585,15 +2661,15 @@ end
25852661
end
25862662
end
25872663

2588-
function update_bbstate!(lattice::AbstractLattice, frame::InferenceState, bb::Int, vartable::VarTable)
2664+
function update_bbstate!(𝕃ᵢ::AbstractLattice, frame::InferenceState, bb::Int, vartable::VarTable)
25892665
bbtable = frame.bb_vartables[bb]
25902666
if bbtable === nothing
25912667
# if a basic block hasn't been analyzed yet,
25922668
# we can update its state a bit more aggressively
25932669
frame.bb_vartables[bb] = copy(vartable)
25942670
return true
25952671
else
2596-
return stupdate!(lattice, bbtable, vartable)
2672+
return stupdate!(𝕃ᵢ, bbtable, vartable)
25972673
end
25982674
end
25992675

@@ -2615,6 +2691,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
26152691
ssavaluetypes = frame.ssavaluetypes
26162692
bbs = frame.cfg.blocks
26172693
nbbs = length(bbs)
2694+
𝕃ₚ, 𝕃ᵢ = ipo_lattice(interp), typeinf_lattice(interp)
26182695

26192696
currbb = frame.currbb
26202697
if currbb != 1
@@ -2693,19 +2770,19 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
26932770
# We continue with the true branch, but process the false
26942771
# branch here.
26952772
if isa(condt, Conditional)
2696-
else_change = conditional_change(currstate, condt.elsetype, condt.slot)
2773+
else_change = conditional_change(𝕃ᵢ, currstate, condt.elsetype, condt.slot)
26972774
if else_change !== nothing
26982775
false_vartable = stoverwrite1!(copy(currstate), else_change)
26992776
else
27002777
false_vartable = currstate
27012778
end
2702-
changed = update_bbstate!(typeinf_lattice(interp), frame, falsebb, false_vartable)
2703-
then_change = conditional_change(currstate, condt.thentype, condt.slot)
2779+
changed = update_bbstate!(𝕃ᵢ, frame, falsebb, false_vartable)
2780+
then_change = conditional_change(𝕃ᵢ, currstate, condt.thentype, condt.slot)
27042781
if then_change !== nothing
27052782
stoverwrite1!(currstate, then_change)
27062783
end
27072784
else
2708-
changed = update_bbstate!(typeinf_lattice(interp), frame, falsebb, currstate)
2785+
changed = update_bbstate!(𝕃ᵢ, frame, falsebb, currstate)
27092786
end
27102787
if changed
27112788
handle_control_backedge!(interp, frame, currpc, stmt.dest)
@@ -2717,7 +2794,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27172794
elseif isa(stmt, ReturnNode)
27182795
bestguess = frame.bestguess
27192796
rt = abstract_eval_value(interp, stmt.val, currstate, frame)
2720-
rt = widenreturn(ipo_lattice(interp), rt, bestguess, nargs, slottypes, currstate)
2797+
rt = widenreturn(rt, BestguessInfo(interp, bestguess, nargs, slottypes, currstate))
27212798
# narrow representation of bestguess slightly to prepare for tmerge with rt
27222799
if rt isa InterConditional && bestguess isa Const
27232800
let slot_id = rt.slot
@@ -2737,9 +2814,9 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27372814
if !isempty(frame.limitations)
27382815
rt = LimitedAccuracy(rt, copy(frame.limitations))
27392816
end
2740-
if tchanged(ipo_lattice(interp), rt, bestguess)
2817+
if tchanged(𝕃ₚ, rt, bestguess)
27412818
# new (wider) return type for frame
2742-
bestguess = tmerge(ipo_lattice(interp), bestguess, rt)
2819+
bestguess = tmerge(𝕃ₚ, bestguess, rt)
27432820
# TODO: if bestguess isa InterConditional && !interesting(bestguess); bestguess = widenconditional(bestguess); end
27442821
frame.bestguess = bestguess
27452822
for (caller, caller_pc) in frame.cycle_backedges
@@ -2755,7 +2832,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27552832
# Propagate entry info to exception handler
27562833
l = stmt.args[1]::Int
27572834
catchbb = block_for_inst(frame.cfg, l)
2758-
if update_bbstate!(typeinf_lattice(interp), frame, catchbb, currstate)
2835+
if update_bbstate!(𝕃ᵢ, frame, catchbb, currstate)
27592836
push!(W, catchbb)
27602837
end
27612838
ssavaluetypes[currpc] = Any
@@ -2780,7 +2857,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27802857
# propagate new type info to exception handler
27812858
# the handling for Expr(:enter) propagates all changes from before the try/catch
27822859
# so this only needs to propagate any changes
2783-
if stupdate1!(typeinf_lattice(interp), states[exceptbb]::VarTable, changes)
2860+
if stupdate1!(𝕃ᵢ, states[exceptbb]::VarTable, changes)
27842861
push!(W, exceptbb)
27852862
end
27862863
cur_hand = frame.handler_at[cur_hand]
@@ -2792,7 +2869,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
27922869
continue
27932870
end
27942871
if !isempty(frame.ssavalue_uses[currpc])
2795-
record_ssa_assign!(currpc, type, frame)
2872+
record_ssa_assign!(𝕃ᵢ, currpc, type, frame)
27962873
else
27972874
ssavaluetypes[currpc] = type
27982875
end
@@ -2805,7 +2882,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
28052882

28062883
# Case 2: Directly branch to a different BB
28072884
begin @label branch
2808-
if update_bbstate!(typeinf_lattice(interp), frame, nextbb, currstate)
2885+
if update_bbstate!(𝕃ᵢ, frame, nextbb, currstate)
28092886
push!(W, nextbb)
28102887
end
28112888
end
@@ -2829,13 +2906,13 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
28292906
nothing
28302907
end
28312908

2832-
function conditional_change(state::VarTable, @nospecialize(typ), slot::Int)
2909+
function conditional_change(𝕃ᵢ::AbstractLattice, state::VarTable, @nospecialize(typ), slot::Int)
28332910
vtype = state[slot]
28342911
oldtyp = vtype.typ
28352912
if iskindtype(typ)
28362913
# this code path corresponds to the special handling for `isa(x, iskindtype)` check
28372914
# implemented within `abstract_call_builtin`
2838-
elseif ignorelimited(typ) ignorelimited(oldtyp)
2915+
elseif (𝕃ᵢ, ignorelimited(typ), ignorelimited(oldtyp))
28392916
# approximate test for `typ ∩ oldtyp` being better than `oldtyp`
28402917
# since we probably formed these types with `typesubstract`,
28412918
# the comparison is likely simple
@@ -2845,29 +2922,11 @@ function conditional_change(state::VarTable, @nospecialize(typ), slot::Int)
28452922
if oldtyp isa LimitedAccuracy
28462923
# typ is better unlimited, but we may still need to compute the tmeet with the limit
28472924
# "causes" since we ignored those in the comparison
2848-
typ = tmerge(typ, LimitedAccuracy(Bottom, oldtyp.causes))
2925+
typ = tmerge(𝕃ᵢ, typ, LimitedAccuracy(Bottom, oldtyp.causes))
28492926
end
28502927
return StateUpdate(SlotNumber(slot), VarState(typ, vtype.undef), state, true)
28512928
end
28522929

2853-
function bool_rt_to_conditional(@nospecialize(rt), slottypes::Vector{Any}, state::VarTable, slot_id::Int)
2854-
old = slottypes[slot_id]
2855-
new = widenconditional(state[slot_id].typ) # avoid nested conditional
2856-
if new old && !(old new)
2857-
if isa(rt, Const)
2858-
val = rt.val
2859-
if val === true
2860-
return InterConditional(slot_id, new, Bottom)
2861-
elseif val === false
2862-
return InterConditional(slot_id, Bottom, new)
2863-
end
2864-
elseif rt === Bool
2865-
return InterConditional(slot_id, new, new)
2866-
end
2867-
end
2868-
return rt
2869-
end
2870-
28712930
# make as much progress on `frame` as possible (by handling cycles)
28722931
function typeinf_nocycle(interp::AbstractInterpreter, frame::InferenceState)
28732932
typeinf_local(interp, frame)

base/compiler/inferencestate.jl

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

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

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

0 commit comments

Comments
 (0)