[compiler] Remove ConstAPI struct (#48598)

staticfloat · N5N3 · oscardssmith · web-flow · commit ce292c1ba4f5 · 2023-02-09T21:32:52.000-05:00
* Avoid split `y::Union` when comparing with a forall var.

If `y` has no free typevar. Then `x &lt;:  y` wont cause any env change. Thus there's no need to split `y`.

* [compiler] Remove `ConstAPI` struct

`ConstAPI` predates the effect analysis framework, and was relying on an
ad-hoc implementation of effect analysis that only worked for functions
of less than 15 statements.  As a result, it wasn't used much anymore,
but it could cause trouble with external `AbstractInterpreters` that may
want to cache additional code metadata within `CodeInstance`s.

This commit simply removes this, expecting that the existing effect
analysis will pick up the slack.

* Add short-circuit `has_offset_axes(::Array)`

This ensures that `Base.require_one_based_indexing()` is always fully
eliminated, since previously `has_offset_axes(::Array)` is
effect-tainted as non-consistent due to calculating `size(::Array)`.
This dodges the effect taint by adding this short-circuit, fixing broken
tests due to using effects analysis more extensively.

* Move coverage effects analysis into inference

Code coverage is implemented as an optimization pass that inserts an SSA
IR node during optimization, however effects analysis is perfomed during
inference, and so the desired effects tainting that should occur from
insertion of the code coverage IR node is ignored.

This teaches `InferenceState` to taint itself with the effects flag
 `effects_free = ALWAYS_FALSE` when coverage is applicable to this
inference state's method.

We just assume that you don't want code coverage if you're using this
constructor. This may not always be true.

---------

Co-authored-by: N5N3 &lt;2642243996@qq.com&gt;
Co-authored-by: Oscar Smith &lt;oscardssmith@gmail.com&gt;
diff --git a/base/abstractarray.jl b/base/abstractarray.jl
@@ -113,6 +113,7 @@ has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve perf
 # note: this could call `any` directly if the compiler can infer it
 has_offset_axes(As...) = _any_tuple(has_offset_axes, false, As...)
 has_offset_axes(::Colon) = false
+has_offset_axes(::Array) = false
 
 """
     require_one_based_indexing(A::AbstractArray)
diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl
@@ -129,14 +129,15 @@ mutable struct InferenceState
     # Set by default for toplevel frame.
     restrict_abstract_call_sites::Bool
     cached::Bool # TODO move this to InferenceResult?
+    insert_coverage::Bool
 
     # The interpreter that created this inference state. Not looked at by
     # NativeInterpreter. But other interpreters may use this to detect cycles
     interp::AbstractInterpreter
 
     # src is assumed to be a newly-allocated CodeInfo, that can be modified in-place to contain intermediate results
     function InferenceState(result::InferenceResult, src::CodeInfo, cache::Symbol,
-        interp::AbstractInterpreter)
+                            interp::AbstractInterpreter)
         linfo = result.linfo
         world = get_world_counter(interp)
         def = linfo.def
@@ -179,6 +180,32 @@ mutable struct InferenceState
         bestguess = Bottom
         ipo_effects = EFFECTS_TOTAL
 
+        # check if coverage mode is enabled
+        insert_coverage = coverage_enabled(mod)
+        if !insert_coverage && JLOptions().code_coverage == 3 # path-specific coverage mode
+            linetable = src.linetable
+            if isa(linetable, Vector{Any})
+                for line in linetable
+                    line = line::LineInfoNode
+                    if is_file_tracked(line.file)
+                        # if any line falls in a tracked file enable coverage for all
+                        insert_coverage = true
+                        break
+                    end
+                end
+            elseif isa(linetable, Vector{LineInfo})
+                for line in linetable
+                    if is_file_tracked(line.file)
+                        insert_coverage = true
+                        break
+                    end
+                end
+            end
+        end
+        if insert_coverage
+            ipo_effects = Effects(ipo_effects; effect_free = ALWAYS_FALSE)
+        end
+
         params = InferenceParams(interp)
         restrict_abstract_call_sites = isa(linfo.def, Module)
         @assert cache === :no || cache === :local || cache === :global
@@ -189,7 +216,7 @@ mutable struct InferenceState
             currbb, currpc, ip, handler_at, ssavalue_uses, bb_vartables, ssavaluetypes, stmt_edges, stmt_info,
             pclimitations, limitations, cycle_backedges, callers_in_cycle, dont_work_on_me, parent, inferred,
             result, valid_worlds, bestguess, ipo_effects,
-            params, restrict_abstract_call_sites, cached,
+            params, restrict_abstract_call_sites, cached, insert_coverage,
             interp)
 
         # some more setups
diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl
@@ -149,13 +149,14 @@ mutable struct OptimizationState{Interp<:AbstractInterpreter}
     slottypes::Vector{Any}
     inlining::InliningState{Interp}
     cfg::Union{Nothing,CFG}
+    insert_coverage::Bool
 end
 function OptimizationState(frame::InferenceState, params::OptimizationParams,
                            interp::AbstractInterpreter, recompute_cfg::Bool=true)
     inlining = InliningState(frame, params, interp)
     cfg = recompute_cfg ? nothing : frame.cfg
     return OptimizationState(frame.linfo, frame.src, nothing, frame.stmt_info, frame.mod,
-               frame.sptypes, frame.slottypes, inlining, cfg)
+               frame.sptypes, frame.slottypes, inlining, cfg, frame.insert_coverage)
 end
 function OptimizationState(linfo::MethodInstance, src::CodeInfo, params::OptimizationParams,
                            interp::AbstractInterpreter)
@@ -179,7 +180,7 @@ function OptimizationState(linfo::MethodInstance, src::CodeInfo, params::Optimiz
     # Allow using the global MI cache, but don't track edges.
     # This method is mostly used for unit testing the optimizer
     inlining = InliningState(params, interp)
-    return OptimizationState(linfo, src, nothing, stmt_info, mod, sptypes, slottypes, inlining, nothing)
+    return OptimizationState(linfo, src, nothing, stmt_info, mod, sptypes, slottypes, inlining, nothing, false)
 end
 function OptimizationState(linfo::MethodInstance, params::OptimizationParams, interp::AbstractInterpreter)
     src = retrieve_code_info(linfo)
@@ -228,23 +229,6 @@ is_stmt_inline(stmt_flag::UInt8)      = stmt_flag & IR_FLAG_INLINE      ≠ 0
 is_stmt_noinline(stmt_flag::UInt8)    = stmt_flag & IR_FLAG_NOINLINE    ≠ 0
 is_stmt_throw_block(stmt_flag::UInt8) = stmt_flag & IR_FLAG_THROW_BLOCK ≠ 0
 
-# These affect control flow within the function (so may not be removed
-# if there is no usage within the function), but don't affect the purity
-# of the function as a whole.
-function stmt_affects_purity(@nospecialize(stmt), ir)
-    if isa(stmt, GotoNode) || isa(stmt, ReturnNode)
-        return false
-    end
-    if isa(stmt, GotoIfNot)
-        t = argextype(stmt.cond, ir)
-        return !(t ⊑ Bool)
-    end
-    if isa(stmt, Expr)
-        return stmt.head !== :loopinfo && stmt.head !== :enter
-    end
-    return true
-end
-
 """
     stmt_effect_flags(stmt, rt, src::Union{IRCode,IncrementalCompact}) ->
         (consistent::Bool, effect_free_and_nothrow::Bool, nothrow::Bool)
@@ -406,80 +390,26 @@ function argextype(
 end
 abstract_eval_ssavalue(s::SSAValue, src::Union{IRCode,IncrementalCompact}) = types(src)[s]
 
-struct ConstAPI
-    val
-    ConstAPI(@nospecialize val) = new(val)
-end
 
 """
     finish(interp::AbstractInterpreter, opt::OptimizationState,
-           params::OptimizationParams, ir::IRCode, caller::InferenceResult) -> analyzed::Union{Nothing,ConstAPI}
+           params::OptimizationParams, ir::IRCode, caller::InferenceResult) -> analyzed::Nothing
 
 Post process information derived by Julia-level optimizations for later uses:
 - computes "purity", i.e. side-effect-freeness
 - computes inlining cost
-
-In a case when the purity is proven, `finish` can return `ConstAPI` object wrapping the constant
-value so that the runtime system will use the constant calling convention for the method calls.
 """
 function finish(interp::AbstractInterpreter, opt::OptimizationState,
                 params::OptimizationParams, ir::IRCode, caller::InferenceResult)
     (; src, linfo) = opt
     (; def, specTypes) = linfo
 
-    analyzed = nothing # `ConstAPI` if this call can use constant calling convention
     force_noinline = is_declared_noinline(src)
 
     # compute inlining and other related optimizations
     result = caller.result
     @assert !(result isa LimitedAccuracy)
     result = widenslotwrapper(result)
-    if (isa(result, Const) || isconstType(result))
-        proven_pure = false
-        # must be proven pure to use constant calling convention;
-        # otherwise we might skip throwing errors (issue #20704)
-        # TODO: Improve this analysis; if a function is marked @pure we should really
-        # only care about certain errors (e.g. method errors and type errors).
-        if length(ir.stmts) < 15
-            proven_pure = true
-            for i in 1:length(ir.stmts)
-                node = ir.stmts[i]
-                stmt = node[:inst]
-                if stmt_affects_purity(stmt, ir) && !stmt_effect_flags(optimizer_lattice(interp), stmt, node[:type], ir)[2]
-                    proven_pure = false
-                    break
-                end
-            end
-            if proven_pure
-                for fl in src.slotflags
-                    if (fl & SLOT_USEDUNDEF) != 0
-                        proven_pure = false
-                        break
-                    end
-                end
-            end
-        end
-
-        if proven_pure
-            # use constant calling convention
-            # Do not emit `jl_fptr_const_return` if coverage is enabled
-            # so that we don't need to add coverage support
-            # to the `jl_call_method_internal` fast path
-            # Still set pure flag to make sure `inference` tests pass
-            # and to possibly enable more optimization in the future
-            src.pure = true
-            if isa(result, Const)
-                val = result.val
-                if is_inlineable_constant(val)
-                    analyzed = ConstAPI(val)
-                end
-            else
-                @assert isconstType(result)
-                analyzed = ConstAPI(result.parameters[1])
-            end
-            force_noinline || set_inlineable!(src, true)
-        end
-    end
 
     opt.ir = ir
 
@@ -528,8 +458,7 @@ function finish(interp::AbstractInterpreter, opt::OptimizationState,
             src.inlining_cost = inline_cost(ir, params, union_penalties, cost_threshold)
         end
     end
-
-    return analyzed
+    return nothing
 end
 
 # run the optimization work
@@ -612,18 +541,6 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
         linetable = collect(LineInfoNode, linetable::Vector{Any})::Vector{LineInfoNode}
     end
 
-    # check if coverage mode is enabled
-    coverage = coverage_enabled(sv.mod)
-    if !coverage && JLOptions().code_coverage == 3 # path-specific coverage mode
-        for line in linetable
-            if is_file_tracked(line.file)
-                # if any line falls in a tracked file enable coverage for all
-                coverage = true
-                break
-            end
-        end
-    end
-
     # Go through and add an unreachable node after every
     # Union{} call. Then reindex labels.
     code = copy_exprargs(ci.code)
@@ -639,7 +556,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
     prevloc = zero(eltype(ci.codelocs))
     while idx <= length(code)
         codeloc = codelocs[idx]
-        if coverage && codeloc != prevloc && codeloc != 0
+        if sv.insert_coverage && codeloc != prevloc && codeloc != 0
             # insert a side-effect instruction before the current instruction in the same basic block
             insert!(code, idx, Expr(:code_coverage_effect))
             insert!(codelocs, idx, codeloc)
@@ -650,7 +567,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
                 ssachangemap = fill(0, nstmts)
             end
             if labelchangemap === nothing
-                labelchangemap = coverage ? fill(0, nstmts) : ssachangemap
+                labelchangemap = fill(0, nstmts)
             end
             ssachangemap[oldidx] += 1
             if oldidx < length(labelchangemap)
@@ -671,11 +588,11 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
                     ssachangemap = fill(0, nstmts)
                 end
                 if labelchangemap === nothing
-                    labelchangemap = coverage ? fill(0, nstmts) : ssachangemap
+                    labelchangemap = sv.insert_coverage ? fill(0, nstmts) : ssachangemap
                 end
                 if oldidx < length(ssachangemap)
                     ssachangemap[oldidx + 1] += 1
-                    coverage && (labelchangemap[oldidx + 1] += 1)
+                    sv.insert_coverage && (labelchangemap[oldidx + 1] += 1)
                 end
                 idx += 1
             end
diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl
@@ -843,10 +843,10 @@ function resolve_todo(mi::MethodInstance, result::Union{MethodMatch,InferenceRes
     #XXX: update_valid_age!(min_valid[1], max_valid[1], sv)
     if isa(result, InferenceResult)
         src = result.src
-        if isa(src, ConstAPI)
+        if is_total(result.ipo_effects) && isa(result.result, Const)
             # use constant calling convention
             add_inlining_backedge!(et, mi)
-            return ConstantCase(quoted(src.val))
+            return ConstantCase(quoted(result.result.val))
         end
         effects = result.ipo_effects
     else
@@ -866,16 +866,6 @@ function resolve_todo(mi::MethodInstance, result::Union{MethodMatch,InferenceRes
     end
 
     src = inlining_policy(state.interp, src, info, flag, mi, argtypes)
-
-    if isa(src, ConstAPI)
-        # duplicates the check above in case inlining_policy has a better idea.
-        # We still keep the check above to make sure we can inline to ConstAPI
-        # even if is_stmt_noinline. This doesn't currently happen in Base, but
-        # can happen with external AbstractInterpreter.
-        add_inlining_backedge!(et, mi)
-        return ConstantCase(quoted(src.val))
-    end
-
     src === nothing && return compileable_specialization(result, effects, et, info;
         compilesig_invokes=state.params.compilesig_invokes)
 
diff --git a/base/compiler/ssair/irinterp.jl b/base/compiler/ssair/irinterp.jl
@@ -144,7 +144,9 @@ function concrete_eval_invoke(interp::AbstractInterpreter,
     inst::Expr, mi::MethodInstance, irsv::IRInterpretationState)
     mi_cache = WorldView(code_cache(interp), irsv.world)
     code = get(mi_cache, mi, nothing)
-    code === nothing && return Pair{Any, Bool}(nothing, false)
+    if code === nothing
+        return Pair{Any, Bool}(nothing, false)
+    end
     argtypes = collect_argtypes(interp, inst.args[2:end], nothing, irsv.ir)
     argtypes === nothing && return Pair{Any, Bool}(Union{}, false)
     effects = decode_effects(code.ipo_purity_bits)
diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl
@@ -269,14 +269,6 @@ function _typeinf(interp::AbstractInterpreter, frame::InferenceState)
         opt = caller.src
         if opt isa OptimizationState{typeof(interp)} # implies `may_optimize(interp) === true`
             analyzed = optimize(interp, opt, OptimizationParams(interp), caller)
-            if isa(analyzed, ConstAPI)
-                # XXX: The work in ir_to_codeinf! is essentially wasted. The only reason
-                # we're doing it is so that code_llvm can return the code
-                # for the `return ...::Const` (which never runs anyway). We should do this
-                # as a post processing step instead.
-                ir_to_codeinf!(opt)
-                caller.src = analyzed
-            end
             caller.valid_worlds = (opt.inlining.et::EdgeTracker).valid_worlds[]
         end
     end
@@ -300,9 +292,12 @@ function CodeInstance(
     local const_flags::Int32
     result_type = result.result
     @assert !(result_type isa LimitedAccuracy)
-    if inferred_result isa ConstAPI
+
+    if isa(result_type, Const) && is_foldable(result.ipo_effects) &&
+                                  is_nothrow(result.ipo_effects) &&
+                                  is_inlineable_constant(result_type.val)
         # use constant calling convention
-        rettype_const = inferred_result.val
+        rettype_const = result_type.val
         const_flags = 0x3
         inferred_result = nothing
     else
@@ -379,7 +374,7 @@ function transform_result_for_cache(interp::AbstractInterpreter,
         inferred_result = maybe_compress_codeinfo(interp, linfo, inferred_result)
     end
     # The global cache can only handle objects that codegen understands
-    if !isa(inferred_result, Union{CodeInfo, Vector{UInt8}, ConstAPI})
+    if !isa(inferred_result, MaybeCompressed)
         inferred_result = nothing
     end
     return inferred_result
diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl
@@ -569,16 +569,9 @@ Base.@pure function fpure(a=rand(); b=rand())
     # but would be too big to inline
     return a + b + rand()
 end
-gpure() = fpure()
-gpure(x::Irrational) = fpure(x)
+
 @test which(fpure, ()).pure
 @test which(fpure, (typeof(pi),)).pure
-@test !which(gpure, ()).pure
-@test !which(gpure, (typeof(pi),)).pure
-@test code_typed(gpure, ())[1][1].pure
-@test code_typed(gpure, (typeof(π),))[1][1].pure
-@test gpure() == gpure() == gpure()
-@test gpure(π) == gpure(π) == gpure(π)
 
 # Make sure @pure works for functions using the new syntax
 Base.@pure (fpure2(x::T) where T) = T
@@ -941,13 +934,6 @@ Base.@pure c20704() = (f20704(1.0); 1)
 d20704() = c20704()
 @test_throws MethodError d20704()
 
-Base.@pure function a20704(x)
-    rand()
-    42
-end
-aa20704(x) = x(nothing)
-@test code_typed(aa20704, (typeof(a20704),))[1][1].pure
-
 #issue #21065, elision of _apply_iterate when splatted expression is not effect_free
 function f21065(x,y)
     println("x=$x, y=$y")
diff --git a/test/reducedim.jl b/test/reducedim.jl
@@ -6,7 +6,16 @@ using Random
 
 # issue #35800
 # tested very early since it can be state-dependent
-@test @inferred(mapreduce(x->count(!iszero,x), +, [rand(1)]; init = 0.)) == 1.0
+
+function my_simple_count(pred, g::Vector{T}) where {T}
+    n::T = zero(T)
+    for x in g
+        n += pred(x)
+    end
+    return n
+end
+
+@test @inferred(mapreduce(x->my_simple_count(!iszero,x), +, [rand(1)]; init = 0.)) == 1.0
 
 function safe_mapslices(op, A, region)
     newregion = intersect(region, 1:ndims(A))
