[NewOptimizer] Backport reverse affinity changes from #26778

In #26778, I changed the way that the reverse affinity flag works
in order to support node insertion during compaction. In particular,
I had originally (for ease of implementation primarily) had that flag
simply change which BB an inserted node belongs to (i.e. if that flag
was set and there was a basic block boundary at the insertion site,
the new node would belong to the previous BB). In #26778, I changed
this to instead be a flag of whether to insert before or after a given
instruction. As it turns out, this change is also required for correctness.
Because domsorting can change the notion of "previous instruction" by
moving basic blocks, we had (in rare circumstances) instructions end
up in the wrong BB. Backporting those changes fixes that.

base/compiler/ssair/ir.jl

@@ -146,8 +146,18 @@ function first_insert_for_bb(code, cfg::CFG, block::Int)
  end
 end
 
-
-const NewNode = Tuple{Int, #= reverse affinity =#Bool, Any, Any, #=LineNumber=#Int}
+struct NewNode
+ # Insertion position (interpretation depends on which array this is in)
+ pos::Int
+ # Place the new instruction after this instruction (but in the same BB if this is an implicit terminator)
+ attach_after::Bool
+ # The type of the instruction to insert
+ typ::Any
+ # The node itself
+ node::Any
+ # The index into the line number table of this entry
+ line::Int
+end
 
 struct IRCode
  stmts::Vector{Any}
@@ -174,7 +184,7 @@ function getindex(x::IRCode, s::SSAValue)
  if s.id <= length(x.stmts)
  return x.stmts[s.id]
  else
- return x.new_nodes[s.id - length(x.stmts)][4]
+ return x.new_nodes[s.id - length(x.stmts)].node
  end
 end
 
@@ -374,9 +384,9 @@ function foreachssa(f, @nospecialize(stmt))
  end
 end
 
-function insert_node!(ir::IRCode, pos::Int, @nospecialize(typ), @nospecialize(val), reverse_affinity::Bool=false)
+function insert_node!(ir::IRCode, pos::Int, @nospecialize(typ), @nospecialize(val), attach_after::Bool=false)
  line = ir.lines[pos]
- push!(ir.new_nodes, (pos, reverse_affinity, typ, val, line))
+ push!(ir.new_nodes, NewNode(pos, attach_after, typ, val, line))
  return SSAValue(length(ir.stmts) + length(ir.new_nodes))
 end
 
@@ -407,8 +417,8 @@ mutable struct IncrementalCompact
  result_idx::Int
  active_result_bb::Int
  function IncrementalCompact(code::IRCode)
- # Sort by position with reverse affinity nodes before regular ones
- perm = my_sortperm(Int[(code.new_nodes[i][1]*2 + Int(!code.new_nodes[i][2])) for i in 1:length(code.new_nodes)])
+ # Sort by position with attach after nodes affter regular ones
+ perm = my_sortperm(Int[(code.new_nodes[i].pos*2 + Int(code.new_nodes[i].attach_after)) for i in 1:length(code.new_nodes)])
  new_len = length(code.stmts) + length(code.new_nodes)
  result = Array{Any}(undef, new_len)
  result_types = Array{Any}(undef, new_len)
@@ -422,7 +432,7 @@ mutable struct IncrementalCompact
 
  # For inlining
  function IncrementalCompact(parent::IncrementalCompact, code::IRCode, result_offset)
- perm = my_sortperm(Int[code.new_nodes[i][1] for i in 1:length(code.new_nodes)])
+ perm = my_sortperm(Int[code.new_nodes[i].pos for i in 1:length(code.new_nodes)])
  new_len = length(code.stmts) + length(code.new_nodes)
  ssa_rename = Any[SSAValue(i) for i = 1:new_len]
  used_ssas = fill(0, new_len)
@@ -509,7 +519,7 @@ function getindex(view::TypesView, idx)
  if idx <= length(ir.types)
  return ir.types[idx]
  else
- return ir.new_nodes[idx - length(ir.types)][3]
+ return ir.new_nodes[idx - length(ir.types)].typ
  end
 end
 
@@ -603,10 +613,27 @@ function finish_current_bb!(compact, old_result_idx=compact.result_idx)
  end
 end
 
-function reverse_affinity_stmt_after(compact::IncrementalCompact, idx::Int)
+function attach_after_stmt_after(compact::IncrementalCompact, idx::Int)
  compact.new_nodes_idx > length(compact.perm) && return false
  entry = compact.ir.new_nodes[compact.perm[compact.new_nodes_idx]]
- entry[1] == idx + 1 && entry[2]
+ entry.pos == idx && entry.attach_after
+end
+
+function process_newnode!(compact, new_idx, new_node_entry, idx, active_bb)
+ old_result_idx = compact.result_idx
+ bb = compact.ir.cfg.blocks[active_bb]
+ compact.result_types[old_result_idx] = new_node_entry.typ
+ compact.result_lines[old_result_idx] = new_node_entry.line
+ result_idx = process_node!(compact, old_result_idx, new_node_entry.node, new_idx, idx)
+ compact.result_idx = result_idx
+ # If this instruction has reverse affinity and we were at the end of a basic block,
+ # finish it now.
+ if new_node_entry.attach_after && idx == last(bb.stmts)+1 && !attach_after_stmt_after(compact, idx-1)
+ active_bb += 1
+ finish_current_bb!(compact, old_result_idx)
+ end
+ (old_result_idx == result_idx) && return next(compact, (idx, active_bb))
+ return Pair{Int, Any}(old_result_idx, compact.result[old_result_idx]), (compact.idx, active_bb)
 end
 
 function next(compact::IncrementalCompact, (idx, active_bb)::Tuple{Int, Int})
@@ -615,24 +642,14 @@ function next(compact::IncrementalCompact, (idx, active_bb)::Tuple{Int, Int})
  resize!(compact, old_result_idx)
  end
  bb = compact.ir.cfg.blocks[active_bb]
- if compact.new_nodes_idx <= length(compact.perm) && compact.ir.new_nodes[compact.perm[compact.new_nodes_idx]][1] == idx
+ if compact.new_nodes_idx <= length(compact.perm) &&
+ (entry = compact.ir.new_nodes[compact.perm[compact.new_nodes_idx]];
+ entry.attach_after ? entry.pos == idx - 1 : entry.pos == idx)
  new_idx = compact.perm[compact.new_nodes_idx]
  compact.new_nodes_idx += 1
- _, reverse_affinity, typ, new_node, new_line = compact.ir.new_nodes[new_idx]
+ new_node_entry = compact.ir.new_nodes[new_idx]
  new_idx += length(compact.ir.stmts)
- compact.result_types[old_result_idx] = typ
- compact.result_lines[old_result_idx] = new_line
- compact.result_flags[old_result_idx] = 0x00
- result_idx = process_node!(compact, old_result_idx, new_node, new_idx, idx)
- compact.result_idx = result_idx
- # If this instruction has reverse affinity and we were at the end of a basic block,
- # finish it now.
- if reverse_affinity && idx == last(bb.stmts)+1 && !reverse_affinity_stmt_after(compact, idx-1)
- active_bb += 1
- finish_current_bb!(compact, old_result_idx)
- end
- (old_result_idx == result_idx) && return next(compact, (idx, active_bb))
- return Pair{Int, Any}(old_result_idx, compact.result[old_result_idx]), (compact.idx, active_bb)
+ return process_newnode!(compact, new_idx, new_node_entry, idx, active_bb)
  end
  # This will get overwritten in future iterations if
  # result_idx is not, incremented, but that's ok and expected
@@ -642,7 +659,7 @@ function next(compact::IncrementalCompact, (idx, active_bb)::Tuple{Int, Int})
  result_idx = process_node!(compact, old_result_idx, compact.ir.stmts[idx], idx, idx)
  stmt_if_any = old_result_idx == result_idx ? nothing : compact.result[old_result_idx]
  compact.result_idx = result_idx
- if idx == last(bb.stmts) && !reverse_affinity_stmt_after(compact, idx)
+ if idx == last(bb.stmts) && !attach_after_stmt_after(compact, idx)
  active_bb += 1
  finish_current_bb!(compact, old_result_idx)
  end

base/compiler/ssair/show.jl

@@ -80,7 +80,7 @@ function Base.show(io::IO, code::IRCode)
  used = IdSet{Int}()
  Base.println(io, "Code")
  foreach(stmt->scan_ssa_use!(used, stmt), code.stmts)
- foreach(((_a, _b, node, _d),) -> scan_ssa_use!(used, node), code.new_nodes)
+ foreach(nn -> scan_ssa_use!(used, nn.node), new_nodes)
  if isempty(used)
  maxsize = 0
  else
@@ -118,7 +118,7 @@ function Base.show(io::IO, code::IRCode)
  floop = true
  while !isempty(new_nodes_perm) && code.new_nodes[peek(new_nodes_perm)][1] == idx
  node_idx = popfirst!(new_nodes_perm)
- _, reverse_affinity, typ, node, line = code.new_nodes[node_idx]
+ new_node = code.new_nodes[node_idx]
  node_idx += length(code.stmts)
  if print_sep
  if floop
@@ -129,13 +129,13 @@ function Base.show(io::IO, code::IRCode)
  end
  print_sep = true
  floop = false
- print_ssa_typ = !isa(node, PiNode) && node_idx in used
+ print_ssa_typ = !isa(new_node.node, PiNode) && node_idx in used
  Base.with_output_color(:yellow, io) do io′
- print_node(io′, node_idx, node, used, maxsize; color = false,
- print_typ=!print_ssa_typ || (isa(node, Expr) && typ != node.typ))
+ print_node(io′, node_idx, new_node.node, used, maxsize; color = false,
+ print_typ=!print_ssa_typ || (isa(new_node.node, Expr) && typ != new_node.node.typ))
  end
  if print_ssa_typ
- Base.printstyled(io, "::$(typ)", color=:red)
+ Base.printstyled(io, "::$(new_node.typ)", color=:red)
  end
  Base.println(io)
  end

base/compiler/ssair/slot2ssa.jl

@@ -426,11 +426,15 @@ function domsort_ssa!(ir::IRCode, domtree::DomTree)
  end
  result_stmts = Any[renumber_ssa!(result_stmts[i], inst_rename, true) for i in 1:length(result_stmts)]
  cfg = CFG(new_bbs, Int[first(bb.stmts) for bb in new_bbs[2:end]])
- new_new_nodes = NewNode[let (pos, reverse_affinity, typ, node, lno) = ir.new_nodes[i]
- isa(node, PhiNode) && (node = rename_phinode_edges(node, 0, result_order, bb_rename))
- node = renumber_ssa!(node, inst_rename, true)
- (inst_rename[pos].id, reverse_affinity, typ, node, lno)
- end for i in 1:length(ir.new_nodes)]
+ new_new_nodes = Vector{NewNode}(undef, length(ir.new_nodes))
+ for i = 1:length(ir.new_nodes)
+ entry = ir.new_nodes[i]
+ new_new_nodes[i] = NewNode(inst_rename[entry.pos].id, entry.attach_after, entry.typ,
+ renumber_ssa!(isa(entry.node, PhiNode) ?
+ rename_phinode_edges(entry.node, 0, result_order, bb_rename) : entry.node,
+ inst_rename, true),
+ entry.line)
+ end
  new_ir = IRCode(ir, result_stmts, result_types, result_ltable, result_flags, cfg, new_new_nodes)
  return new_ir
 end
@@ -602,12 +606,12 @@ function construct_ssa!(ci::CodeInfo, code::Vector{Any}, ir::IRCode, domtree::Do
  end
  typ = incoming_val == undef_token ? MaybeUndef(Union{}) : typ_for_val(incoming_val, ci)
  new_node_id = ssaval - length(ir.stmts)
- old_insert, reverse_affinity, old_typ, _, old_line = ir.new_nodes[new_node_id]
+ old_entry = ir.new_nodes[new_node_id]
  if isa(typ, DelayedTyp)
  push!(type_refine_phi, ssaval)
  end
- new_typ = isa(typ, DelayedTyp) ? Union{} : tmerge(old_typ, typ)
- ir.new_nodes[new_node_id] = (old_insert, reverse_affinity, new_typ, node, old_line)
+ new_typ = isa(typ, DelayedTyp) ? Union{} : tmerge(old_entry.typ, typ)
+ ir.new_nodes[new_node_id] = NewNode(old_entry.pos, old_entry.attach_after, new_typ, node, old_entry.line)
  incoming_vals[slot] = NewSSAValue(ssaval)
  end
  (item in visited) && continue
@@ -666,7 +670,7 @@ function construct_ssa!(ci::CodeInfo, code::Vector{Any}, ir::IRCode, domtree::Do
  typ = MaybeUndef(Union{})
  end
  push!(phicnodes[exc][cidx][3].values,
- NewSSAValue(insert_node!(ir, idx+1, typ, node, true).id))
+ NewSSAValue(insert_node!(ir, idx, typ, node, true).id))
  end
  end
  end
@@ -717,16 +721,16 @@ function construct_ssa!(ci::CodeInfo, code::Vector{Any}, ir::IRCode, domtree::Do
  for (_, ssa, node) in nodes
  new_typ = Union{}
  new_idx = ssa.id - length(ir.stmts)
- old_insert, reverse_affinity, old_typ, _, old_line = ir.new_nodes[new_idx]
- for i = 1:length(node.values)
- orig_typ = typ = typ_for_val(node.values[i], ci)
+ node = ir.new_nodes[new_idx]
+ for i = 1:length(node.node.values)
+ orig_typ = typ = typ_for_val(node.node.values[i], ci)
  @assert !isa(typ, MaybeUndef)
  while isa(typ, DelayedTyp)
- typ = ir.new_nodes[typ.phi.id - length(ir.stmts)][3]
+ typ = ir.new_nodes[typ.phi.id - length(ir.stmts)].typ
  end
  new_typ = tmerge(new_typ, typ)
  end
- ir.new_nodes[new_idx] = (old_insert, reverse_affinity, new_typ, node, old_line)
+ ir.new_nodes[new_idx] = NewNode(node.pos, node.attach_after, new_typ, node.node, node.line)
  end
  end
  # This is a bit awkward, because it basically duplicates what type
@@ -737,42 +741,44 @@ function construct_ssa!(ci::CodeInfo, code::Vector{Any}, ir::IRCode, domtree::Do
  changed = false
  for phi in type_refine_phi
  new_idx = phi - length(ir.stmts)
- old_insert, reverse_affinity, old_typ, node, old_line = ir.new_nodes[new_idx]
+ node = ir.new_nodes[new_idx]
  new_typ = Union{}
- for i = 1:length(node.values)
- if !isassigned(node.values, i)
+ for i = 1:length(node.node.values)
+ if !isassigned(node.node.values, i)
  if !isa(new_typ, MaybeUndef)
  new_typ = MaybeUndef(new_typ)
  end
  continue
  end
- typ = typ_for_val(node.values[i], ci)
+ typ = typ_for_val(node.node.values[i], ci)
  was_maybe_undef = false
  if isa(typ, MaybeUndef)
  typ = typ.typ
  was_maybe_undef = true
  end
  @assert !isa(typ, MaybeUndef)
  while isa(typ, DelayedTyp)
- typ = ir.new_nodes[typ.phi.id - length(ir.stmts)][3]
+ typ = ir.new_nodes[typ.phi.id - length(ir.stmts)].typ
  end
  new_typ = tmerge(new_typ, was_maybe_undef ? MaybeUndef(typ) : typ)
  end
- if !(old_typ ⊑ new_typ) || !(new_typ ⊑ old_typ)
- ir.new_nodes[new_idx] = (old_insert, reverse_affinity, new_typ, node, old_line)
+ if !(node.typ ⊑ new_typ) || !(new_typ ⊑ node.typ)
+ ir.new_nodes[new_idx] = NewNode(node.pos, node.attach_after, new_typ, node.node, node.line)
  changed = true
  end
  end
  end
- types = Any[isa(types[i], DelayedTyp) ? ir.new_nodes[types[i].phi.id - length(ir.stmts)][2] : types[i] for i in 1:length(types)]
- new_nodes = NewNode[let (pos, reverse_affinity, typ, node, line) = ir.new_nodes[i]
- typ = isa(typ, DelayedTyp) ? ir.new_nodes[typ.phi.id - length(ir.stmts)][2] : typ
- (pos, reverse_affinity, typ, node, line)
+ types = Any[isa(types[i], DelayedTyp) ? ir.new_nodes[types[i].phi.id - length(ir.stmts)].typ : types[i] for i in 1:length(types)]
+ new_nodes = NewNode[let node = ir.new_nodes[i]
+ typ = isa(node.typ, DelayedTyp) ? ir.new_nodes[node.typ.phi.id - length(ir.stmts)].typ : node.typ
+ NewNode(node.pos, node.attach_after, typ, node.node, node.line)
  end for i in 1:length(ir.new_nodes)]
  # Renumber SSA values
  new_code = Any[new_to_regular(renumber_ssa!(new_code[i], ssavalmap)) for i in 1:length(new_code)]
- new_nodes = NewNode[let (pt, reverse_affinity, typ, stmt, line) = new_nodes[i]
- (pt, reverse_affinity, typ, new_to_regular(renumber_ssa!(stmt, ssavalmap)), line)
+ new_nodes = NewNode[let node = new_nodes[i]
+ NewNode(node.pos, node.attach_after, node.typ,
+ new_to_regular(renumber_ssa!(node.node, ssavalmap)),
+ node.line)
  end for i in 1:length(new_nodes)]
  ir = IRCode(ir, new_code, types, ir.lines, ir.flags, ir.cfg, new_nodes)
  @timeit "domsort" ir = domsort_ssa!(ir, domtree)

base/compiler/ssair/verify.jl

@@ -11,13 +11,13 @@ end
 function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int, use_idx::Int)
  if isa(op, SSAValue)
  if op.id > length(ir.stmts)
- def_bb = block_for_inst(ir.cfg, ir.new_nodes[op.id - length(ir.stmts)][1])
+ def_bb = block_for_inst(ir.cfg, ir.new_nodes[op.id - length(ir.stmts)].pos)
  else
  def_bb = block_for_inst(ir.cfg, op.id)
  end
  if (def_bb == use_bb)
  if op.id > length(ir.stmts)
- @assert ir.new_nodes[op.id - length(ir.stmts)][1] <= use_idx
+ @assert ir.new_nodes[op.id - length(ir.stmts)].pos <= use_idx
  else
  @assert op.id < use_idx
  end