Skip to content

Commit

Permalink
Fix inling of _apply(_apply, ...)
Browse files Browse the repository at this point in the history
Cassette and packages that work similarly (e.g. Zygote) perform
essentially the following transformation:

For `f(args...) = g(args...)`, they define

```
(::overloaded{f})(c::Context, f, args...) = _apply(overloaded{g}(), (c, g), args)
```

With all of our recent compiler improvements inference can now handle
this pattern fairly well. However, inlining was falling over for the
case where `g === _apply` (because it didn't check the _apply special
case again after inlining the first _apply), causing inefficiencies
(_apply is fairly expensive). Fix that by revisiting the _apply check.
While we're at it, also remove the obsolete isapply flag in InliningTodo,
since we now do all the rewriting up front.
  • Loading branch information
Keno committed Oct 15, 2018
1 parent e11b0b0 commit 75f512b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 16 deletions.
32 changes: 16 additions & 16 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ struct InliningTodo
# need to be rewritten.
isva::Bool
isinvoke::Bool
isapply::Bool
na::Int
method::Method # The method being inlined
sparams::Vector{Any} # The static parameters we computed for this call site
Expand Down Expand Up @@ -634,7 +633,7 @@ end

function analyze_method!(idx::Int, @nospecialize(f), @nospecialize(ft), @nospecialize(metharg), methsp::SimpleVector,
method::Method, stmt::Expr, atypes::Vector{Any}, sv::OptimizationState, @nospecialize(atype_unlimited),
isinvoke::Bool, isapply::Bool, invoke_data::Union{InvokeData,Nothing}, @nospecialize(stmttyp))
isinvoke::Bool, invoke_data::Union{InvokeData,Nothing}, @nospecialize(stmttyp))
methsig = method.sig

# Check whether this call just evaluates to a constant
Expand Down Expand Up @@ -705,7 +704,7 @@ function analyze_method!(idx::Int, @nospecialize(f), @nospecialize(ft), @nospeci

return InliningTodo(idx,
na > 0 && method.isva,
isinvoke, isapply, na,
isinvoke, na,
method, Any[methsp...], metharg,
inline_linetable, ir2, linear_inline_eligible(ir2))
end
Expand Down Expand Up @@ -775,7 +774,7 @@ function handle_single_case!(ir::IRCode, stmt::Expr, idx::Int, @nospecialize(cas
end

function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::OptimizationState)
# todo = (inline_idx, (isva, isinvoke, isapply, na), method, spvals, inline_linetable, inline_ir, lie)
# todo = (inline_idx, (isva, isinvoke, na), method, spvals, inline_linetable, inline_ir, lie)
todo = Any[]
for idx in 1:length(ir.stmts)
stmt = ir.stmts[idx]
Expand Down Expand Up @@ -817,16 +816,13 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::

# Special handling for Core.invoke and Core._apply, which can follow the normal inliner
# logic with modified inlining target
isapply = isinvoke = false
isinvoke = false

# Handle _apply
if f === Core._apply
ft = atypes[2]
has_free_typevars(ft) && continue
f = singleton_type(ft)
ok = true
while f === Core._apply
# Try to figure out the signature of the function being called
# and if rewrite_apply_exprargs can deal with this form
ok = true
for i = 3:length(atypes)
typ = atypes[i]
typ = widenconst(typ)
Expand All @@ -837,12 +833,16 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::
break
end
end
ok || continue
isapply = true
ok || break
# Independent of whether we can inline, the above analysis allows us to rewrite
# this apply call to a regular call
ft = atypes[2]
stmt.args, atypes = rewrite_apply_exprargs!(ir, idx, stmt.args, atypes, sv)
ok = !has_free_typevars(ft)
ok || break
f = singleton_type(ft)
end
ok || continue

if f !== Core.invoke && (isa(f, IntrinsicFunction) || ft IntrinsicFunction || isa(f, Builtin) || ft Builtin)
# TODO: this test is wrong if we start to handle Unions of function types later
Expand Down Expand Up @@ -878,7 +878,7 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
atype, method.sig)::SimpleVector
methsp = methsp::SimpleVector
result = analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv, atype, isinvoke, isapply, invoke_data,
result = analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv, atype, isinvoke, invoke_data,
calltype)
handle_single_case!(ir, stmt, idx, result, isinvoke, todo, sv)
continue
Expand Down Expand Up @@ -907,7 +907,7 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::
fully_covered = false
continue
end
case = analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv, metharg, isinvoke, isapply, invoke_data, calltype)
case = analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv, metharg, isinvoke, invoke_data, calltype)
if case === nothing
fully_covered = false
continue
Expand All @@ -933,7 +933,7 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::
for (i, match) in enumerate(meth)
(metharg, methsp, method) = (match[1]::Type, match[2]::SimpleVector, match[3]::Method)
metharg′ <: method.sig || continue
case = analyze_method!(idx, f, ft, metharg′, methsp, method, stmt, atypes, sv, metharg′, isinvoke, isapply, invoke_data,
case = analyze_method!(idx, f, ft, metharg′, methsp, method, stmt, atypes, sv, metharg′, isinvoke, invoke_data,
calltype)
if case !== nothing
found_any = true
Expand All @@ -955,7 +955,7 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::
methsp = meth[1][2]::SimpleVector
method = meth[1][3]::Method
fully_covered = true
case = analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv, atype, isinvoke, isapply, invoke_data, calltype)
case = analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv, atype, isinvoke, invoke_data, calltype)
case === nothing && continue
push!(cases, Pair{Any,Any}(metharg, case))
end
Expand Down
8 changes: 8 additions & 0 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2078,3 +2078,11 @@ function g28955(x, y)
end

@test @inferred(g28955((1,), 1.0)) === Bool

# Test that inlining can look through repeated _applys
foo_inlining_apply(args...) = ccall(:jl_, Nothing, (Any,), args[1])
bar_inlining_apply() = Core._apply(Core._apply, (foo_inlining_apply,), ((1,),))
let ci = code_typed(bar_inlining_apply, Tuple{})[1].first
@test length(ci.code) == 2
@test ci.code[1].head == :foreigncall
end

0 comments on commit 75f512b

Please sign in to comment.