Fix DFS tree construction

It's called a Depth-First tree after all, not a
dept-first-but-feel-free-to-record-any-of-the-parents tree. In
particular, in order for the semi-dominator condition to hold,
the parent in the DFS tree needs to be the predecessor with the
highest DFS number. Here we were accidentally doing the opposite
casuing us to look at the wrong node in case the sdom and the idom
are not the same. To understand #31121, consider the following
CFG (minimized from the bug report to show the issue), as well as
the corresponding DFS numbering and sdom assignment

```
     CFG     DFS     sdom

      A       1      0
      | \     | \    |\
      B C     2 5    1 1
     /|/     /|/    /|/
    | D     | 3    | 1
     \|      \|     \|
      E       4      2
```

This bug caused us to record the parent of `E` as `B`, when it should
have been `D` (the relevant invariant here is that the parent in the DFS
tree is the predecessor with the highest DFS number).
As a result, when computing idoms from the sdoms, we
were incorrectly looking at `B`, seeing that the sdom matched the
ancestor in the DFS tree and thus concluding that `E`'s idom was `B`
rather than `A`. As a result, we neglected to insert a phi node in
`E`.

Fixes #31121

base/compiler/ssair/domtree.jl

@@ -177,13 +177,11 @@ begin
         parent = 0
         while !isempty(worklist)
             (parent, current_node) = pop!(worklist)
+            dfs.reverse[current_node] != 0 && continue
             dfs.reverse[current_node] = dfs_num
             dfs.numbering[dfs_num] = current_node
             dfs.parents[dfs_num] = parent
             for succ in cfg.blocks[current_node].succs
-                dfs.reverse[succ] != 0 && continue
-                # Mark things that are currently in the worklist
-                dfs.reverse[succ] = 1
                 push!(worklist, (dfs_num, succ))
             end
             dfs_num += 1
@@ -239,7 +237,7 @@ begin
         # SLT would, but never simultaneously, so we could still
         # do this.
         ancestors = D.parents
-        for w ∈ reverse(_drop(preorder(D), 1))
+        for w::DFSNumber ∈ reverse(_drop(preorder(D), 1))
             # LLVM initializes this to the parent, the paper initializes this to
             # `w`, but it doesn't really matter (the parent is a predecessor,
             # so at worst we'll discover it below). Save a memory reference here.

test/compiler/ssair.jl

@@ -27,6 +27,53 @@ const Compiler = Core.Compiler
 #    # XXX: missing @test
 #end
 
+# Issue #31121
+using .Compiler: CFG, BasicBlock
+
+# We have the following CFG and corresponding DFS numbering:
+#
+#     CFG     DFS
+#
+#      A       1
+#      | \     | \
+#      B C     2 5
+#     /|/     /|/
+#    | D     | 3
+#     \|      \|
+#      E       4
+#
+# In the bug `E` got the wrong dominator (`B` instead of `A`), because the DFS
+# tree had the wrong parent (i.e. we recorded the parent of `4` as `2` rather
+# than `3`, so the idom search missed that `1` is `3`'s semi-dominator). Here
+# we manually construct that CFG and verify that the DFS records the correct
+# parent.
+make_bb(preds, succs) = BasicBlock(Compiler.StmtRange(0, 0), preds, succs)
+let cfg = CFG(BasicBlock[
+    make_bb([]     , [2, 3]),
+    make_bb([1]    , [4, 5]),
+    make_bb([1]    , [4]   ),
+    make_bb([2, 3] , [5]   ),
+    make_bb([2, 4] , []    ),
+], Int[])
+    dfs = Compiler.DFS(cfg, Compiler.BBNumber(1))
+    @test dfs.numbering[dfs.parents[dfs.reverse[5]]] == 4
+    let correct_idoms = Compiler.naive_idoms(cfg)
+        @test Compiler.SNCA(cfg) == correct_idoms
+        # For completeness, reverse the order of pred/succ in the CFG and verify
+        # the answer doesn't change (it does change the which node is chosen
+        # as the semi-dominator, since it changes the DFS numbering).
+        for (a, b, c, d) in Iterators.product(((true, false) for _ = 1:4)...)
+            let cfg′ = Compiler.copy(cfg)
+                a && reverse!(cfg′.blocks[1].succs)
+                b && reverse!(cfg′.blocks[2].succs)
+                c && reverse!(cfg′.blocks[4].preds)
+                d && reverse!(cfg′.blocks[5].preds)
+                @test Compiler.SNCA(cfg′) == correct_idoms
+            end
+        end
+    end
+end
+
 # test >:
 let
     f(a, b) = a >: b