Fix phi node value renumbering issue

[martinholters]

If a φ node got placed at a position one of its values referenced, that reference wasn't updated.
Fixes #28077.

[martinholters]

The test passes without this one. I do think the change is correct, but if someone can come up with a piece of code that comes here with val.id==processed_idx to build a test case, that would be appreciated.

[martinholters]

freebsd failure is #28101.
Circle says: "Your current usage represents 78% of your JuliaLang Linux plan's limit." Looking at the queue, that percentage sounds, eh, slightly underestimated.

[Keno]

Thanks. I'm very happy that other people are starting to be able to make changes to this code. I particularly appreciate the test case in this PR. However, I think the real bug here is different. Adding the current statement to the rename table is supposed to happen before renaming of the phinode arguments, so it seemed to me the original code is correct. Instead I think what's happening is that the processed_idx is wrong for newly inserted nodes (passing the old idx of the reference statement instead). I think something like this may work:

diff --git a/base/compiler/ssair/ir.jl b/base/compiler/ssair/ir.jl
index 1db42da..6e3f23b 100644
--- a/base/compiler/ssair/ir.jl
+++ b/base/compiler/ssair/ir.jl
@@ -836,14 +836,16 @@ function iterate(compact::IncrementalCompact, (idx, active_bb)::Tuple{Int, Int}=
         compact.new_nodes_idx += 1
         new_node_entry = compact.ir.new_nodes[new_idx]
         new_idx += length(compact.ir.stmts)
-        return process_newnode!(compact, new_idx, new_node_entry, idx, active_bb, true)
+        return process_newnode!(compact, new_idx, new_node_entry,
+            entry.attach_after ? idx : idx - 1, active_bb, true)
     elseif !isempty(compact.pending_perm) &&
         (entry = compact.pending_nodes[compact.pending_perm[1]];
          entry.attach_after ? entry.pos == idx - 1 : entry.pos == idx)
         new_idx = popfirst!(compact.pending_perm)
         new_node_entry = compact.pending_nodes[new_idx]
         new_idx += length(compact.ir.stmts) + length(compact.ir.new_nodes)
-        return process_newnode!(compact, new_idx, new_node_entry, idx, active_bb, false)
+        return process_newnode!(compact, new_idx, new_node_entry,
+            entry.attach_after ? idx : idx - 1, active_bb, false)
     end
     # This will get overwritten in future iterations if
     # result_idx is not, incremented, but that's ok and expected

[martinholters]

Yep, that patch seems to be working. Updated PR accordingly. (I don't fully understand what's happening, so @Keno if you can come up with a more descriptive commit message...)

[Keno]

Actually, that's still incorrect. I think idx-1 is always correct. I'll update the PR.

[martinholters]

The fact that we had three similar, but not equivalent patches that all fixed the issue and did not break any other tests makes me a bit uneasy. Ideally, we would have tests that had failed for the wrong fixes here. I'm clueless how to construct such tests, though.