use ReturnNode, GotoIfNot, and Argument more consistently in IR (#36318)

Author: JeffBezanson

base/boot.jl

@@ -103,6 +103,15 @@
 #    label::Int
 #end
 
+#struct GotoIfNot
+#    cond::Any
+#    dest::Int
+#end
+
+#struct ReturnNode
+#    val::Any
+#end
+
 #struct PiNode
 #    val
 #    typ
@@ -368,6 +377,10 @@ _new(:GotoNode, :Int)
 _new(:NewvarNode, :SlotNumber)
 _new(:QuoteNode, :Any)
 _new(:SSAValue, :Int)
+_new(:Argument, :Int)
+_new(:ReturnNode, :Any)
+eval(Core, :(ReturnNode() = $(Expr(:new, :ReturnNode)))) # unassigned val indicates unreachable
+eval(Core, :(GotoIfNot(@nospecialize(cond), dest::Int) = $(Expr(:new, :GotoIfNot, :cond, :dest))))
 eval(Core, :(LineNumberNode(l::Int) = $(Expr(:new, :LineNumberNode, :l, nothing))))
 eval(Core, :(LineNumberNode(l::Int, @nospecialize(f)) = $(Expr(:new, :LineNumberNode, :l, :f))))
 LineNumberNode(l::Int, f::String) = LineNumberNode(l, Symbol(f))
@@ -453,12 +466,12 @@ Symbol(s::Symbol) = s
 
 # module providing the IR object model
 module IR
-export CodeInfo, MethodInstance, CodeInstance, GotoNode,
-    NewvarNode, SSAValue, Slot, SlotNumber, TypedSlot,
+export CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, ReturnNode,
+    NewvarNode, SSAValue, Slot, SlotNumber, TypedSlot, Argument,
     PiNode, PhiNode, PhiCNode, UpsilonNode, LineInfoNode
 
-import Core: CodeInfo, MethodInstance, CodeInstance, GotoNode,
-    NewvarNode, SSAValue, Slot, SlotNumber, TypedSlot,
+import Core: CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, ReturnNode,
+    NewvarNode, SSAValue, Slot, SlotNumber, TypedSlot, Argument,
     PiNode, PhiNode, PhiCNode, UpsilonNode, LineInfoNode
 
 end

base/compiler/abstractinterpretation.jl

@@ -1174,13 +1174,13 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
                 changes[sn] = VarState(Bottom, true)
             elseif isa(stmt, GotoNode)
                 pc´ = (stmt::GotoNode).label
-            elseif hd === :gotoifnot
-                condt = abstract_eval(interp, stmt.args[1], s[pc], frame)
+            elseif isa(stmt, GotoIfNot)
+                condt = abstract_eval(interp, stmt.cond, s[pc], frame)
                 if condt === Bottom
                     break
                 end
                 condval = maybe_extract_const_bool(condt)
-                l = stmt.args[2]::Int
+                l = stmt.dest::Int
                 # constant conditions
                 if condval === true
                 elseif condval === false
@@ -1207,9 +1207,9 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
                         s[l] = newstate_else
                     end
                 end
-            elseif hd === :return
+            elseif isa(stmt, ReturnNode)
                 pc´ = n + 1
-                rt = widenconditional(abstract_eval(interp, stmt.args[1], s[pc], frame))
+                rt = widenconditional(abstract_eval(interp, stmt.val, s[pc], frame))
                 if !isa(rt, Const) && !isa(rt, Type) && !isa(rt, PartialStruct)
                     # only propagate information we know we can store
                     # and is valid inter-procedurally

base/compiler/optimize.jl

@@ -339,12 +339,6 @@ function statement_cost(ex::Expr, line::Int, src::CodeInfo, sptypes::Vector{Any}
         # prevent inlining.
         extyp = line == -1 ? Any : src.ssavaluetypes[line]
         return extyp === Union{} ? 0 : 20
-    elseif head === :return
-        a = ex.args[1]
-        if a isa Expr
-            return statement_cost(a, -1, src, sptypes, slottypes, params)
-        end
-        return 0
     elseif head === :(=)
         if ex.args[1] isa GlobalRef
             cost = 20
@@ -364,12 +358,6 @@ function statement_cost(ex::Expr, line::Int, src::CodeInfo, sptypes::Vector{Any}
         # since these aren't usually performance-sensitive functions,
         # and llvm is more likely to miscompile them when these functions get large
         return typemax(Int)
-    elseif head === :gotoifnot
-        target = ex.args[2]::Int
-        # loops are generally always expensive
-        # but assume that forward jumps are already counted for from
-        # summing the cost of the not-taken branch
-        return target < line ? 40 : 0
     end
     return 0
 end
@@ -386,6 +374,8 @@ function inline_worthy(body::Array{Any,1}, src::CodeInfo, sptypes::Vector{Any},
             # but assume that forward jumps are already counted for from
             # summing the cost of the not-taken branch
             thiscost = stmt.label < line ? 40 : 0
+        elseif stmt isa GotoIfNot
+            thiscost = stmt.dest < line ? 40 : 0
         else
             continue
         end
@@ -423,20 +413,23 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
         el = body[i]
         if isa(el, GotoNode)
             body[i] = GotoNode(el.label + labelchangemap[el.label])
+        elseif isa(el, GotoIfNot)
+            cond = el.cond
+            if isa(cond, SSAValue)
+                cond = SSAValue(cond.id + ssachangemap[cond.id])
+            end
+            body[i] = GotoIfNot(cond, el.dest + labelchangemap[el.dest])
+        elseif isa(el, ReturnNode)
+            if isdefined(el, :val) && isa(el.val, SSAValue)
+                body[i] = ReturnNode(SSAValue(el.val.id + ssachangemap[el.val.id]))
+            end
         elseif isa(el, SSAValue)
             body[i] = SSAValue(el.id + ssachangemap[el.id])
         elseif isa(el, Expr)
             if el.head === :(=) && el.args[2] isa Expr
                 el = el.args[2]::Expr
             end
-            if el.head === :gotoifnot
-                cond = el.args[1]
-                if isa(cond, SSAValue)
-                    el.args[1] = SSAValue(cond.id + ssachangemap[cond.id])
-                end
-                tgt = el.args[2]::Int
-                el.args[2] = tgt + labelchangemap[tgt]
-            elseif el.head === :enter
+            if el.head === :enter
                 tgt = el.args[1]::Int
                 el.args[1] = tgt + labelchangemap[tgt]
             elseif !is_meta_expr_head(el.head)

base/compiler/ssair/driver.jl

@@ -20,18 +20,6 @@ include("compiler/ssair/verify.jl")
 include("compiler/ssair/legacy.jl")
 #@isdefined(Base) && include("compiler/ssair/show.jl")
 
-function normalize_expr(stmt::Expr)
-    if stmt.head === :gotoifnot
-        return GotoIfNot(stmt.args[1], stmt.args[2]::Int)
-    elseif stmt.head === :return
-        return (length(stmt.args) == 0) ? ReturnNode(nothing) : ReturnNode(stmt.args[1])
-    elseif stmt.head === :unreachable
-        return ReturnNode()
-    else
-        return stmt
-    end
-end
-
 function normalize(@nospecialize(stmt), meta::Vector{Any})
     if isa(stmt, Expr)
         if stmt.head === :meta
@@ -40,10 +28,6 @@ function normalize(@nospecialize(stmt), meta::Vector{Any})
                 push!(meta, stmt)
             end
             return nothing
-        elseif stmt.head === :line
-            return nothing # deprecated - we shouldn't encounter this
-        else
-            return normalize_expr(stmt)
         end
     end
     return stmt
@@ -72,7 +56,7 @@ function convert_to_ircode(ci::CodeInfo, code::Vector{Any}, coverage::Bool, narg
             prevloc = codeloc
         end
         if code[idx] isa Expr && ci.ssavaluetypes[idx] === Union{}
-            if !(idx < length(code) && isexpr(code[idx + 1], :unreachable))
+            if !(idx < length(code) && isa(code[idx + 1], ReturnNode) && !isdefined((code[idx + 1]::ReturnNode), :val))
                 # insert unreachable in the same basic block after the current instruction (splitting it)
                 insert!(code, idx + 1, ReturnNode())
                 insert!(ci.codelocs, idx + 1, ci.codelocs[idx])

base/compiler/ssair/ir.jl

@@ -4,23 +4,6 @@
 @eval Core.UpsilonNode() = $(Expr(:new, Core.UpsilonNode))
 Core.PhiNode() = Core.PhiNode(Any[], Any[])
 
-struct Argument
-    n::Int
-end
-
-struct GotoIfNot
-    cond::Any
-    dest::Int
-    GotoIfNot(@nospecialize(cond), dest::Int) = new(cond, dest)
-end
-
-struct ReturnNode
-    val::Any
-    ReturnNode(@nospecialize(val)) = new(val)
-    # unassigned val indicates unreachable
-    ReturnNode() = new()
-end
-
 """
 Like UnitRange{Int}, but can handle the `last` field, being temporarily
 < first (this can happen during compacting)
@@ -85,14 +68,6 @@ function basic_blocks_starts(stmts::Vector{Any})
                 push!(jump_dests, idx+1)
                 # The catch block is a jump dest
                 push!(jump_dests, stmt.args[1]::Int)
-            elseif stmt.head === :gotoifnot
-                # also tolerate expr form of IR
-                push!(jump_dests, idx+1)
-                push!(jump_dests, stmt.args[2]::Int)
-            elseif stmt.head === :return
-                # also tolerate expr form of IR
-                # This is a fake dest to force the next stmt to start a bb
-                idx < length(stmts) && push!(jump_dests, idx+1)
             end
         end
         if isa(stmt, PhiNode)
@@ -129,7 +104,7 @@ function compute_basic_blocks(stmts::Vector{Any})
     # Compute successors/predecessors
     for (num, b) in enumerate(blocks)
         terminator = stmts[last(b.stmts)]
-        if isa(terminator, ReturnNode) || isexpr(terminator, :return)
+        if isa(terminator, ReturnNode)
             # return never has any successors
             continue
         end
@@ -161,15 +136,6 @@ function compute_basic_blocks(stmts::Vector{Any})
                 push!(blocks[block′].preds, num)
                 push!(blocks[block′].preds, 0)
                 push!(b.succs, block′)
-            elseif terminator.head === :gotoifnot
-                block′ = block_for_inst(basic_block_index, terminator.args[2]::Int)
-                if block′ == num + 1
-                    # This GotoIfNot acts like a noop - treat it as such.
-                    # We will drop it during SSA renaming
-                else
-                    push!(blocks[block′].preds, num)
-                    push!(b.succs, block′)
-                end
             end
         end
         # statement fall-through
@@ -396,8 +362,7 @@ function is_relevant_expr(e::Expr)
                       :gc_preserve_begin, :gc_preserve_end,
                       :foreigncall, :isdefined, :copyast,
                       :undefcheck, :throw_undef_if_not,
-                      :cfunction, :method, :pop_exception,
-                      #=legacy IR format support=# :gotoifnot, :return)
+                      :cfunction, :method, :pop_exception)
 end
 
 function setindex!(x::UseRef, @nospecialize(v))

base/compiler/ssair/legacy.jl

@@ -12,22 +12,9 @@ end
 
 function inflate_ir(ci::CodeInfo, sptypes::Vector{Any}, argtypes::Vector{Any})
     code = copy_exprargs(ci.code) # TODO: this is a huge hot-spot
-    for i = 1:length(code)
-        if isa(code[i], Expr)
-            code[i] = normalize_expr(code[i])
-        end
-    end
     cfg = compute_basic_blocks(code)
     for i = 1:length(code)
         stmt = code[i]
-        urs = userefs(stmt)
-        for op in urs
-            val = op[]
-            if isa(val, SlotNumber)
-                op[] = Argument(val.id)
-            end
-        end
-        stmt = urs[]
         # Translate statement edges to bb_edges
         if isa(stmt, GotoNode)
             code[i] = GotoNode(block_for_inst(cfg, stmt.label))
@@ -67,26 +54,12 @@ function replace_code_newstyle!(ci::CodeInfo, ir::IRCode, nargs::Int)
     # (and undo normalization for now)
     for i = 1:length(ci.code)
         stmt = ci.code[i]
-        urs = userefs(stmt)
-        for op in urs
-            val = op[]
-            if isa(val, Argument)
-                op[] = SlotNumber(val.n)
-            end
-        end
-        stmt = urs[]
         if isa(stmt, GotoNode)
             stmt = GotoNode(first(ir.cfg.blocks[stmt.label].stmts))
         elseif isa(stmt, GotoIfNot)
-            stmt = Expr(:gotoifnot, stmt.cond, first(ir.cfg.blocks[stmt.dest].stmts))
+            stmt = GotoIfNot(stmt.cond, first(ir.cfg.blocks[stmt.dest].stmts))
         elseif isa(stmt, PhiNode)
             stmt = PhiNode(Any[last(ir.cfg.blocks[edge::Int].stmts) for edge in stmt.edges], stmt.values)
-        elseif isa(stmt, ReturnNode)
-            if isdefined(stmt, :val)
-                stmt = Expr(:return, stmt.val)
-            else
-                stmt = Expr(:unreachable)
-            end
         elseif isa(stmt, Expr) && stmt.head === :enter
             stmt.args[1] = first(ir.cfg.blocks[stmt.args[1]::Int].stmts)
         end

base/compiler/ssair/show.jl

@@ -141,7 +141,7 @@ end
 
 function should_print_ssa_type(@nospecialize node)
     if isa(node, Expr)
-        return !(node.head in (:gc_preserve_begin, :gc_preserve_end, :meta, :return, :enter, :leave))
+        return !(node.head in (:gc_preserve_begin, :gc_preserve_end, :meta, :enter, :leave))
     end
     return !isa(node, PiNode)   && !isa(node, GotoIfNot) &&
            !isa(node, GotoNode) && !isa(node, ReturnNode) &&
@@ -722,9 +722,7 @@ function show_ir_stmt(io::IO, code::CodeInfo, idx::Int, line_info_preprinter, li
     print(io, inlining_indent, " ")
     # convert statement index to labels, as expected by print_stmt
     if stmt isa Expr
-        if stmt.head === :gotoifnot && length(stmt.args) == 2 && stmt.args[2] isa Int
-            stmt = GotoIfNot(stmt.args[1], block_for_inst(cfg, stmt.args[2]::Int))
-        elseif stmt.head === :enter && length(stmt.args) == 1 && stmt.args[1] isa Int
+        if stmt.head === :enter && length(stmt.args) == 1 && stmt.args[1] isa Int
             stmt = Expr(:enter, block_for_inst(cfg, stmt.args[1]::Int))
         end
     elseif isa(stmt, GotoIfNot)

base/compiler/ssair/slot2ssa.jl

@@ -43,7 +43,6 @@ function lift_defuse(cfg::CFG, defuse)
     end
 end
 
-@inline slot_id(s) = isa(s, SlotNumber) ? (s::SlotNumber).id : (s::TypedSlot).id
 function scan_slot_def_use(nargs::Int, ci::CodeInfo, code::Vector{Any})
     nslots = length(ci.slotflags)
     result = SlotInfo[SlotInfo() for i = 1:nslots]
@@ -821,7 +820,7 @@ function construct_ssa!(ci::CodeInfo, ir::IRCode, domtree::DomTree, defuse, narg
         elseif isexpr(stmt, :enter)
             new_code[idx] = Expr(:enter, block_for_inst(cfg, stmt.args[1]))
             ssavalmap[idx] = SSAValue(idx) # Slot to store token for pop_exception
-        elseif isexpr(stmt, :leave) || isexpr(stmt, :(=)) || isexpr(stmt, :return) ||
+        elseif isexpr(stmt, :leave) || isexpr(stmt, :(=)) || isa(stmt, ReturnNode) ||
             isexpr(stmt, :meta) || isa(stmt, NewvarNode)
             new_code[idx] = stmt
         else

base/compiler/typeinfer.jl

@@ -251,6 +251,15 @@ function annotate_slot_load!(e::Expr, vtypes::VarTable, sv::InferenceState, unde
     end
 end
 
+function annotate_slot_load(@nospecialize(e), vtypes::VarTable, sv::InferenceState, undefs::Array{Bool,1})
+    if isa(e, Expr)
+        annotate_slot_load!(e, vtypes, sv, undefs)
+    elseif isa(e, Slot)
+        return visit_slot_load!(e, vtypes, sv, undefs)
+    end
+    return e
+end
+
 function visit_slot_load!(sl::Slot, vtypes::VarTable, sv::InferenceState, undefs::Array{Bool,1})
     id = slot_id(sl)
     s = vtypes[id]
@@ -330,13 +339,12 @@ function type_annotate!(sv::InferenceState)
     body = src.code::Array{Any,1}
     nexpr = length(body)
 
-    # replace gotoifnot with its condition if the branch target is unreachable
+    # replace GotoIfNot with its condition if the branch target is unreachable
     for i = 1:nexpr
         expr = body[i]
-        if isa(expr, Expr) && expr.head === :gotoifnot
-            tgt = expr.args[2]::Int
-            if !isa(states[tgt], VarTable)
-                body[i] = expr.args[1]
+        if isa(expr, GotoIfNot)
+            if !isa(states[expr.dest], VarTable)
+                body[i] = expr.cond
             end
         end
     end
@@ -353,6 +361,10 @@ function type_annotate!(sv::InferenceState)
             # st_i === nothing  =>  unreached statement  (see issue #7836)
             if isa(expr, Expr)
                 annotate_slot_load!(expr, st_i, sv, undefs)
+            elseif isa(expr, ReturnNode) && isdefined(expr, :val)
+                body[i] = ReturnNode(annotate_slot_load(expr.val, st_i, sv, undefs))
+            elseif isa(expr, GotoIfNot)
+                body[i] = GotoIfNot(annotate_slot_load(expr.cond, st_i, sv, undefs), expr.dest)
             elseif isa(expr, Slot)
                 body[i] = visit_slot_load!(expr, st_i, sv, undefs)
             end
@@ -549,7 +561,7 @@ function typeinf_ext(interp::AbstractInterpreter, mi::MethodInstance)
             if invoke_api(code) == 2
                 i == 2 && ccall(:jl_typeinf_end, Cvoid, ())
                 tree = ccall(:jl_new_code_info_uninit, Ref{CodeInfo}, ())
-                tree.code = Any[ Expr(:return, quoted(code.rettype_const)) ]
+                tree.code = Any[ ReturnNode(quoted(code.rettype_const)) ]
                 nargs = Int(method.nargs)
                 tree.slotnames = ccall(:jl_uncompress_argnames, Vector{Symbol}, (Any,), method.slot_syms)
                 tree.slotflags = fill(0x00, nargs)