IncrementalCompact: fix stateful behavior when using multiple iterators.#46952
IncrementalCompact: fix stateful behavior when using multiple iterators.#46952
Conversation
|
Seems good to me. I think there's some argument that maybe it should restart at the beginning and iterate the already-compacted stmts first, but I'm ok with the stateful version also. |
|
This crashes during bootstrap, because in inlining we have some special IncrementalCompact usage sharing state: julia/base/compiler/ssair/inlining.jl Lines 422 to 423 in 9fd4087 julia/base/compiler/ssair/ir.jl Lines 630 to 643 in 9fd4087 @Keno what's the exact intended behavior of EDIT: apparently that's used by inlining, to splice IR into another function: julia> parent(i) = i + 42
julia> ir = only(Base.code_ircode(parent, (Int,)))[1]
1 1 ─ %1 = Base.add_int(_2, 42)::Int64
└── return %1
# partial compaction (advancing to the point we'll inline at)
julia> compact = Core.Compiler.IncrementalCompact(ir)
julia> state = Core.Compiler.iterate(compact)
1 1 ─ %1 = Base.add_int(_2, 42)::Int64
─────────────────────────────────────────────────────────────────
└── return %1
# code to inline
julia> child() = ccall(:jl_breakpoint, Nothing, (Any,), nothing)
julia> inline = only(Base.code_ircode(child, ()))[1]
1 1 ─ %1 = $(Expr(:foreigncall, :(:jl_breakpoint), Nothing, svec(Any), 0, :(:ccall), :(Main.nothing)))::Core.Const(nothing)
└── return %1
# iterator to splice both together
julia> inline_compact = Core.Compiler.IncrementalCompact(compact, inline, compact.result_idx)
1 1 ─ %1 = Base.add_int(_2, 42)::Int64
─────────────────────────────────────────────────────────────────
1 1 ─ %1 = $(Expr(:foreigncall, :(:jl_breakpoint), Nothing, svec(Any), 0, :(:ccall), :(Main.nothing)))::Core.Const(nothing)
└── return %1
# iterating it will inline the code
julia> Core.Compiler.iterate(inline_compact)
1 1 ─ Base.add_int(_2, 42)::Int64
└── $(Expr(:foreigncall, :(:jl_breakpoint), Nothing, svec(Any), 0, :(:ccall), :(Main.nothing)))::Core.Const(nothing)
─────────────────────────────────────────────────────────────────
!!! ─ return %1
# not complete the original iterator
julia> while state !== nothing
state = Core.Compiler.iterate(compact, state[2])
end
julia> ir = Core.Compiler.complete(compact)
1 1 ─ %1 = Base.add_int(_2, 42)::Int64
│ $(Expr(:foreigncall, :(:jl_breakpoint), Nothing, svec(Any), 0, :(:ccall), :(Main.nothing)))::Core.Const(nothing)
└── return %1 |
89729b9 to
20aa918
Compare
20aa918 to
ae81c88
Compare
|
Found the issue: the iterator state was tracking the active bb, not the active result bb, so I added a field to IncrementalCompact to keep track of that, and use it during initialization of the iterator. Now, with @nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
|
That's more of an improvement than I expected... |
|
Let's see if this reproduces if we test against the exact merge-base: @nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
|
Seems to be better or equal and fixes the issue so SGTM |
Fixes #46945