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.
  • Loading branch information
Keno committed Jan 6, 2019
1 parent efe367d commit f8f2045
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 @@ -688,10 +688,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 @@ -37,3 +37,29 @@ let m = Meta.@lower 1 + 1
Core.Compiler.verify_ir(ir)
@test isa(ir.stmts[3], Core.PhiNode) && length(ir.stmts[3].edges) == 1
end

# 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

2 comments on commit f8f2045

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.