Skip to content

Commit

Permalink
[NewOptimizer] Port #26826 to new optimizer (#26981)
Browse files Browse the repository at this point in the history
* fix InferenceResult cache lookup for new optimizer

* utilize cached vararg type info in new inlining pass

* fix test to work with new optimizer

* fix predicate for exploiting cached varargs type info

* atypes no longer needs to be modified to match cached types

* don't require varargs to be in last position to exploit cached type info

* restore missing isva boolean
  • Loading branch information
jrevels authored and Keno committed May 6, 2018
1 parent bf29b40 commit 74af878
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 19 deletions.
17 changes: 5 additions & 12 deletions base/compiler/ssair/inlining2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -576,15 +576,6 @@ function analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv,
return ConstantCase(quoted(linfo.inferred_const), method, Any[methsp...], metharg)
end

# Handle vararg functions
isva = na > 0 && method.isva
if isva
@assert length(atypes) >= na - 1
va_type = tuple_tfunc(Tuple{Any[widenconst(atypes[i]) for i in 1:length(atypes)]...})
atypes = Any[atypes[1:(na - 1)]..., va_type]
end

# Go see if we already have a pre-inferred result
res = find_inferred(linfo, atypes, sv)
res === nothing && return nothing

Expand Down Expand Up @@ -627,7 +618,8 @@ function analyze_method!(idx, f, ft, metharg, methsp, method, stmt, atypes, sv,
#verify_ir(ir2)

return InliningTodo(idx,
isva, isinvoke, isapply, na,
na > 0 && method.isva,
isinvoke, isapply, na,
method, Any[methsp...], metharg,
inline_linetable, ir2, linear_inline_eligible(ir2))
end
Expand Down Expand Up @@ -694,7 +686,6 @@ function handle_single_case!(ir, stmt, idx, case, isinvoke, todo)
end
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 = Any[]
Expand Down Expand Up @@ -761,11 +752,13 @@ function assemble_inline_todo!(ir::IRCode, linetable::Vector{LineInfoNode}, sv::
# As a special case, if we can see the tuple() call, look at it's arguments to find
# our types. They can be more precise (e.g. f(Bool, A...) would be lowered as
# _apply(f, tuple(Bool)::Tuple{DataType}, A), which might not be precise enough to
# get a good method match. This pattern is used in the array code a bunch.
# get a good method match). This pattern is used in the array code a bunch.
if isa(def, SSAValue) && is_tuple_call(ir, ir[def])
for tuparg in ir[def].args[2:end]
push!(new_atypes, exprtype(tuparg, ir, ir.mod))
end
elseif isa(def, Argument) && def.n === length(ir.argtypes) && !isempty(sv.result_vargs)
append!(new_atypes, sv.result_vargs)
else
append!(new_atypes, typ.parameters)
end
Expand Down
12 changes: 5 additions & 7 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1536,9 +1536,9 @@ x26826 = rand()

apply26826(f, args...) = f(args...)

f26826(x) = apply26826(Base.getproperty, Foo26826(1, x), :b)
# We use getproperty to drive these tests because it requires constant
# propagation in order to lower to a well-inferred getfield call.
f26826(x) = apply26826(Base.getproperty, Foo26826(1, x), :b)

@test @inferred(f26826(x26826)) === x26826

Expand All @@ -1554,12 +1554,10 @@ g26826(x) = getfield26826(x, :a, :b)
# InferenceResult cache properly for varargs methods.
typed_code = Core.Compiler.code_typed(f26826, (Float64,))[1].first.code
found_well_typed_getfield_call = false
for stmnt in typed_code
if Meta.isexpr(stmnt, :(=)) && Meta.isexpr(stmnt.args[2], :call)
lhs = stmnt.args[2]
if lhs.args[1] == GlobalRef(Base, :getfield) && lhs.typ === Float64
global found_well_typed_getfield_call = true
end
for stmt in typed_code
lhs = Meta.isexpr(stmt, :(=)) ? stmt.args[2] : stmt
if Meta.isexpr(lhs, :call) && lhs.args[1] == GlobalRef(Base, :getfield) && lhs.typ === Float64
global found_well_typed_getfield_call = true
end
end

Expand Down

2 comments on commit 74af878

@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 benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(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 benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.