Fix SROA miscompile in large functions (#46819)

* Fix SROA miscompile in large functions

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.

* optimizer: address TODO for computing `joint_effects` more efficiently

Co-authored-by: Shuhei Kadowaki <aviatesk@gmail.com>

Author: Keno

base/compiler/ssair/inlining.jl

@@ -1338,27 +1338,6 @@ function info_effects(@nospecialize(result), match::MethodMatch, state::Inlining
     end
 end
 
-function compute_joint_effects(info::Union{ConstCallInfo, Vector{MethodMatchInfo}}, state::InliningState)
-    if isa(info, ConstCallInfo)
-        (; call, results) = info
-        infos = isa(call, MethodMatchInfo) ? MethodMatchInfo[call] : call.matches
-    else
-        results = nothing
-        infos = info
-    end
-    local all_result_count = 0
-    local joint_effects::Effects = EFFECTS_TOTAL
-    for i in 1:length(infos)
-        meth = infos[i].results
-        for (j, match) in enumerate(meth)
-            all_result_count += 1
-            result = results === nothing ? nothing : results[all_result_count]
-            joint_effects = merge_effects(joint_effects, info_effects(result, match, state))
-        end
-    end
-    return joint_effects
-end
-
 function compute_inlining_cases(info::Union{ConstCallInfo, Vector{MethodMatchInfo}},
     flag::UInt8, sig::Signature, state::InliningState)
     argtypes = sig.argtypes
@@ -1376,6 +1355,8 @@ function compute_inlining_cases(info::Union{ConstCallInfo, Vector{MethodMatchInf
     local only_method = nothing
     local meth::MethodLookupResult
     local all_result_count = 0
+    local joint_effects::Effects = EFFECTS_TOTAL
+    local nothrow::Bool = true
     for i in 1:length(infos)
         meth = infos[i].results
         if meth.ambig
@@ -1400,6 +1381,8 @@ function compute_inlining_cases(info::Union{ConstCallInfo, Vector{MethodMatchInf
         for (j, match) in enumerate(meth)
             all_result_count += 1
             result = results === nothing ? nothing : results[all_result_count]
+            joint_effects = merge_effects(joint_effects, info_effects(result, match, state))
+            nothrow &= match.fully_covers
             any_fully_covered |= match.fully_covers
             if !validate_sparams(match.sparams)
                 if !match.fully_covers
@@ -1418,6 +1401,8 @@ function compute_inlining_cases(info::Union{ConstCallInfo, Vector{MethodMatchInf
         end
     end
 
+    joint_effects = Effects(joint_effects; nothrow)
+
     if handled_all_cases && revisit_idx !== nothing
         # we handled everything except one match with unmatched sparams,
         # so try to handle it by bypassing validate_sparams
@@ -1447,9 +1432,6 @@ function compute_inlining_cases(info::Union{ConstCallInfo, Vector{MethodMatchInf
         filter!(case::InliningCase->isdispatchtuple(case.sig), cases)
     end
 
-    # TODO fuse `compute_joint_effects` into the loop above, which currently causes compilation error
-    joint_effects = Effects(compute_joint_effects(info, state); nothrow=handled_all_cases)
-
     return cases, (handled_all_cases & any_fully_covered), joint_effects
 end
 

base/compiler/ssair/ir.jl

@@ -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)
@@ -674,7 +680,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
@@ -683,7 +690,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