Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inference regression (julia 0.5) on types-as-arguments #19096

Closed
timholy opened this issue Oct 25, 2016 · 4 comments
Closed

Inference regression (julia 0.5) on types-as-arguments #19096

timholy opened this issue Oct 25, 2016 · 4 comments
Labels
compiler:inference Type inference regression Regression in behavior compared to a previous version

Comments

@timholy
Copy link
Sponsor Member

timholy commented Oct 25, 2016

julia 0.4 can handle a function definition like this:

julia> myrand(args...) = rand(args...)
myrand (generic function with 1 method)

even when one of the passed arguments is a type:

julia> @code_warntype myrand(Int32, 4096)
Variables:
  args::Tuple{Type{Int32},Int64}
  ##dims#7150::Tuple{}

Body:
  begin  # none, line 1:
      return (Base.Random.rand!)(Base.Random.GLOBAL_RNG,(top(ccall))(:jl_new_array,(top(apply_type))(Base.Array,Int32,1)::Type{Array{Int32,1}},(top(svec))(Base.Any,Base.Any)::SimpleVector,Array{Int32,1},0,(Base.Random.tuple)((top(getfield))(args::Tuple{Type{Int32},Int64},2)::Int64)::Tuple{Int64},0)::Array{Int32,1})::Array{Int32,1}
  end::Array{Int32,1}

In contrast, with julia 0.5:

julia> @code_warntype myrand(Int32, 4096)
Variables:
  #self#::#myrand
  args::Tuple{DataType,Int64}

Body:
  begin 
      return (Base.Random.rand!)(Base.Random.GLOBAL_RNG,((Core.apply_type)(Base.Random.Array,(Core.getfield)(args::Tuple{DataType,Int64},1)::DataType)::Type{_<:Array})((Base.Random.tuple)((Core.getfield)(args::Tuple{DataType,Int64},2)::Int64)::Tuple{Int64})::Array{T,N})::Any
  end::Any

even though rand itself is perfectly fine:

julia> @code_warntype rand(Int32, 4096)
Variables:
  #self#::Base.Random.#rand
  T::Type{Int32}
  d1::Int64
  dims::Tuple{}

Body:
  begin 
      return $(Expr(:invoke, LambdaInfo for rand!(::MersenneTwister, ::Array{Int32,1}), :(Base.Random.rand!), :(Base.Random.GLOBAL_RNG), :((Core.ccall)(:jl_new_array,(Core.apply_type)(Core.Array,Int32,1)::Type{Array{Int32,1}},(Core.svec)(Core.Any,Core.Any)::SimpleVector,Array{Int32,1},0,(Base.Random.tuple)(d1)::Tuple{Int64},0)::Array{Int32,1})))
  end::Array{Int32,1}

This was the subject of multiple PRs during the final days of 0.5 development (e.g., #17794, #18107), but perhaps we failed to notice the more systematic source, that this wasn't something we had to worry about in julia 0.4.

Also, this solves a mystery: since BaseBenchmark's samerand function uses this exact construction, this is almost surely the source of the massive performance regressions seen on the SIMD benchmarks with julia 0.5. (#16128 (comment)).

@timholy timholy added regression Regression in behavior compared to a previous version compiler:inference Type inference labels Oct 25, 2016
@vtjnash
Copy link
Sponsor Member

vtjnash commented Oct 25, 2016

Note that in v0.4, it inferred the wrong type for args, which let it give slightly better information in some cases, at the cost of giving wrong information in other cases.

@Sacha0
Copy link
Member

Sacha0 commented Oct 26, 2016

Another example:

julia> VERSION
v"0.6.0-dev.1079"

julia> begin
           pareval(h, x) = y -> h(x, y)
           evalparevald(hx, y) = hx(y)
           evalbypareval(h, x, y) = evalparevald(pareval(h, x), y)
       end;

julia> @code_warntype evalbypareval((x,y) -> round(x,y), Int, 1.0)
Variables:
  #self#::#evalbypareval
  h::##3#4
  x::Type{Int64}
  y::Float64
  #1::##1#2{##3#4,DataType}

Body:
  begin
      $(Expr(:inbounds, false))
      # meta: location REPL[2] pareval 2
      SSAValue(0) = h::##3#4
      SSAValue(1) = x::Type{Int64}
      # meta: pop location
      $(Expr(:inbounds, :pop))
      return (Main.round)(SSAValue(1),y::Float64)::Any
  end::Any

Workaround thanks to @yuyichao:

begin
    pareval{T}(h, ::Type{T}) = y -> h(T, y)
    evalparevald(hx, y) = hx(y)
    evalbypareval{T}(h, ::Type{T}, y) = evalparevald(pareval(h, T), y)
end;

Best!

@vtjnash
Copy link
Sponsor Member

vtjnash commented Oct 27, 2016

That's a somewhat different issue, and is basically a special case of wanting inference to do type/constant propagation across memory load/store operations.

@KristofferC
Copy link
Sponsor Member

julia> @code_warntype myrand(Int32, 4096)
...
  end::Array{Int32,1}

Sachas comment seems dup of #23471

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

4 participants