@code_warntype with non-specialized type arguments shows the wrong specialization

[cstjean]

I was informed in #23471 that functions that accept ::Type (vs. ::Type{T} where T) do not specialize on that argument. However, @code_warntype shows a specialized function body (on 0.6):

julia> foo(t::Type) = t
foo (generic function with 1 method)

julia> @code_warntype foo(Int)
Variables:
  #self#::#foo
  t::Type{Int64}

Body:
  begin 
      return t::Type{Int64}
  end::Type{Int64}

This is confirmed by looking at the specializations:

julia> foo(Int)   # run it normally
Int64

julia> Base.visit(println, methods(foo).ms[1].specializations)
MethodInstance for foo(::Type{T} where T)    # specialization triggered by foo(Int)
MethodInstance for foo(::Type{Int64})        # specialization triggered by @code_warntype

I'm guessing that the correct outcome is this:

julia> code_warntype(foo, (Type{T} where T,))
Variables:
  #self#::#foo
  t::Type{T} where T

Body:
  begin 
      return t::Type{T} where T
  end::Type{T} where T

[yuyichao]

The result is correct in that it is what the inference see and uses. Even though the functions is not specialized, the caller will still have good type info.

julia> @noinline foo(t::Type, b) = t(b)
foo (generic function with 1 method)

julia> g() = foo(Int, 1.0)
g (generic function with 1 method)

julia> @code_warntype g()
Variables:
  #self# <optimized out>

Body:
  begin
      return $(Expr(:invoke, MethodInstance for foo(::Type{T} where T, ::Float64), :(Main.foo), :(Main.Int), 1.0))::Int64
  end::Int64

julia> @code_llvm g()

; Function g
; Location: REPL[1]
define i64 @julia_g_63641() #0 {
top:
  %0 = alloca %jl_value_t addrspace(10)*, i32 3
; Location: REPL[1]:1
  %1 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %0, i32 0
  store %jl_value_t addrspace(10)* addrspacecast (%jl_value_t* inttoptr (i64 139909964693592 to %jl_value_t*) to %jl_value_t addrspace(10)*), %jl_value_t addrspace(10)** %1
  %2 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %0, i32 1
  store %jl_value_t addrspace(10)* addrspacecast (%jl_value_t* inttoptr (i64 139909964646192 to %jl_value_t*) to %jl_value_t addrspace(10)*), %jl_value_t addrspace(10)** %2
  %3 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %0, i32 2
  store %jl_value_t addrspace(10)* addrspacecast (%jl_value_t* inttoptr (i64 139909975372304 to %jl_value_t*) to %jl_value_t addrspace(10)*), %jl_value_t addrspace(10)** %3
  %4 = call %jl_value_t addrspace(10)* @jl_invoke(%jl_value_t addrspace(10)* addrspacecast (%jl_value_t* inttoptr (i64 139910009557776 to %jl_value_t*) to %jl_value_t addrspace(10)*), %jl_value_t addrspace(10)** %0, i32 3)
  %5 = bitcast %jl_value_t addrspace(10)* %4 to i64 addrspace(10)*
  %6 = load i64, i64 addrspace(10)* %5, align 8
  ret i64 %6
}

Automatically including specialization will be confusing since the function does not have any type instability, which is what code_warntype is showing. It should be ok to include a message about specialization like what we do with llvm code though.

[cstjean]

Then consider

function foo2(t::Type)
    x = ones(t, 10)
    return sum(map(sin, x))
end

code_warntype(foo2, (Type{T} where T,)) shows plenty of true type instabilities, but @code_warntype foo2(Int) doesn't show any.

[yuyichao]

There's no true type instability since you'll see that the result will be correctly inferred for all callers. It's just not getting specialized. I do agree it's confusing and we should improve tooling. However, the way to improve it is not to hide the difference. Rather, we should make it possible to see both results (we do now) but make it clear to the user (when we print the result) that they should be aware of what they are getting.

[cstjean]

I agree that both versions are useful in their own ways, however

There's no true type instability since you'll see that the result will be correctly inferred for all callers.

The sum call above is done through dynamic dispatch because the argument types weren't fully inferred in the code that was run. Regardless of whether it's a "true" type-instability, that's the piece of information I'm after when I use @code_warntype

In any case, it seems that @code_llvm foo2(Int) suffers from the same issue. It shows Type{Int}-specialized code that will never be executed, and there's no use for it. Ironically, the code produced by code_llvm(foo2, (Type{T} where T,)) will be executed, but it shows an additional warning: WARNING: This code may not match what actually runs.

Wouldn't it be more natural if all of the @code_xxx macros show the code that will be executed, rather than the code that the inference engine sees? We can already check for type-stability of the output via @inferred.

[yuyichao]

The sum call above is done through dynamic dispatch because the argument types weren't fully inferred in the code that was run.

It is inferred, just not specialized. Note that it does not mean there's a performance issue. If the code is inlined, for example, it won't matter at all.

but it shows an additional warning: WARNING: This code may not match what actually runs.

Right, and I'm saying we should add/fix the logic for a similar warning.

The code that actually runs is a more complicated question than what code_** are designed to answer (the large number of optimizations that relies on inlining means that what you actually get might be very different from what you can possibly infer from a single function), which is why I said I would perfer for having all of them answering a simpler question while giving the user enough information about what they are seeing, rather than trying to be smart here and there and guessing what the user want.

[ViralBShah]

+1 to having code_warntype warn the user about this. I ran into this today and it isn't easy to figure this out if you run into it.

[vtjnash]

We now have a function for computing this (ccall(:jl_isa_compileable_sig, Int32, (Any, Any), linfo.specTypes, linfo.def::Method) == 0, added in 5c0b64b, but still has some errors to be shaken out) which tells you whether the runtime thinks this method signature is something we might compile into the cache for this method.

If someone wants to inject this message into the result, just be aware that the text should clarify that this widening for compilation does not affect inlining or the inferred return-type.

[KristofferC]

Dup of #32834. This one was earlier but it is unclear which one is best to be kept open.