Merge pull request JuliaLang#45701 from JuliaLang/avi/rtconsistency

effects: improve the `:consistent`-cy analysis accuracy

Author: aviatesk

base/compiler/abstractinterpretation.jl

@@ -221,7 +221,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
     elseif isa(matches, MethodMatches) ? (!matches.fullmatch || any_ambig(matches)) :
             (!all(matches.fullmatches) || any_ambig(matches))
         # Account for the fact that we may encounter a MethodError with a non-covered or ambiguous signature.
-        all_effects = Effects(all_effects; nothrow=TRISTATE_UNKNOWN)
+        all_effects = Effects(all_effects; nothrow=ALWAYS_FALSE)
     end
 
     rettype = from_interprocedural!(rettype, sv, arginfo, conditionals)
@@ -619,7 +619,7 @@ function abstract_call_method(interp::AbstractInterpreter, method::Method, @nosp
     elseif edgecycle
         # Some sort of recursion was detected. Even if we did not limit types,
         # we cannot guarantee that the call will terminate
-        effects = Effects(effects; terminates=TRISTATE_UNKNOWN)
+        effects = Effects(effects; terminates=ALWAYS_FALSE)
     end
 
     return MethodCallResult(rt, edgecycle, edgelimited, edge, effects)
@@ -1769,7 +1769,7 @@ function abstract_call_opaque_closure(interp::AbstractInterpreter,
         (aty, rty) = (unwrap_unionall(ftt)::DataType).parameters
         rty = rewrap_unionall(rty isa TypeVar ? rty.lb : rty, ftt)
         if !(rt ⊑ rty && tuple_tfunc(arginfo.argtypes[2:end]) ⊑ rewrap_unionall(aty, ftt))
-            effects = Effects(effects; nothrow=TRISTATE_UNKNOWN)
+            effects = Effects(effects; nothrow=ALWAYS_FALSE)
         end
     end
     rt = from_interprocedural!(rt, sv, arginfo, match.spec_types)
@@ -1953,9 +1953,7 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
                 at = tmeet(at, ft)
                 if at === Bottom
                     t = Bottom
-                    tristate_merge!(sv, Effects(EFFECTS_TOTAL;
-                        # consistent = ALWAYS_TRUE, # N.B depends on !ismutabletype(t) above
-                        nothrow = TRISTATE_UNKNOWN))
+                    tristate_merge!(sv, EFFECTS_THROWS)
                     @goto t_computed
                 elseif !isa(at, Const)
                     allconst = false
@@ -1985,7 +1983,7 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
         end
         tristate_merge!(sv, Effects(EFFECTS_TOTAL;
             consistent = !ismutabletype(t) ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-            nothrow = is_nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN))
+            nothrow = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE))
     elseif ehead === :splatnew
         t, isexact = instanceof_tfunc(abstract_eval_value(interp, e.args[1], vtypes, sv))
         is_nothrow = false # TODO: More precision
@@ -2003,8 +2001,8 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
             end
         end
         tristate_merge!(sv, Effects(EFFECTS_TOTAL;
-            consistent = ismutabletype(t) ? TRISTATE_UNKNOWN : ALWAYS_TRUE,
-            nothrow = is_nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN))
+            consistent = !ismutabletype(t) ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
+            nothrow = is_nothrow ? ALWAYS_TRUE : ALWAYS_FALSE))
     elseif ehead === :new_opaque_closure
         tristate_merge!(sv, Effects()) # TODO
         t = Union{}
@@ -2040,12 +2038,12 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
             effects = v[2]
             effects = decode_effects_override(effects)
             tristate_merge!(sv, Effects(
-                effects.consistent ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-                effects.effect_free ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-                effects.nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-                effects.terminates_globally ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
+                effects.consistent ? ALWAYS_TRUE : ALWAYS_FALSE,
+                effects.effect_free ? ALWAYS_TRUE : ALWAYS_FALSE,
+                effects.nothrow ? ALWAYS_TRUE : ALWAYS_FALSE,
+                effects.terminates_globally ? ALWAYS_TRUE : ALWAYS_FALSE,
                 #=nonoverlayed=#true,
-                effects.notaskstate ? ALWAYS_TRUE : TRISTATE_UNKNOWN
+                effects.notaskstate ? ALWAYS_TRUE : ALWAYS_FALSE
             ))
         else
             tristate_merge!(sv, EFFECTS_UNKNOWN)
@@ -2127,20 +2125,20 @@ function abstract_eval_global(M::Module, s::Symbol, frame::InferenceState)
     ty = abstract_eval_global(M, s)
     isa(ty, Const) && return ty
     if isdefined(M,s)
-        tristate_merge!(frame, Effects(EFFECTS_TOTAL; consistent=TRISTATE_UNKNOWN))
+        tristate_merge!(frame, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE))
     else
         tristate_merge!(frame, Effects(EFFECTS_TOTAL;
-            consistent=TRISTATE_UNKNOWN,
-            nothrow=TRISTATE_UNKNOWN))
+            consistent=ALWAYS_FALSE,
+            nothrow=ALWAYS_FALSE))
     end
     return ty
 end
 
 function handle_global_assignment!(interp::AbstractInterpreter, frame::InferenceState, lhs::GlobalRef, @nospecialize(newty))
-    nothrow = global_assignment_nothrow(lhs.mod, lhs.name, newty)
-    tristate_merge!(frame, Effects(EFFECTS_TOTAL,
-        effect_free=TRISTATE_UNKNOWN,
-        nothrow=nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN))
+    effect_free = ALWAYS_FALSE
+    nothrow = global_assignment_nothrow(lhs.mod, lhs.name, newty) ?
+        ALWAYS_TRUE : ALWAYS_FALSE
+    tristate_merge!(frame, Effects(EFFECTS_TOTAL; effect_free, nothrow))
 end
 
 abstract_eval_ssavalue(s::SSAValue, sv::InferenceState) = abstract_eval_ssavalue(s, sv.ssavaluetypes)
@@ -2230,12 +2228,10 @@ end
 
 function handle_control_backedge!(frame::InferenceState, from::Int, to::Int)
     if from > to
-        if is_effect_overridden(frame, :terminates_globally)
-            # this frame is known to terminate
-        elseif is_effect_overridden(frame, :terminates_locally)
+        if is_effect_overridden(frame, :terminates_locally)
             # this backedge is known to terminate
         else
-            tristate_merge!(frame, Effects(EFFECTS_TOTAL; terminates=TRISTATE_UNKNOWN))
+            tristate_merge!(frame, Effects(EFFECTS_TOTAL; terminates=ALWAYS_FALSE))
         end
     end
     return nothing

base/compiler/inferencestate.jl

@@ -179,7 +179,7 @@ mutable struct InferenceState
         #       requires dynamic reachability, while the former is global).
         inbounds = inbounds_option()
         inbounds_taints_consistency = !(inbounds === :on || (inbounds === :default && !any_inbounds(code)))
-        consistent = inbounds_taints_consistency ? TRISTATE_UNKNOWN : ALWAYS_TRUE
+        consistent = inbounds_taints_consistency ? ALWAYS_FALSE : ALWAYS_TRUE
         ipo_effects = Effects(EFFECTS_TOTAL; consistent, inbounds_taints_consistency)
 
         params = InferenceParams(interp)

base/compiler/ssair/show.jl

@@ -791,7 +791,11 @@ function show_ir(io::IO, code::Union{IRCode, CodeInfo}, config::IRShowConfig=def
 end
 
 tristate_letter(t::TriState) = t === ALWAYS_TRUE ? '+' : t === ALWAYS_FALSE ? '!' : '?'
-tristate_color(t::TriState) = t === ALWAYS_TRUE ? :green : t === ALWAYS_FALSE ? :red : :orange
+tristate_color(t::TriState) = t === ALWAYS_TRUE ? :green : t === ALWAYS_FALSE ? :red : :yellow
+tristate_repr(t::TriState) =
+    t === ALWAYS_TRUE ? "ALWAYS_TRUE" :
+    t === ALWAYS_FALSE ? "ALWAYS_FALSE" :
+    t === TRISTATE_UNKNOWN ? "TRISTATE_UNKNOWN" : nothing
 
 function Base.show(io::IO, e::Core.Compiler.Effects)
     print(io, "(")
@@ -808,4 +812,13 @@ function Base.show(io::IO, e::Core.Compiler.Effects)
     e.nonoverlayed || printstyled(io, '′'; color=:red)
 end
 
+function Base.show(io::IO, t::TriState)
+    s = tristate_repr(t)
+    if s !== nothing
+        printstyled(io, s; color = tristate_color(t))
+    else # unknown state, redirect to the fallback printing
+        Base.@invoke show(io::IO, t::Any)
+    end
+end
+
 @specialize

base/compiler/tfuncs.jl

@@ -1794,14 +1794,14 @@ const _SPECIAL_BUILTINS = Any[
     Core._apply_iterate
 ]
 
-function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt)
+function builtin_effects(f::Builtin, argtypes::Vector{Any}, @nospecialize rt)
     if isa(f, IntrinsicFunction)
         return intrinsic_effects(f, argtypes)
     end
 
     @assert !contains_is(_SPECIAL_BUILTINS, f)
 
-    nothrow = false
+    argtypes′ = argtypes[2:end]
     if (f === Core.getfield || f === Core.isdefined) && length(argtypes) >= 3
         # consistent if the argtype is immutable
         if isvarargtype(argtypes[2])
@@ -1812,37 +1812,42 @@ function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt)
             return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, nonoverlayed=true)
         end
         s = s::DataType
-        ipo_consistent = !ismutabletype(s)
-        nothrow = false
-        if f === Core.getfield && !isvarargtype(argtypes[end]) &&
-                getfield_boundscheck(argtypes[2:end]) !== true
+        consistent = !ismutabletype(s) ? ALWAYS_TRUE : ALWAYS_FALSE
+        if f === Core.getfield && !isvarargtype(argtypes[end]) && getfield_boundscheck(argtypes′) !== true
             # If we cannot independently prove inboundsness, taint consistency.
             # The inbounds-ness assertion requires dynamic reachability, while
             # :consistent needs to be true for all input values.
             # N.B. We do not taint for `--check-bounds=no` here -that happens in
             # InferenceState.
-            nothrow = getfield_nothrow(argtypes[2], argtypes[3], true)
-            ipo_consistent &= nothrow
+            if getfield_nothrow(argtypes[2], argtypes[3], true)
+                nothrow = ALWAYS_TRUE
+            else
+                consistent = nothrow = ALWAYS_FALSE
+            end
         else
-            nothrow = isvarargtype(argtypes[end]) ? false :
-                builtin_nothrow(f, argtypes[2:end], rt)
+            nothrow = (!isvarargtype(argtypes[end]) && builtin_nothrow(f, argtypes′, rt)) ?
+                ALWAYS_TRUE : ALWAYS_FALSE
         end
-        effect_free = true
+        effect_free = ALWAYS_TRUE
     elseif f === getglobal && length(argtypes) >= 3
-        nothrow = getglobal_nothrow(argtypes[2:end])
-        ipo_consistent = nothrow && isconst( # types are already checked in `getglobal_nothrow`
-            (argtypes[2]::Const).val::Module, (argtypes[3]::Const).val::Symbol)
-        effect_free = true
+        if getglobal_nothrow(argtypes′)
+            consistent = isconst( # types are already checked in `getglobal_nothrow`
+                (argtypes[2]::Const).val::Module, (argtypes[3]::Const).val::Symbol) ?
+                ALWAYS_TRUE : ALWAYS_FALSE
+            nothrow = ALWAYS_TRUE
+        else
+            consistent = nothrow = ALWAYS_FALSE
+        end
+        effect_free = ALWAYS_TRUE
     else
-        ipo_consistent = contains_is(_CONSISTENT_BUILTINS, f)
-        effect_free = contains_is(_EFFECT_FREE_BUILTINS, f) || contains_is(_PURE_BUILTINS, f)
-        nothrow = isvarargtype(argtypes[end]) ? false : builtin_nothrow(f, argtypes[2:end], rt)
+        consistent = contains_is(_CONSISTENT_BUILTINS, f) ? ALWAYS_TRUE : ALWAYS_FALSE
+        effect_free = (contains_is(_EFFECT_FREE_BUILTINS, f) || contains_is(_PURE_BUILTINS, f)) ?
+            ALWAYS_TRUE : ALWAYS_FALSE
+        nothrow = (!isvarargtype(argtypes[end]) && builtin_nothrow(f, argtypes′, rt)) ?
+            ALWAYS_TRUE : ALWAYS_FALSE
     end
 
-    return Effects(EFFECTS_TOTAL;
-        consistent = ipo_consistent ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-        effect_free = effect_free ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-        nothrow = nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN)
+    return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow)
 end
 
 function builtin_nothrow(@nospecialize(f), argtypes::Array{Any, 1}, @nospecialize(rt))
@@ -2008,19 +2013,18 @@ function intrinsic_effects(f::IntrinsicFunction, argtypes::Vector{Any})
         return Effects()
     end
 
-    ipo_consistent = !(
+    consistent = !(
         f === Intrinsics.pointerref ||      # this one is volatile
         f === Intrinsics.arraylen ||        # this one is volatile
         f === Intrinsics.sqrt_llvm_fast ||  # this one may differ at runtime (by a few ulps)
         f === Intrinsics.have_fma ||        # this one depends on the runtime environment
-        f === Intrinsics.cglobal)           # cglobal lookup answer changes at runtime
-    effect_free = !(f === Intrinsics.pointerset)
-    nothrow = !isvarargtype(argtypes[end]) && intrinsic_nothrow(f, argtypes[2:end])
-
-    return Effects(EFFECTS_TOTAL;
-        consistent = ipo_consistent ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-        effect_free = effect_free ? ALWAYS_TRUE : TRISTATE_UNKNOWN,
-        nothrow = nothrow ? ALWAYS_TRUE : TRISTATE_UNKNOWN)
+        f === Intrinsics.cglobal            # cglobal lookup answer changes at runtime
+        ) ? ALWAYS_TRUE : ALWAYS_FALSE
+    effect_free = !(f === Intrinsics.pointerset) ? ALWAYS_TRUE : ALWAYS_FALSE
+    nothrow = (!isvarargtype(argtypes[end]) && intrinsic_nothrow(f, argtypes[2:end])) ?
+        ALWAYS_TRUE : ALWAYS_FALSE
+
+    return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow)
 end
 
 # TODO: this function is a very buggy and poor model of the return_type function

base/compiler/typeinfer.jl

@@ -430,10 +430,18 @@ end
 function adjust_effects(sv::InferenceState)
     ipo_effects = Effects(sv)
 
-    # Always throwing an error counts or never returning both count as consistent,
-    # but we don't currently model idempontency using dataflow, so we don't notice.
-    # Fix that up here to improve precision.
-    if !ipo_effects.inbounds_taints_consistency && sv.bestguess === Union{}
+    # refine :consistent-cy effect using the return type information
+    # TODO this adjustment tries to compromise imprecise :consistent-cy information,
+    # that is currently modeled in a flow-insensitive way: ideally we want to model it
+    # with a proper dataflow analysis instead
+    rt = sv.bestguess
+    if !ipo_effects.inbounds_taints_consistency && rt === Bottom
+        # always throwing an error counts or never returning both count as consistent
+        ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE)
+    elseif ipo_effects.consistent === TRISTATE_UNKNOWN && is_consistent_rt(rt)
+        # in a case when the :consistent-cy here is only tainted by mutable allocations
+        # (indicated by `TRISTATE_UNKNOWN`), we may be able to refine it if the return
+        # type guarantees that the allocations are never returned
         ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE)
     end
 
@@ -461,6 +469,14 @@ function adjust_effects(sv::InferenceState)
     return ipo_effects
 end
 
+is_consistent_rt(@nospecialize rt) = _is_consistent_rt(widenconst(ignorelimited(rt)))
+function _is_consistent_rt(@nospecialize ty)
+    if isa(ty, Union)
+        return _is_consistent_rt(ty.a) && _is_consistent_rt(ty.b)
+    end
+    return ty === Symbol || isbitstype(ty)
+end
+
 # inference completed on `me`
 # update the MethodInstance
 function finish(me::InferenceState, interp::AbstractInterpreter)
@@ -775,11 +791,11 @@ function merge_call_chain!(parent::InferenceState, ancestor::InferenceState, chi
     # and ensure that walking the parent list will get the same result (DAG) from everywhere
     # Also taint the termination effect, because we can no longer guarantee the absence
     # of recursion.
-    tristate_merge!(parent, Effects(EFFECTS_TOTAL; terminates=TRISTATE_UNKNOWN))
+    tristate_merge!(parent, Effects(EFFECTS_TOTAL; terminates=ALWAYS_FALSE))
     while true
         add_cycle_backedge!(child, parent, parent.currpc)
         union_caller_cycle!(ancestor, child)
-        tristate_merge!(child, Effects(EFFECTS_TOTAL; terminates=TRISTATE_UNKNOWN))
+        tristate_merge!(child, Effects(EFFECTS_TOTAL; terminates=ALWAYS_FALSE))
         child = parent
         child === ancestor && break
         parent = child.parent::InferenceState

base/compiler/types.jl

@@ -56,15 +56,20 @@ Along the abstract interpretation, `Effects` at each statement are analyzed loca
 they are merged into the single global `Effects` that represents the entire effects of
 the analyzed method (see `tristate_merge!`).
 Each effect property is represented as tri-state and managed separately.
-The tri-state consists of `ALWAYS_TRUE`, `TRISTATE_UNKNOWN` and `ALWAYS_FALSE`.
+The tri-state consists of `ALWAYS_TRUE`, `TRISTATE_UNKNOWN` and `ALWAYS_FALSE`, where they
+have the following meanings:
+- `ALWAYS_TRUE`: this method is guaranteed to not have this effect.
+- `ALWAYS_FALSE`: this method may have this effect, and there is no need to do any further
+  analysis w.r.t. this effect property as this conclusion will not be refined anyway.
+- `TRISTATE_UNKNOWN`: this effect property may still be refined to `ALWAYS_TRUE` or
+  `ALWAYS_FALSE`, e.g. using return type information.
+
 An effect property is initialized with `ALWAYS_TRUE` and then transitioned towards
-`TRISTATE_UNKNOWN` or `ALWAYS_FALSE`. When we find a statement that has some effect,
-`ALWAYS_TRUE` is propagated if that effect is known to _always_ happen, otherwise
-`TRISTATE_UNKNOWN` is propagated. If a property is known to be `ALWAYS_FALSE`,
-there is no need to do additional analysis as it can not be refined anyway.
-Note that however, within the current data-flow analysis design, it is hard to derive a global
-conclusion from a local analysis on each statement, and as a result, the effect analysis
-usually propagates `TRISTATE_UNKNOWN` currently.
+`ALWAYS_FALSE`. When we find a statement that has some effect, either of `TRISTATE_UNKNOWN`
+or `ALWAYS_FALSE` is propagated. Note that however, within the current flow-insensitive
+analysis design, it is usually difficult to derive a global conclusion accurately from local
+analysis on each statement, and therefore, the effect analysis usually propagates the
+`ALWAYS_FALSE` state conservatively.
 """
 struct Effects
     consistent::TriState
@@ -94,10 +99,10 @@ function Effects(
         false)
 end
 
-const EFFECTS_TOTAL    = Effects(ALWAYS_TRUE,      ALWAYS_TRUE,      ALWAYS_TRUE,      ALWAYS_TRUE,      true, ALWAYS_TRUE)
-const EFFECTS_THROWS   = Effects(ALWAYS_TRUE,      ALWAYS_TRUE,      TRISTATE_UNKNOWN, ALWAYS_TRUE,      true, ALWAYS_TRUE)
-const EFFECTS_UNKNOWN  = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, true, TRISTATE_UNKNOWN)  # mostly unknown, but it's not overlayed at least (e.g. it's not a call)
-const EFFECTS_UNKNOWN′ = Effects(TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, TRISTATE_UNKNOWN, false, TRISTATE_UNKNOWN) # unknown, really
+const EFFECTS_TOTAL    = Effects(ALWAYS_TRUE,  ALWAYS_TRUE,  ALWAYS_TRUE,  ALWAYS_TRUE,  true,  ALWAYS_TRUE)
+const EFFECTS_THROWS   = Effects(ALWAYS_TRUE,  ALWAYS_TRUE,  ALWAYS_FALSE, ALWAYS_TRUE,  true,  ALWAYS_TRUE)
+const EFFECTS_UNKNOWN  = Effects(ALWAYS_FALSE, ALWAYS_FALSE, ALWAYS_FALSE, ALWAYS_FALSE, true,  ALWAYS_FALSE)  # mostly unknown, but it's not overlayed at least (e.g. it's not a call)
+const EFFECTS_UNKNOWN′ = Effects(ALWAYS_FALSE, ALWAYS_FALSE, ALWAYS_FALSE, ALWAYS_FALSE, false, ALWAYS_FALSE) # unknown, really
 
 function Effects(e::Effects = EFFECTS_UNKNOWN′;
     consistent::TriState = e.consistent,