Skip to content

Commit

Permalink
Fix SROA miscompile in large functions
Browse files Browse the repository at this point in the history
During the development of #46775, we saw a miscompile in the compiler,
specifically, we saw SROA failing to provide a proper PHI nest. The
root cause of this turned out to be an incorrect dominance query.
In particular, during incremental compaction, the non-compacted
portion of the IncrementalCompact cfg is allocated, but not valid,
so we must not query it for basic block information.

Unfortunately, I don't really have a good test case for this.
This code has grown mostly organically for a few years, and I
think it's probably overdue for an overhaul.
  • Loading branch information
Keno committed Sep 18, 2022
1 parent e758982 commit f9d3e16
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions base/compiler/ssair/ir.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,18 @@ function cfg_delete_edge!(cfg::CFG, from::Int, to::Int)
nothing
end

function bb_ordering()
lt=(<=)
by=x->first(x.stmts)
ord(lt, by, nothing, Forward)
end

function block_for_inst(index::Vector{Int}, inst::Int)
return searchsortedfirst(index, inst, lt=(<=))
end

function block_for_inst(index::Vector{BasicBlock}, inst::Int)
return searchsortedfirst(index, BasicBlock(StmtRange(inst, inst)), by=x->first(x.stmts), lt=(<=))-1
return searchsortedfirst(index, BasicBlock(StmtRange(inst, inst)), bb_ordering())-1
end

block_for_inst(cfg::CFG, inst::Int) = block_for_inst(cfg.index, inst)
Expand Down Expand Up @@ -672,7 +678,8 @@ end
function block_for_inst(compact::IncrementalCompact, idx::SSAValue)
id = idx.id
if id < compact.result_idx # if ssa within result
return block_for_inst(compact.result_bbs, id)
return searchsortedfirst(compact.result_bbs, BasicBlock(StmtRange(id, id)),
1, compact.active_result_bb, bb_ordering())-1
else
return block_for_inst(compact.ir.cfg, id)
end
Expand All @@ -681,7 +688,8 @@ end
function block_for_inst(compact::IncrementalCompact, idx::OldSSAValue)
id = idx.id
if id < compact.idx # if ssa within result
return block_for_inst(compact.result_bbs, compact.ssa_rename[id])
id = compact.ssa_rename[id]
return block_for_inst(compact, SSAValue(id))
else
return block_for_inst(compact.ir.cfg, id)
end
Expand Down

0 comments on commit f9d3e16

Please sign in to comment.