From 0b9da90bc3e1ba5d5b3eafe343eeced3eddb0991 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 6 Aug 2018 19:34:02 +0100 Subject: [PATCH] Fix legality check in SROA (#28478) The legality check was using use counts after `finish(compact)` got to delete, which made them inaccurate. Instead, take a copy of the use counts before. Additionally, ignore any uses that got deleted during `finish(compact)`. Fixes #28444. --- base/compiler/ssair/passes.jl | 15 ++++++++++++--- test/compiler/compiler.jl | 13 +++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 0eaa190f94a5b..ce04b81bac1eb 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -688,6 +688,9 @@ 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. + used_ssas = copy(compact.used_ssas) ir = finish(compact) # Now go through any mutable structs and see which ones we can eliminate for (idx, (intermediaries, defuse)) in defuses @@ -699,9 +702,9 @@ function getfield_elim_pass!(ir::IRCode, domtree::DomTree) nleaves = length(defuse.uses) + length(defuse.defs) + length(defuse.ccall_preserve_uses) nuses = 0 for idx in intermediaries - nuses += compact.used_ssas[idx] + nuses += used_ssas[idx] end - nuses_total = compact.used_ssas[idx] + nuses - length(intermediaries) + nuses_total = used_ssas[idx] + nuses - length(intermediaries) nleaves == nuses_total || continue # Find the type for this allocation defexpr = ir[SSAValue(idx)] @@ -717,7 +720,13 @@ function getfield_elim_pass!(ir::IRCode, domtree::DomTree) fielddefuse = SSADefUse[SSADefUse() for _ = 1:fieldcount(typ)] ok = true for use in defuse.uses - field = try_compute_fieldidx_expr(typ, ir[SSAValue(use)]) + stmt = ir[SSAValue(use)] + # We may have discovered above that this use is dead + # after the getfield elim of immutables. In that case, + # it would have been deleted. That's fine, just ignore + # the use in that case. + stmt === nothing && continue + field = try_compute_fieldidx_expr(typ, stmt) field === nothing && (ok = false; break) push!(fielddefuse[field].uses, use) end diff --git a/test/compiler/compiler.jl b/test/compiler/compiler.jl index 570d5d683939a..3a14af16a8f36 100644 --- a/test/compiler/compiler.jl +++ b/test/compiler/compiler.jl @@ -1950,3 +1950,16 @@ end h28356() = f28356(Any[Float64][1]) @test h28356() isa S28356{Float64} + +# Issue #28444 +mutable struct foo28444 + a::Int + b::Int +end +function bar28444() + a = foo28444(1, 2) + c, d = a.a, a.b + e = (c, d) + e[1] +end +@test bar28444() == 1