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

Simple type inference failure setting Tuple field in-place on 0.6/0.7 #26956

Closed
marius311 opened this issue May 2, 2018 · 2 comments
Closed

Comments

@marius311
Copy link
Contributor

I just ran into the following surprising (to me) inference failure on both 0.6.2 / 0.7.0-DEV.4959 (output below is for 0.6.2):

julia> zero_field1(t) = (t[1] .= 0)
zero_field1 (generic function with 1 method)

julia> @code_warntype zero_field1(([1.],[2]))
Variables:
  #self# <optimized out>
  t::Tuple{Array{Float64,1},Array{Int64,1}}
  #temp#@_3::Core.MethodInstance
  #temp#@_4::Any

Body:
  begin 
      SSAValue(1) = t::Tuple{Array{Float64,1},Array{Int64,1}}
      unless ((Base.getfield)(SSAValue(1), 1)::Union{Array{Float64,1}, Array{Int64,1}} isa Array{Float64,1})::Bool goto 5
      #temp#@_3::Core.MethodInstance = MethodInstance for broadcast!(::Base.#identity, ::Array{Float64,1}, ::Int64)
      goto 14
      5: 
      unless ((Base.getfield)(SSAValue(1), 1)::Union{Array{Float64,1}, Array{Int64,1}} isa Array{Int64,1})::Bool goto 9
      #temp#@_3::Core.MethodInstance = MethodInstance for broadcast!(::Base.#identity, ::Array{Int64,1}, ::Int64)
      goto 14
      9: 
      goto 11
      11: 
      #temp#@_4::Any = (Base.broadcast!)(Base.identity, (Base.getfield)(SSAValue(1), 1)::Union{Array{Float64,1}, Array{Int64,1}}, 0)::Union{Array{Float64,1}, Array{Int64,1}}
      goto 16
      14: 
      #temp#@_4::Any = $(Expr(:invoke, :(#temp#@_3), :(Base.broadcast!), :(Base.identity), :((Base.getfield)(SSAValue(1), 1)::Union{Array{Float64,1}, Array{Int64,1}}), 0))
      16: 
      return #temp#@_4::Any
  end::Union{Array{Float64,1}, Array{Int64,1}}

It can be fixed in the following odd way (which makes me believe this is more a bug than just a sub-optimality of inference),

julia> zero_field1(t) = (t[1] :: typeof(t[1]) .= 0)
zero_field1 (generic function with 1 method)

julia> @code_warntype zero_field1(([1.],[2]))
Variables:
  #self# <optimized out>
  t::Tuple{Array{Float64,1},Array{Int64,1}}

Body:
  begin 
      return $(Expr(:invoke, MethodInstance for fill!(::Array{Float64,1}, ::Int64), :(Base.Broadcast.fill!), :((Base.getfield)(t, 1)::Array{Float64,1}), 0))
  end::Array{Float64,1}

Hope this is helpful!

@KristofferC
Copy link
Member

This also fixes it

julia> function zero_field1(t)
              x = t[1]
              return x .= 0
          end

Difference in lowered code is

      Core.SSAValue(1) = (Base.dotview)(t, 1)

vs

    x = (Base.getindex)(t, 1)
    Core.SSAValue(1) = x

Diff in inferred is

    # meta: location tuple.jl getindex 22
    Core.SSAValue(15) = (Base.getfield)(t::Tuple{Array{Float64,1},Array{Int64,1}}, 1, true)::Union{Array{Float64,1}, Array{Int64,1}}

vs

    # meta: location tuple.jl getindex 22
    Core.SSAValue(5) = (Base.getfield)(t::Tuple{Array{Float64,1},Array{Int64,1}}, 1, true)::Array{Float64,1

Perhaps the constant propagation doesn't get through the whole dotview-> maybeview machinery.

@KristofferC
Copy link
Member

Fixed now. Probably #26826.

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

2 participants