Fix yet more cfg_simplify bugs (JuliaLang#47286)

I continue to throw cfg_simplify at big external IR examples and
unfortunately, I continue finding bugs. Hopefully this is the last
of it. The particular bug here had to do with scheduling fallthroughs
properly across dropped basic blocks. The test gives an example,
but it's pretty hard to hit this case.

base/compiler/ssair/domtree.jl

@@ -162,6 +162,12 @@ function DFS!(D::DFSTree, blocks::Vector{BasicBlock}, is_post_dominator::Bool)
 
  # Push children to the stack
  for succ_bb in edges
+ if succ_bb == 0
+ # Edge 0 indicates an error entry, but shouldn't affect
+ # the post-dominator tree.
+ @assert is_post_dominator
+ continue
+ end
  push!(to_visit, (succ_bb, pre_num, false))
  end
 

base/compiler/ssair/passes.jl

@@ -1850,18 +1850,40 @@ function is_legal_bb_drop(ir::IRCode, bbidx::Int, bb::BasicBlock)
  stmt === nothing && return true
  ((stmt::GotoNode).label == bbidx + 1) && return true
  end
- # Otherwise make sure we're not the fallthrough case of any predecessor
- for pred in bb.preds
- if pred == bbidx - 1
- terminator = ir[SSAValue(first(bb.stmts)-1)][:inst]
- if isa(terminator, GotoIfNot)
- if terminator.dest != bbidx
- return false
- end
+ return true
+end
+
+function legalize_bb_drop_pred!(ir::IRCode, bb::BasicBlock, bbidx::Int, bbs::Vector{BasicBlock}, dropped_bbs::Vector{Int})
+ (bbidx-1) in bb.preds || return true
+ last_fallthrough = bbidx-1
+ dbi = length(dropped_bbs)
+ while dbi != 0 && dropped_bbs[dbi] == last_fallthrough && (last_fallthrough-1 in bbs[last_fallthrough].preds)
+ last_fallthrough -= 1
+ dbi -= 1
+ end
+ last_fallthrough_term_ssa = SSAValue(last(bbs[last_fallthrough].stmts))
+ terminator = ir[last_fallthrough_term_ssa][:inst]
+ if isa(terminator, GotoIfNot)
+ if terminator.dest != bbidx
+ # The previous terminator's destination matches our fallthrough.
+ # If we're also a fallthrough terminator, then we just have
+ # to delete the GotoIfNot.
+ our_terminator = ir[SSAValue(last(bb.stmts))][:inst]
+ if terminator.dest != (isa(our_terminator, GotoNode) ? our_terminator.label : bbidx + 1)
+ return false
  end
- break
  end
+ ir[last_fallthrough_term_ssa] = nothing
+ kill_edge!(bbs, last_fallthrough, terminator.dest)
+ elseif isexpr(terminator, :enter)
+ return false
+ elseif isa(terminator, GotoNode)
+ return true
  end
+ # Hack, but effective. If we have a predecessor with a fall-through terminator, change the
+ # instruction numbering to merge the blocks now such that below processing will properly
+ # update it.
+ bbs[last_fallthrough] = BasicBlock(first(bbs[last_fallthrough].stmts):last(bb.stmts), bbs[last_fallthrough].preds, bbs[last_fallthrough].succs)
  return true
 end
 
@@ -1927,24 +1949,9 @@ function cfg_simplify!(ir::IRCode)
  end
  end
  @label done
- if !found_interference
- # Hack, but effective. If we have a predecessor with a fall-through terminator, change the
- # instruction numbering to merge the blocks now such that below processing will properly
- # update it.
- if idx-1 in bb.preds
- last_fallthrough = idx-1
- dbi = length(dropped_bbs)
- while dbi != 0 && dropped_bbs[dbi] == last_fallthrough && (last_fallthrough-1 in bbs[last_fallthrough].preds)
- last_fallthrough -= 1
- dbi -= 1
- end
- terminator = ir[SSAValue(last(bbs[last_fallthrough].stmts))][:inst]
- if !is_terminator(terminator)
- bbs[last_fallthrough] = BasicBlock(first(bbs[last_fallthrough].stmts):last(bb.stmts), bbs[last_fallthrough].preds, bbs[last_fallthrough].succs)
- end
- end
- push!(dropped_bbs, idx)
- end
+ found_interference && continue
+ legalize_bb_drop_pred!(ir, bb, idx, bbs, dropped_bbs) || continue
+ push!(dropped_bbs, idx)
  end
  end
  end
@@ -1990,11 +1997,14 @@ function cfg_simplify!(ir::IRCode)
  push!(worklist, terminator.dest)
  end
  elseif isexpr(terminator, :enter)
- push!(worklist, terminator.args[1])
+ if bb_rename_succ[terminator.args[1]] == 0
+ push!(worklist, terminator.args[1])
+ end
  end
  ncurr = curr + 1
- if !isempty(searchsorted(dropped_bbs, ncurr))
- break
+ while !isempty(searchsorted(dropped_bbs, ncurr))
+ bb_rename_succ[ncurr] = -bbs[ncurr].succs[1]
+ ncurr += 1
  end
  curr = ncurr
  end
@@ -2146,7 +2156,7 @@ function cfg_simplify!(ir::IRCode)
  if new_bb.succs[1] == new_bb.succs[2]
  old_bb2 = findfirst(x->x==bbidx, bb_rename_pred)
  terminator = ir[SSAValue(last(bbs[old_bb2].stmts))]
- @assert isa(terminator[:inst], GotoIfNot)
+ @assert terminator[:inst] isa GotoIfNot
  # N.B.: The dest will be renamed in process_node! below
  terminator[:inst] = GotoNode(terminator[:inst].dest)
  pop!(new_bb.succs)

test/compiler/irpasses.jl

@@ -743,6 +743,94 @@ let m = Meta.@lower 1 + 1
  @test length(ir.cfg.blocks) == 1 && Core.Compiler.length(ir.stmts) == 1
 end
 
+# Test cfg_simplify in complicated sequences of dropped and merged bbs
+using Core.Compiler: Argument, IRCode, GotoNode, GotoIfNot, ReturnNode, NoCallInfo, BasicBlock, StmtRange, SSAValue
+bb_term(ir, bb) = Core.Compiler.getindex(ir, SSAValue(Core.Compiler.last(ir.cfg.blocks[bb].stmts)))[:inst]
+
+function each_stmt_a_bb(stmts, preds, succs)
+ ir = IRCode()
+ empty!(ir.stmts.inst)
+ append!(ir.stmts.inst, stmts)
+ empty!(ir.stmts.type); append!(ir.stmts.type, [Nothing for _ = 1:length(stmts)])
+ empty!(ir.stmts.flag); append!(ir.stmts.flag, [0x0 for _ = 1:length(stmts)])
+ empty!(ir.stmts.line); append!(ir.stmts.line, [Int32(0) for _ = 1:length(stmts)])
+ empty!(ir.stmts.info); append!(ir.stmts.info, [NoCallInfo() for _ = 1:length(stmts)])
+ empty!(ir.cfg.blocks); append!(ir.cfg.blocks, [BasicBlock(StmtRange(i, i), preds[i], succs[i]) for i = 1:length(stmts)])
+ Core.Compiler.verify_ir(ir)
+ return ir
+end
+
+for gotoifnot in (false, true)
+ stmts = [
+ # BB 1
+ GotoIfNot(Argument(1), 8),
+ # BB 2
+ GotoIfNot(Argument(2), 4),
+ # BB 3
+ GotoNode(9),
+ # BB 4
+ GotoIfNot(Argument(3), 10),
+ # BB 5
+ GotoIfNot(Argument(4), 11),
+ # BB 6
+ GotoIfNot(Argument(5), 12),
+ # BB 7
+ GotoNode(13),
+ # BB 8
+ ReturnNode(1),
+ # BB 9
+ nothing,
+ # BB 10
+ nothing,
+ # BB 11
+ gotoifnot ? GotoIfNot(Argument(6), 13) : GotoNode(13),
+ # BB 12
+ ReturnNode(2),
+ # BB 13
+ ReturnNode(3),
+ ]
+ preds = Vector{Int}[Int[], [1], [2], [2], [4], [5], [6], [1], [3], [4, 9], [5, 10], gotoifnot ? [6,11] : [6], [7, 11]]
+ succs = Vector{Int}[[2, 8], [3, 4], [9], [5, 10], [6, 11], [7, 12], [13], Int[], [10], [11], gotoifnot ? [12, 13] : [13], Int[], Int[]]
+ ir = each_stmt_a_bb(stmts, preds, succs)
+ ir = Core.Compiler.cfg_simplify!(ir)
+ Core.Compiler.verify_ir(ir)
+
+ if gotoifnot
+ let term4 = bb_term(ir, 4), term5 = bb_term(ir, 5)
+ @test isa(term4, GotoIfNot) && bb_term(ir, term4.dest).val == 3
+ @test isa(term5, ReturnNode) && term5.val == 2
+ end
+ else
+ @test length(ir.cfg.blocks) == 10
+ let term = bb_term(ir, 3)
+ @test isa(term, GotoNode) && bb_term(ir, term.label).val == 3
+ end
+ end
+end
+
+let stmts = [
+ # BB 1
+ GotoIfNot(Argument(1), 4),
+ # BB 2
+ GotoIfNot(Argument(2), 5),
+ # BB 3
+ GotoNode(5),
+ # BB 4
+ ReturnNode(1),
+ # BB 5
+ ReturnNode(2)
+ ]
+ preds = Vector{Int}[Int[], [1], [2], [1], [2, 3]]
+ succs = Vector{Int}[[2, 4], [3, 5], [5], Int[], Int[]]
+ ir = each_stmt_a_bb(stmts, preds, succs)
+ ir = Core.Compiler.cfg_simplify!(ir)
+ Core.Compiler.verify_ir(ir)
+
+ @test length(ir.cfg.blocks) == 4
+ terms = map(i->bb_term(ir, i), 1:length(ir.cfg.blocks))
+ @test Set(term.val for term in terms if isa(term, ReturnNode)) == Set([1,2])
+end
+
 let m = Meta.@lower 1 + 1
  # Test that CFG simplify doesn't mess up when chaining past return blocks
  @assert Meta.isexpr(m, :thunk)