From da5d637adc93d98ba7078165e97ad545b030abef Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Fri, 4 Jan 2019 19:35:28 -0500 Subject: [PATCH] Fix use counts for mutable struct SROA 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 f8f20453c4ca7a6aeb23a189a420dbff130679ce) --- base/compiler/ssair/passes.jl | 10 +++++++--- test/compiler/irpasses.jl | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 887da342b663c..12cd0cd582f8c 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -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) diff --git a/test/compiler/irpasses.jl b/test/compiler/irpasses.jl index e37b9fa5c4a0e..91bb336a5ab9a 100644 --- a/test/compiler/irpasses.jl +++ b/test/compiler/irpasses.jl @@ -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