Skip to content

Commit

Permalink
Fix legality check in SROA (#28478)
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 0f3d586)
  • Loading branch information
Keno authored and ararslan committed Aug 6, 2018
1 parent 0c76bfa commit 7db5750
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
15 changes: 12 additions & 3 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)]
Expand All @@ -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
Expand Down
13 changes: 13 additions & 0 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7db5750

Please sign in to comment.