Skip to content

Commit

Permalink
inference: always use const-prop'ed result (#44001)
Browse files Browse the repository at this point in the history
Previously, for `invoke`/opaque closure callsite, we use constant-prop'ed
method body only when the inferred return type gets strictly improved
by const-prop. But since the inliner is now able to inline the method
body from `InferenceResult`, I believe it is always better to use method
body shaped up by const-prop' no matter if the return type is improved
(as we already do for ordinal call sites).

> use constant prop' result even when the return type doesn't get refined
```julia
const Gx = Ref{Any}()
Base.@constprop :aggressive function conditional_escape!(cnd, x)
    if cnd
        Gx[] = x
    end
    return nothing
end
@test fully_eliminated((String,)) do x
    Base.@invoke conditional_escape!(false::Any, x::Any)
end
```
  • Loading branch information
aviatesk authored Feb 2, 2022
1 parent b405562 commit e3b681c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 20 deletions.
28 changes: 8 additions & 20 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
this_arginfo = ArgInfo(fargs, this_argtypes)
const_result = abstract_call_method_with_const_args(interp, result, f, this_arginfo, match, sv, false)
if const_result !== nothing
const_rt, const_result = const_result
if const_rt !== rt && const_rt rt
rt = const_rt
end
rt, const_result = const_result
end
push!(const_results, const_result)
if const_result !== nothing
Expand Down Expand Up @@ -125,10 +122,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
this_arginfo = ArgInfo(fargs, this_argtypes)
const_result = abstract_call_method_with_const_args(interp, result, f, this_arginfo, match, sv, false)
if const_result !== nothing
const_this_rt, const_result = const_result
if const_this_rt !== this_rt && const_this_rt this_rt
this_rt = const_this_rt
end
this_rt, const_result = const_result
end
push!(const_results, const_result)
if const_result !== nothing
Expand Down Expand Up @@ -622,7 +616,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter, resul
# if constant inference hits a cycle, just bail out
isa(result, InferenceState) && return nothing
add_backedge!(mi, sv)
return result, inf_result
return Pair{Any,InferenceResult}(result, inf_result)
end

# if there's a possibility we could get a better result (hopefully without doing too much work)
Expand Down Expand Up @@ -1364,12 +1358,9 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
# end
const_result = abstract_call_method_with_const_args(interp, result, singleton_type(ft′), arginfo, match, sv, false)
if const_result !== nothing
const_rt, const_result = const_result
if const_rt !== rt && const_rt rt
rt, res = const_rt, const_result
end
rt, const_result = const_result
end
return CallMeta(from_interprocedural!(rt, sv, arginfo, sig), InvokeCallInfo(match, res))
return CallMeta(from_interprocedural!(rt, sv, arginfo, sig), InvokeCallInfo(match, const_result))
end

function invoke_rewrite(xs::Vector{Any})
Expand Down Expand Up @@ -1491,18 +1482,15 @@ function abstract_call_opaque_closure(interp::AbstractInterpreter, closure::Part
tt = closure.typ
sigT = (unwrap_unionall(tt)::DataType).parameters[1]
match = MethodMatch(sig, Core.svec(), closure.source, sig <: rewrap_unionall(sigT, tt))
res = nothing
const_result = nothing
if !result.edgecycle
const_result = abstract_call_method_with_const_args(interp, result, closure,
arginfo, match, sv, closure.isva)
if const_result !== nothing
const_rettype, const_result = const_result
if const_rettype rt
rt, res = const_rettype, const_result
end
rt, const_result = const_result
end
end
info = OpaqueClosureCallInfo(match, res)
info = OpaqueClosureCallInfo(match, const_result)
return CallMeta(from_interprocedural!(rt, sv, arginfo, match.spec_types), info)
end

Expand Down
12 changes: 12 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -895,3 +895,15 @@ end
# sqrt not considered volatile
f_sqrt() = sqrt(2)
@test fully_eliminated(f_sqrt, Tuple{})

# use constant prop' result even when the return type doesn't get refined
const Gx = Ref{Any}()
Base.@constprop :aggressive function conditional_escape!(cnd, x)
if cnd
Gx[] = x
end
return nothing
end
@test fully_eliminated((String,)) do x
Base.@invoke conditional_escape!(false::Any, x::Any)
end

2 comments on commit e3b681c

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

Please sign in to comment.