(with @vtjnash) move the optimizer's varargs types --> tuple type rew…

…rite to after cache lookup

Inference is populating the InferenceResult cache using the varargs form, so the optimizer needs to do the lookup beforewriting the argtypes in order to avoid cache misses

base/compiler/optimize.jl

@@ -1294,35 +1294,21 @@ function inlineable(@nospecialize(f), @nospecialize(ft), e::Expr, atypes::Vector
         end
     end
 
-    argexprs0 = argexprs
-    atypes0 = atypes
     na = Int(method.nargs)
-    # check for vararg function
-    isva = false
-    if na > 0 && method.isva
-        @assert length(argexprs) >= na - 1
-        # construct tuple-forming expression for argument tail
-        vararg = mk_tuplecall(argexprs[na:end], sv)
-        argexprs = Any[argexprs[1:(na - 1)]..., vararg]
-        atypes = Any[atypes[1:(na - 1)]..., vararg.typ]
-        isva = true
-    elseif na != length(argexprs)
+    if method.isva ? length(argexprs) < na : na != length(argexprs)
         # we have a method match only because an earlier
         # inference step shortened our call args list, even
         # though we have too many arguments to actually
         # call this function
         return NOT_FOUND
     end
-
-    @assert na == length(argexprs)
-
     for i = 1:length(methsp)
         isa(methsp[i], TypeVar) && return NOT_FOUND
     end
 
     # see if the method has been previously inferred (and cached)
     linfo = code_for_method(method, metharg, methsp, sv.params.world, true) # Union{Nothing, MethodInstance}
-    isa(linfo, MethodInstance) || return invoke_NF(argexprs0, e.typ, atypes0, sv,
+    isa(linfo, MethodInstance) || return invoke_NF(argexprs, e.typ, atypes, sv,
                                                    atype_unlimited, invoke_data)
     linfo = linfo::MethodInstance
     if invoke_api(linfo) == 2
@@ -1379,7 +1365,7 @@ function inlineable(@nospecialize(f), @nospecialize(ft), e::Expr, atypes::Vector
         src_inlineable = ccall(:jl_ast_flag_inlineable, Bool, (Any,), inferred)
     end
     if !src_inferred || !src_inlineable
-        return invoke_NF(argexprs0, e.typ, atypes0, sv, atype_unlimited,
+        return invoke_NF(argexprs, e.typ, atypes, sv, atype_unlimited,
                          invoke_data)
     end
 
@@ -1399,18 +1385,27 @@ function inlineable(@nospecialize(f), @nospecialize(ft), e::Expr, atypes::Vector
     body = Expr(:block)
     body.args = ast
 
-    # see if each argument occurs only once in the body expression
     stmts = []
     prelude_stmts = []
     stmts_free = true # true = all entries of stmts are effect_free
 
-    argexprs = copy(argexprs)
-    if isva
+    # check for vararg function
+    if na > 0 && method.isva
+        @assert length(argexprs) >= na - 1
+        # construct tuple-forming expression for argument tail
+        vararg = mk_tuplecall(argexprs[na:end], sv)
+        argexprs = Any[argexprs[1:(na - 1)]..., vararg]
+        atypes = Any[atypes[1:(na - 1)]..., vararg.typ]
         # move constructed vararg tuple to an ssavalue
         varargvar = newvar!(sv, atypes[na])
         push!(prelude_stmts, Expr(:(=), varargvar, argexprs[na]))
         argexprs[na] = varargvar
+    else
+        argexprs = copy(argexprs)
     end
+    @assert na == length(argexprs)
+
+    # see if each argument occurs only once in the body expression
     for i = na:-1:1 # stmts_free needs to be calculated in reverse-argument order
         #args_i = args[i]
         aei = argexprs[i]

test/compiler/compiler.jl

@@ -1483,8 +1483,6 @@ g26172(v) = (nothing, g26172(f26172(v))...)
 @test @inferred(g26172(Val(10))) === ntuple(_ -> nothing, 10)
 
 # 26826 constant prop through varargs
-# we use getproperty to drive these tests because it requires constant
-# propagation in order to lower to a well-inferred getfield call.
 
 struct Foo26826{A,B}
     a::A
@@ -1496,6 +1494,8 @@ 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.
 
 @test @inferred(f26826(x26826)) === x26826
 
@@ -1504,3 +1504,20 @@ getfield26826(x, args...) = Base.getproperty(x, getfield(args, 2))
 g26826(x) = getfield26826(x, :a, :b)
 
 @test @inferred(g26826(Foo26826(1, x26826))) === x26826
+
+# Somewhere in here should be a single getfield call, and it should be inferred as Float64.
+# If this test is broken (especially if inference is getting a correct, but loose result,
+# like a Union) then it's potentially an indication that the optimizer isn't hitting the
+# 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
+    end
+end
+
+@test found_well_typed_getfield_call