Fix DFS tree construction

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