Skip to content

Commit c336656

Browse files
committed
post-opt: use augmented post-domtree for visit_conditional_successors
This commit fixes the first problem that was found while digging into #53613. It turns out that the post-domtree constructed from regular `IRCode` doesn't work for visiting conditional successors for post-opt analysis in cases like: ```julia julia> let code = Any[ # block 1 GotoIfNot(Argument(2), 3), # block 2 ReturnNode(Argument(3)), # block 3 (we should visit this block) Expr(:call, throw, "potential throw"), ReturnNode(), # unreachable ] ir = make_ircode(code; slottypes=Any[Any,Bool,Bool]) visited = BitSet() @test !Core.Compiler.visit_conditional_successors(CC.LazyPostDomtree(ir), ir, #=bb=#1) do succ::Int push!(visited, succ) return false end @test 2 ∉ visited @test 3 ∈ visited end Test Failed at REPL[14]:16 Expression: 2 ∉ visited Evaluated: 2 ∉ BitSet([2]) ``` This might mean that we need to fix on the `postdominates` end, but for now, this commit tries to get around it by using the augmented post domtree in `visit_conditional_successors`. Since the augmented post domtree is enforced to have a single return, we can keep using the current `postdominates` to fix the issue. However, this commit isn't enough to fix the NeuralNetworkReachability segfault as reported in #53613, and we need to tackle the second issue reported there too (#53613 (comment)).
1 parent f6504e4 commit c336656

File tree

4 files changed

+312
-44
lines changed

4 files changed

+312
-44
lines changed

base/compiler/optimize.jl

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -527,15 +527,53 @@ function any_stmt_may_throw(ir::IRCode, bb::Int)
527527
return false
528528
end
529529

530-
function visit_conditional_successors(callback, lazypostdomtree::LazyPostDomtree, ir::IRCode, bb::Int)
530+
mutable struct LazyAugmentedDomtrees
531+
const ir::IRCode
532+
cfg::CFG
533+
domtree::DomTree
534+
postdomtree::PostDomTree
535+
LazyAugmentedDomtrees(ir::IRCode) = new(ir)
536+
end
537+
538+
function get!(lazyagdomtrees::LazyAugmentedDomtrees, sym::Symbol)
539+
isdefined(lazyagdomtrees, sym) && return getfield(lazyagdomtrees, sym)
540+
if sym === :cfg
541+
return lazyagdomtrees.cfg = construct_augmented_cfg(lazyagdomtrees.ir)
542+
elseif sym === :domtree
543+
return lazyagdomtrees.domtree = construct_domtree(get!(lazyagdomtrees, :cfg))
544+
elseif sym === :postdomtree
545+
return lazyagdomtrees.postdomtree = construct_postdomtree(get!(lazyagdomtrees, :cfg))
546+
else
547+
error("invalid field access")
548+
end
549+
end
550+
551+
function construct_augmented_cfg(ir::IRCode)
552+
cfg = copy(ir.cfg)
553+
# Add a virtual basic block to represent the exit
554+
push!(cfg.blocks, BasicBlock(StmtRange(0:-1)))
555+
for bb = 1:(length(cfg.blocks)-1)
556+
terminator = ir[SSAValue(last(cfg.blocks[bb].stmts))][:stmt]
557+
if isa(terminator, ReturnNode) && isdefined(terminator, :val)
558+
cfg_insert_edge!(cfg, bb, length(cfg.blocks))
559+
end
560+
end
561+
return cfg
562+
end
563+
564+
visit_conditional_successors(callback, ir::IRCode, bb::Int) =
565+
visit_conditional_successors(callback, construct_postdomtree(construct_augmented_cfg(ir)), ir, bb)
566+
visit_conditional_successors(callback, lazyagdomtrees::LazyAugmentedDomtrees, ir::IRCode, bb::Int) =
567+
visit_conditional_successors(callback, get!(lazyagdomtrees, :postdomtree), ir, bb)
568+
function visit_conditional_successors(callback, postdomtree::PostDomTree, ir::IRCode, bb::Int)
531569
visited = BitSet((bb,))
532570
worklist = Int[bb]
533571
while !isempty(worklist)
534572
thisbb = popfirst!(worklist)
535573
for succ in ir.cfg.blocks[thisbb].succs
536574
succ in visited && continue
537575
push!(visited, succ)
538-
if postdominates(get!(lazypostdomtree), succ, bb)
576+
if postdominates(postdomtree, succ, bb)
539577
# this successor is not conditional, so no need to visit it further
540578
continue
541579
elseif callback(succ)
@@ -548,40 +586,12 @@ function visit_conditional_successors(callback, lazypostdomtree::LazyPostDomtree
548586
return false
549587
end
550588

551-
struct AugmentedDomtree
552-
cfg::CFG
553-
domtree::DomTree
554-
end
555-
556-
mutable struct LazyAugmentedDomtree
557-
const ir::IRCode
558-
agdomtree::AugmentedDomtree
559-
LazyAugmentedDomtree(ir::IRCode) = new(ir)
560-
end
561-
562-
function get!(lazyagdomtree::LazyAugmentedDomtree)
563-
isdefined(lazyagdomtree, :agdomtree) && return lazyagdomtree.agdomtree
564-
ir = lazyagdomtree.ir
565-
cfg = copy(ir.cfg)
566-
# Add a virtual basic block to represent the exit
567-
push!(cfg.blocks, BasicBlock(StmtRange(0:-1)))
568-
for bb = 1:(length(cfg.blocks)-1)
569-
terminator = ir[SSAValue(last(cfg.blocks[bb].stmts))][:stmt]
570-
if isa(terminator, ReturnNode) && isdefined(terminator, :val)
571-
cfg_insert_edge!(cfg, bb, length(cfg.blocks))
572-
end
573-
end
574-
domtree = construct_domtree(cfg)
575-
return lazyagdomtree.agdomtree = AugmentedDomtree(cfg, domtree)
576-
end
577-
578589
mutable struct PostOptAnalysisState
579590
const result::InferenceResult
580591
const ir::IRCode
581592
const inconsistent::BitSetBoundedMinPrioritySet
582593
const tpdum::TwoPhaseDefUseMap
583-
const lazypostdomtree::LazyPostDomtree
584-
const lazyagdomtree::LazyAugmentedDomtree
594+
const lazyagdomtrees::LazyAugmentedDomtrees
585595
const ea_analysis_pending::Vector{Int}
586596
all_retpaths_consistent::Bool
587597
all_effect_free::Bool
@@ -592,9 +602,8 @@ mutable struct PostOptAnalysisState
592602
function PostOptAnalysisState(result::InferenceResult, ir::IRCode)
593603
inconsistent = BitSetBoundedMinPrioritySet(length(ir.stmts))
594604
tpdum = TwoPhaseDefUseMap(length(ir.stmts))
595-
lazypostdomtree = LazyPostDomtree(ir)
596-
lazyagdomtree = LazyAugmentedDomtree(ir)
597-
return new(result, ir, inconsistent, tpdum, lazypostdomtree, lazyagdomtree, Int[],
605+
lazyagdomtrees = LazyAugmentedDomtrees(ir)
606+
return new(result, ir, inconsistent, tpdum, lazyagdomtrees, Int[],
598607
true, true, nothing, true, true, false)
599608
end
600609
end
@@ -834,13 +843,13 @@ function ((; sv)::ScanStmt)(inst::Instruction, lstmt::Int, bb::Int)
834843
# inconsistent region.
835844
if !sv.result.ipo_effects.terminates
836845
sv.all_retpaths_consistent = false
837-
elseif visit_conditional_successors(sv.lazypostdomtree, sv.ir, bb) do succ::Int
846+
elseif visit_conditional_successors(sv.lazyagdomtrees, sv.ir, bb) do succ::Int
838847
return any_stmt_may_throw(sv.ir, succ)
839848
end
840849
# check if this `GotoIfNot` leads to conditional throws, which taints consistency
841850
sv.all_retpaths_consistent = false
842851
else
843-
(; cfg, domtree) = get!(sv.lazyagdomtree)
852+
cfg, domtree = get!(sv.lazyagdomtrees, :cfg), get!(sv.lazyagdomtrees, :domtree)
844853
for succ in iterated_dominance_frontier(cfg, BlockLiveness(sv.ir.cfg.blocks[bb].succs, nothing), domtree)
845854
if succ == length(cfg.blocks)
846855
# Phi node in the virtual exit -> We have a conditional

finite-iterate-interp.jl

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
include("test/compiler/newinterp.jl")
2+
3+
@newinterp FiniteIterateInterpreter
4+
5+
using Core.Compiler:
6+
AbstractLattice, BaseInferenceLattice, IPOResultLattice, InferenceLattice,
7+
ConditionalsLattice, InterConditionalsLattice, PartialsLattice, ConstsLattice,
8+
SimpleInferenceLattice,
9+
widenlattice, is_valid_lattice_norec, typeinf_lattice, ipo_lattice, optimizer_lattice,
10+
widenconst, tmeet, tmerge, , , abstract_eval_special_value, widenreturn
11+
12+
const CC = Core.Compiler
13+
14+
struct FiniteIterateLattice{L<:AbstractLattice} <: AbstractLattice
15+
parent::L
16+
end
17+
CC.widenlattice(𝕃::FiniteIterateLattice) = 𝕃.parent
18+
CC.is_valid_lattice_norec(::FiniteIterateLattice, @nospecialize(elm)) = _is_finite_lattice(elm)
19+
_is_finite_lattice(@nospecialize t) = (
20+
isa(t, FiniteIterate) || isa(t, FiniteState) || isa(t, TerminatingCondition))
21+
22+
CC.typeinf_lattice(::FiniteIterateInterpreter) =
23+
InferenceLattice(ConditionalsLattice(PartialsLattice(FiniteIterateLattice(ConstsLattice()))))
24+
CC.ipo_lattice(::FiniteIterateInterpreter) =
25+
InferenceLattice(InterConditionalsLattice(PartialsLattice(ConstsLattice())))
26+
CC.optimizer_lattice(::FiniteIterateInterpreter) =
27+
FiniteIterateLattice(SimpleInferenceLattice.instance)
28+
29+
struct FiniteIterate
30+
typ
31+
itr
32+
function FiniteIterate(@nospecialize(typ), @nospecialize(itr))
33+
@assert !_is_finite_lattice(typ) "nested FiniteLattice"
34+
return new(typ, itr)
35+
end
36+
end
37+
struct FiniteState
38+
typ
39+
itr
40+
function FiniteState(@nospecialize(typ), @nospecialize(itr))
41+
@assert !_is_finite_lattice(typ) "nested FiniteLattice"
42+
return new(typ, itr)
43+
end
44+
end
45+
struct TerminatingCondition end
46+
function CC.tmeet(𝕃::FiniteIterateLattice, @nospecialize(v), @nospecialize(t::Type))
47+
if isa(v, FiniteIterate)
48+
error("tmeet FiniteIterate")
49+
v = v.typ
50+
elseif isa(v, FiniteState)
51+
error("tmeet FiniteState")
52+
v = v.typ
53+
elseif isa(v, TerminatingCondition)
54+
error("tmeet TerminatingCondition")
55+
if t === Bool
56+
return TerminatingCondition()
57+
end
58+
return Bool
59+
end
60+
return tmeet(widenlattice(𝕃), v, t)
61+
end
62+
function CC.tmerge(𝕃::FiniteIterateLattice, @nospecialize(x), @nospecialize(y))
63+
if isa(x, FiniteIterate)
64+
if isa(y, FiniteIterate) && x.itr === y.itr
65+
return FiniteIterate(tmerge(widenlattice(𝕃), x.typ, y.typ), x.itr)
66+
end
67+
x = x.typ
68+
elseif isa(y, FiniteIterate)
69+
y = y.typ
70+
end
71+
if isa(x, FiniteState)
72+
if isa(y, FiniteState) && x.itr === y.itr
73+
return FiniteState(tmerge(widenlattice(𝕃), x.typ, y.typ), x.itr)
74+
end
75+
x = x.typ
76+
elseif isa(y, FiniteState)
77+
y = y.typ
78+
end
79+
if isa(x, TerminatingCondition)
80+
if isa(y, TerminatingCondition)
81+
return TerminatingCondition()
82+
end
83+
x = Bool
84+
elseif isa(y, TerminatingCondition)
85+
y = Bool
86+
end
87+
return tmerge(widenlattice(𝕃), x, y)
88+
end
89+
function CC.:(𝕃::FiniteIterateLattice, @nospecialize(x), @nospecialize(y))
90+
if isa(x, FiniteIterate)
91+
if isa(y, FiniteIterate)
92+
if x.itr === y.itr
93+
return (widenlattice(𝕃), x.typ, y.typ)
94+
end
95+
return false
96+
elseif isa(y, FiniteState)
97+
return false
98+
elseif isa(y, TerminatingCondition)
99+
return false
100+
end
101+
x = x.typ
102+
elseif isa(y, FiniteIterate)
103+
return x === Union{}
104+
end
105+
if isa(x, FiniteState)
106+
if isa(y, FiniteState)
107+
if x.itr === y.itr
108+
return (widenlattice(𝕃), x.typ, y.typ)
109+
end
110+
return false
111+
elseif isa(y, TerminatingCondition)
112+
return false
113+
end
114+
x = x.typ
115+
elseif isa(y, FiniteState)
116+
return x === Union{}
117+
end
118+
if isa(x, TerminatingCondition)
119+
return x !== Union{}
120+
elseif isa(y, TerminatingCondition)
121+
return x === Union{}
122+
end
123+
return (widenlattice(𝕃), x, y)
124+
end
125+
CC.widenconst(fi::FiniteIterate) = widenconst(fi.typ)
126+
CC.widenconst(fs::FiniteState) = widenconst(fs.typ)
127+
CC.widenconst(::TerminatingCondition) = Bool
128+
function widenfiniteiterate(@nospecialize x)
129+
if isa(x, FiniteIterate)
130+
return x.typ
131+
elseif isa(x, FiniteState)
132+
return x.typ
133+
elseif isa(x, TerminatingCondition)
134+
return Bool
135+
end
136+
return x
137+
end
138+
CC.widenreturn(𝕃::FiniteIterateLattice, @nospecialize(rt), info::CC.BestguessInfo) =
139+
CC.widenreturn(widenlattice(𝕃), widenfiniteiterate(rt), info)
140+
141+
function CC.abstract_call_known(interp::FiniteIterateInterpreter, @nospecialize(f),
142+
arginfo::CC.ArgInfo, si::CC.StmtInfo, sv::CC.AbsIntState,
143+
max_methods::Int)
144+
res = @invoke CC.abstract_call_known(interp::CC.AbstractInterpreter, f::Any,
145+
arginfo::CC.ArgInfo, si::CC.StmtInfo, sv::CC.AbsIntState,
146+
max_methods::Int)
147+
(; fargs, argtypes) = arginfo
148+
la = length(argtypes)
149+
if la 2 && fargs !== nothing && CC.istopfunction(f, :iterate)
150+
if res.rt !== Union{}
151+
if argtypes[2] Tuple
152+
if la == 2
153+
res = CC.CallMeta(FiniteIterate(res.rt, fargs[2]), res.exct, res.effects, res.info)
154+
elseif la == 3
155+
a3 = argtypes[3]
156+
if a3 isa FiniteState && a3.itr === fargs[2]
157+
res = CC.CallMeta(FiniteIterate(res.rt, a3.itr), res.exct, res.effects, res.info)
158+
end
159+
end
160+
end
161+
end
162+
end
163+
if la == 3 && CC.istopfunction(f, :(===))
164+
if CC.widenconditional(res.rt) === Bool && argtypes[2] isa FiniteIterate
165+
return CC.CallMeta(TerminatingCondition(), res.exct, res.effects, res.info)
166+
end
167+
end
168+
return res
169+
end
170+
CC.@nospecs function CC._getfield_tfunc(𝕃::FiniteIterateLattice, s00, name, setfield::Bool)
171+
if isa(s00, FiniteIterate)
172+
if name isa Core.Const && name.val === 2
173+
rt = CC._getfield_tfunc(widenlattice(𝕃), s00.typ, name, setfield)
174+
if rt !== Union{}
175+
return FiniteState(rt, s00.itr)
176+
end
177+
end
178+
s00 = s00.typ
179+
elseif isa(s00, FiniteState)
180+
s00 = s00.typ
181+
elseif isa(s00, TerminatingCondition)
182+
return Union{}
183+
end
184+
name = widenfiniteiterate(name)
185+
return CC._getfield_tfunc(widenlattice(𝕃), s00, name, setfield)
186+
end
187+
CC.@nospecs function CC.not_int_tfunc(𝕃::FiniteIterateLattice, x)
188+
if isa(x, TerminatingCondition)
189+
return TerminatingCondition()
190+
end
191+
return CC.not_int_tfunc(widenlattice(𝕃), x)
192+
end
193+
function CC.handle_control_backedge!(interp::FiniteIterateInterpreter, frame::CC.InferenceState,
194+
from::Int, to::Int, @nospecialize(condt))
195+
if condt === TerminatingCondition()
196+
return nothing
197+
end
198+
@invoke CC.handle_control_backedge!(interp::CC.AbstractInterpreter, frame::CC.InferenceState,
199+
from::Int, to::Int, condt::Any)
200+
end
201+
202+
using Test
203+
@test Base.infer_effects(; interp=FiniteIterateInterpreter()) do
204+
for i = (1,2,3)
205+
end
206+
end |> CC.is_terminates

test/compiler/effects.jl

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,3 +1387,13 @@ let; Base.Experimental.@force_compile; func52843(); end
13871387
# https://github.com/JuliaLang/julia/issues/53508
13881388
@test !Core.Compiler.is_consistent(Base.infer_effects(getindex, (UnitRange{Int},Int)))
13891389
@test !Core.Compiler.is_consistent(Base.infer_effects(getindex, (Base.OneTo{Int},Int)))
1390+
1391+
@noinline f53613() = @assert isdefined(@__MODULE__, :v53613)
1392+
g53613() = f53613()
1393+
@test !Core.Compiler.is_consistent(Base.infer_effects(f53613))
1394+
@test_broken !Core.Compiler.is_consistent(Base.infer_effects(g53613))
1395+
@test_throws AssertionError f53613()
1396+
@test_throws AssertionError g53613()
1397+
global v53613 = nothing
1398+
@test f53613() === nothing
1399+
@test g53613() === nothing

0 commit comments

Comments
 (0)