Update domtrees dynamically when CFGs get modified, and fix bug in dy…

…namic domtree algorithm

base/compiler/ssair/domtree.jl

@@ -71,7 +71,7 @@ const BBNumber = Int
 const PreNumber = Int
 const PostNumber = Int
 
-struct DFSTree
+mutable struct DFSTree
     # These map between BB number and pre- or postorder numbers
     to_pre::Vector{PreNumber}
     from_pre::Vector{BBNumber}
@@ -258,9 +258,27 @@ function SNCA!(domtree::DomTree, blocks::Vector{BasicBlock}, max_pre::PreNumber)
     # we wanted to.
     resize!(state, n_nodes)
     for w in 1:max_pre
+        # Only reset semidominators for nodes we want to recompute
         state[w] = SNCAData(typemax(PreNumber), w)
     end
 
+    # If we are only recomputing some of the semidominators, the remaining
+    # labels should be reset, because they may have become inapplicable to the
+    # node/semidominator we are currently processing/recomputing. They can
+    # become inapplicable because of path compressions that were triggered by
+    # nodes that should only be processed after the current one (but were
+    # processed the last time `SNCA!` was run).
+    #
+    # So, for every node that is not being reprocessed, we reset its label to
+    # its semidominator, which is the value that its label assumes once its
+    # semidominator is computed. If this was too conservative, i.e. if the
+    # label would have been updated before we process the current node in a
+    # situation where all semidominators were recomputed, then path compression
+    # will produce the correct label.
+    for w in max_pre+1:n_nodes
+        state[w].label = state[w].semi
+    end
+
     # Calculate semidominators, but only for blocks with preorder number up to
     # max_pre
     ancestors = copy(D.to_parent_pre)
@@ -372,6 +390,11 @@ end
 "Given updated blocks, update the given dominator tree with an inserted edge."
 function domtree_insert_edge!(domtree::DomTree, blocks::Vector{BasicBlock},
                               from::BBNumber, to::BBNumber)
+    # `from` is unreachable, so `from` and `to` aren't in domtree
+    if bb_unreachable(domtree, from)
+        return domtree
+    end
+
     # Implements Section 3.1 of [GI16]
     dt        = domtree.dfs_tree
     from_pre  = dt.to_pre[from]
@@ -393,6 +416,11 @@ end
 "Given updated blocks, update the given dominator tree with a deleted edge."
 function domtree_delete_edge!(domtree::DomTree, blocks::Vector{BasicBlock},
                               from::BBNumber, to::BBNumber)
+    # `from` is unreachable, so `from` and `to` aren't in domtree
+    if bb_unreachable(domtree, from)
+        return domtree
+    end
+
     # Implements Section 3.1 of [GI16]
     if is_parent(domtree.dfs_tree, from, to)
         # The `from` block is the parent of the `to` block in the DFS tree, so
@@ -445,6 +473,76 @@ function on_semidominator_path(domtree::DomTree, x::BBNumber, y::BBNumber)
     return false
 end
 
+"""
+Rename basic block numbers in a dominator tree, removing the block if it is
+renamed to -1.
+"""
+function rename_nodes!(domtree::DomTree, rename_bb::Vector{BBNumber})
+    # Rename DFS tree
+    rename_nodes!(domtree.dfs_tree, rename_bb)
+
+    # `snca_state` is indexed by preorder number, so should be unchanged
+
+    # Rename `idoms_bb` and `nodes`
+    new_idoms_bb = zeros(BBNumber, length(domtree.idoms_bb))
+    new_nodes = Vector{DomTreeNode}(undef, length(domtree.nodes))
+    for (old_bb, new_bb) in enumerate(rename_bb)
+        if new_bb != -1
+            new_idoms_bb[new_bb] = (new_bb == 1) ?
+                0 : rename_bb[domtree.idoms_bb[old_bb]]
+            new_nodes[new_bb] = domtree.nodes[old_bb]
+            map!(i -> rename_bb[i],
+                 new_nodes[new_bb].children,
+                 new_nodes[new_bb].children)
+        end
+    end
+
+    # length of `to_pre` after renaming DFS tree is new number of basic blocks
+    resize!(new_idoms_bb, length(domtree.dfs_tree.to_pre)) # maybe?
+    resize!(new_nodes, length(domtree.dfs_tree.to_pre))
+
+    domtree.idoms_bb = new_idoms_bb
+    domtree.nodes = new_nodes
+    return domtree
+end
+
+"""
+Rename basic block numbers in a DFS tree, removing the block if it is renamed
+to -1.
+"""
+function rename_nodes!(D::DFSTree, rename_bb::Vector{BBNumber})
+    n_blocks = length(D.to_pre)
+    n_reachable_blocks = length(D.from_pre)
+
+    new_to_pre = zeros(PreNumber, n_blocks)
+    new_from_pre = Vector{BBNumber}(undef, n_reachable_blocks)
+    new_to_post = zeros(PostNumber, n_blocks)
+    new_from_post = Vector{BBNumber}(undef, n_reachable_blocks)
+    max_new_bb = 0
+    for (old_bb, new_bb) in enumerate(rename_bb)
+        if new_bb != -1
+            new_to_pre[new_bb] = D.to_pre[old_bb]
+            new_from_pre[D.to_pre[old_bb]] = new_bb
+            new_to_post[new_bb] = D.to_post[old_bb]
+            new_from_post[D.to_post[old_bb]] = new_bb
+
+            # Keep track of highest BB number to resize arrays with
+            if new_bb > max_new_bb
+                max_new_bb = new_bb
+            end
+        end
+    end
+    resize!(new_to_pre, max_new_bb)
+    resize!(new_to_post, max_new_bb)
+
+    D.to_pre = new_to_pre
+    D.from_pre = new_from_pre
+    D.to_post = new_to_post
+    D.from_post = new_from_post
+    # `to_parent_pre` should be unchanged
+    return D
+end
+
 """
 Checks if bb1 dominates bb2.
 bb1 and bb2 are indexes into the CFG blocks.

base/compiler/ssair/driver.jl

@@ -110,7 +110,6 @@ end
 function run_passes(ci::CodeInfo, nargs::Int, sv::OptimizationState)
     ir = just_construct_ssa(ci, copy_exprargs(ci.code), nargs, sv)
     #@Base.show ("after_construct", ir)
-    # TODO: Domsorting can produce an updated domtree - no need to recompute here
     @timeit "compact 1" ir = compact!(ir)
     @timeit "Inlining" ir = ssa_inlining_pass!(ir, ir.linetable, sv)
     #@timeit "verify 2" verify_ir(ir)

base/compiler/ssair/inlining.jl

@@ -295,6 +295,7 @@ function finish_cfg_inline!(state::CFGInliningState)
     end
 end
 
+# NOTE: The domtree is not kept up-to-date with changes this makes
 function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector{Any},
                          linetable::Vector{LineInfoNode}, item::InliningTodo,
                          boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
@@ -412,6 +413,7 @@ end
 
 const fatal_type_bound_error = ErrorException("fatal error in type inference (type bound)")
 
+# NOTE: The domtree is not kept up-to-date with changes this makes
 function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
                                argexprs::Vector{Any}, linetable::Vector{LineInfoNode},
                                item::UnionSplit, boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
@@ -494,7 +496,9 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
 end
 
 function batch_inline!(todo::Vector{Any}, ir::IRCode, linetable::Vector{LineInfoNode}, propagate_inbounds::Bool)
-    # Compute the new CFG first (modulo statement ranges, which will be computed below)
+    # Compute the new CFG first (modulo statement ranges, which will be
+    # computed below, and the domtree, which will be updated below before we
+    # iterate through the statements)
     state = CFGInliningState(ir)
     for item in todo
         if isa(item, UnionSplit)
@@ -515,6 +519,9 @@ function batch_inline!(todo::Vector{Any}, ir::IRCode, linetable::Vector{LineInfo
 
     let compact = IncrementalCompact(ir, false)
         compact.result_bbs = state.new_cfg_blocks
+        # Recompute the domtree now that the CFG has been modified
+        compact.result_domtree = construct_domtree(compact.result_bbs)
+
         # This needs to be a minimum and is more of a size hint
         nn = 0
         for item in todo
@@ -568,7 +575,6 @@ function batch_inline!(todo::Vector{Any}, ir::IRCode, linetable::Vector{LineInfo
                 compact[idx] = PhiNode(Any[edge == length(state.bb_rename) ? length(state.new_cfg_blocks) : state.bb_rename[edge+1]-1 for edge in stmt.edges], stmt.values)
             end
         end
-
         ir = finish(compact)
     end
     return ir