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

minimum with dims keyword is type-unstable on master #27992

Closed
giordano opened this issue Jul 8, 2018 · 3 comments
Closed

minimum with dims keyword is type-unstable on master #27992

giordano opened this issue Jul 8, 2018 · 3 comments

Comments

@giordano
Copy link
Contributor

giordano commented Jul 8, 2018

Julia 0.6.0:

julia> @code_warntype minimum(rand(5,5), 2)
Variables:
  #self#::Base.#minimum
  A::Array{Float64,2}
  region::Int64

Body:
  begin 
      $(Expr(:inbounds, false))
      # meta: location reducedim.jl minimum 570
      # meta: location reducedim.jl mapreducedim 241
      SSAValue(0) = $(Expr(:invoke, MethodInstance for reducedim_initarray0(::Array{Float64,2}, ::Int64, ::Float64, ::Type{Float64}), :(Base.reducedim_initarray0), :(A), :(region), Inf, Float64))
      # meta: location reducedim.jl mapreducedim! 210
      $(Expr(:invoke, MethodInstance for _mapreducedim!(::Base.#identity, ::Base.#scalarmin, ::Array{Float64,2}, ::Array{Float64,2}), :(Base._mapreducedim!), :(Base.identity), :(Base.scalarmin), SSAValue(0), :(A)))
      # meta: pop location
      # meta: pop location
      # meta: pop location
      $(Expr(:inbounds, :pop))
      return SSAValue(0)
  end::Array{Float64,2}

Julia master:

julia> @code_warntype minimum(rand(5,5), dims=2)
Body::AbstractArray
 1%1  = Base.getfield(%%#temp#, :dims)::Int64                                                            │╻     getindex
 └──       goto 2 if not false                                                                              │╻     isempty
 2%3  = Base.slt_int(0, 1)::Bool                                                                         ││╻╷╷╷  iterate
 └──       goto 4 if not %3                                                                                 │││┃│    iterate
 3 ─       goto 5                                                                                           ││││┃     iterate
 4 ─       invoke Base.getindex(()::Tuple{}, 1::Int64)                                                      │││││ 
 └──       unreachable                                                                                      │││││ 
 5 ─       goto 6                                                                                           ││││  
 6 ─       goto 7                                                                                           ││╻     iterate
 7 ─       goto 8                                                                                           ││    
 8nothing                                                                                          │     
 │   %12 = :(Base.identity)::typeof(identity)                                                               ││╻     _minimum
 │   %13 = :(Base.min)::typeof(min)                                                                         │││╻     _minimum
 │   %14 = invoke Base._mapreduce_dim(%12::Function, %13::Function, :($(QuoteNode(NamedTuple())))::NamedTuple{(),Tuple{}}, %%a::Array{Float64,2}, %1::Int64)::AbstractArray
 └──       return %14

I don't know if it's somewhat related to #27836 and fixed by #27845.

@andreasnoack
Copy link
Member

I don't see this issue when on #27845 but I'm not sure why it would have fixed it. Would have to take a closer look.

@andreasnoack
Copy link
Member

Somehow #27845 also fixed this.

@martinholters
Copy link
Member

We might want to test for this. Probably adding an @inferred to an existing test should do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants