sroa: Fix affinity of inserted undef check (#52864)

There is a bit of complexity here making sure that this instruction ends
up in the correct basic block because there's a fair number of corner
cases (e.g. whether we're deleting the first instruction of a basic
block or the last or both). Side-step all that by never doing the
delete-then-insert dance and instead doing regular instruction
replacement if necessary. Fixes #52857.

Author: Keno

base/compiler/ssair/passes.jl

@@ -1403,7 +1403,7 @@ function sroa_pass!(ir::IRCode, inlining::Union{Nothing,InliningState}=nothing)
         (lifted_val, nest) = perform_lifting!(compact,
             visited_philikes, field, result_t, lifted_leaves, val, lazydomtree)
 
-        node_was_deleted = false
+        should_delete_node = false
         line = compact[SSAValue(idx)][:line]
         if lifted_val !== nothing && !⊑(𝕃ₒ, compact[SSAValue(idx)][:type], result_t)
             compact[idx] = lifted_val === nothing ? nothing : lifted_val.val
@@ -1412,9 +1412,8 @@ function sroa_pass!(ir::IRCode, inlining::Union{Nothing,InliningState}=nothing)
             # Save some work in a later compaction, by inserting this into the renamer now,
             # but only do this if we didn't set the REFINED flag, to save work for irinterp
             # in revisiting only the renamings that came through *this* idx.
-            delete_inst_here!(compact)
             compact.ssa_rename[old_idx] = lifted_val === nothing ? nothing : lifted_val.val
-            node_was_deleted = true
+            should_delete_node = true
         else
             compact[idx] = lifted_val === nothing ? nothing : lifted_val.val
         end
@@ -1435,17 +1434,24 @@ function sroa_pass!(ir::IRCode, inlining::Union{Nothing,InliningState}=nothing)
                 def_val = (def_val::LiftedValue).val
                 finish_phi_nest!(compact, nest)
             end
-            ni = NewInstruction(
-                Expr(:throw_undef_if_not, Symbol("##getfield##"), def_val), Nothing, line)
-            if node_was_deleted
-                insert_node_here!(compact, ni, true)
+            throw_expr = Expr(:throw_undef_if_not, Symbol("##getfield##"), def_val)
+            if should_delete_node
+                # Replace the node we already have rather than deleting/re-inserting.
+                # This way it is easier to handle BB boundary corner cases.
+                compact[SSAValue(idx)] = throw_expr
+                compact[SSAValue(idx)][:type] = Nothing
+                compact[SSAValue(idx)][:flag] = IR_FLAG_EFFECT_FREE | IR_FLAG_CONSISTENT | IR_FLAG_NOUB
+                should_delete_node = false
             else
+                ni = NewInstruction(throw_expr, Nothing, line)
                 insert_node!(compact, SSAValue(idx), ni)
             end
         else
             # val must be defined
             @assert lifted_val !== nothing
         end
+
+        should_delete_node && delete_inst_here!(compact)
     end
 
     non_dce_finish!(compact)

test/compiler/irpasses.jl

@@ -1764,3 +1764,17 @@ let code = Any[
     @test Core.Compiler.verify_ir(ir) === nothing
     @test count(x->isa(x, GotoIfNot), ir.stmts.stmt) == 1
 end
+
+# Issue #52857 - Affinity of sroa definedness check
+let code = Any[
+    Expr(:new, ImmutableRef{Any}),
+    GotoIfNot(Argument(1), 4),
+    Expr(:call, GlobalRef(Base, :getfield), SSAValue(1), 1), # Will throw
+    ReturnNode(1)
+]
+    ir = make_ircode(code; ssavaluetypes = Any[ImmutableRef{Any}, Any, Any, Any], slottypes=Any[Bool], verify=true)
+    ir = Core.Compiler.sroa_pass!(ir)
+    @test Core.Compiler.verify_ir(ir) === nothing
+    @test !any(iscall((ir, getfield)), ir.stmts.stmt)
+    @test length(ir.cfg.blocks[end].stmts) == 1
+end