Skip to content

Commit

Permalink
Fix use counts for mutable struct SROA
Browse files Browse the repository at this point in the history
PR #28478 moved the computation of the use counts before the finish call.
to fix #28444. However, the early parts of the finish call fixes up phi
node arguments, which fail to get counted if we look at use counts before
that fixup is performed. This causes #30594 where the only non-trivial use
is on the backedge of the phi and would thus incorrectly fail to get accounted
for. Fix that by taking the use count after phi fixup but before dce.

(cherry picked from commit f8f2045)
  • Loading branch information
Keno authored and KristofferC committed Jan 11, 2019
1 parent bac9350 commit da5d637
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
10 changes: 7 additions & 3 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -694,10 +694,14 @@ function getfield_elim_pass!(ir::IRCode, domtree::DomTree)
compact[idx] = val === nothing ? nothing : val.x
end

# Copy the use count, `finish` may modify it and for our predicate
# below we need it consistent with the state of the IR here.

non_dce_finish!(compact)
# Copy the use count, `simple_dce!` may modify it and for our predicate
# below we need it consistent with the state of the IR here (after tracking
# phi node arguments, but before dce).
used_ssas = copy(compact.used_ssas)
ir = finish(compact)
simple_dce!(compact)
ir = complete(compact)
# Now go through any mutable structs and see which ones we can eliminate
for (idx, (intermediaries, defuse)) in defuses
intermediaries = collect(intermediaries)
Expand Down
26 changes: 26 additions & 0 deletions test/compiler/irpasses.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,32 @@

using Test

# Tests for SROA

mutable struct Foo30594; x::Float64; end
Base.copy(x::Foo30594) = Foo30594(x.x)
function add!(p::Foo30594, off::Foo30594)
p.x += off.x
return p
end
Base.:(+)(a::Foo30594, b::Foo30594) = add!(copy(a), b)

let results = Float64[]
@noinline use30594(x) = push!(results, x.x); nothing
function foo30594(cnt::Int, dx::Int)
step = Foo30594(dx)
curr = step + Foo30594(1)
for i in 1:cnt
use30594(curr)
curr = curr + step
end
nothing
end

foo30594(4, -1)
@test results == [0.0, -1.0, -2.0, -3.0]
end

# Issue #29983
# This one is a bit hard to trigger, but the key is to create a case
# where SROA needs to introduce an intermediate type-unstable phi node
Expand Down

0 comments on commit da5d637

Please sign in to comment.