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 fails when adding a generator to the function. #45725

Closed
gbaraldi opened this issue Jun 17, 2022 · 3 comments
Closed

Inference fails when adding a generator to the function. #45725

gbaraldi opened this issue Jun 17, 2022 · 3 comments

Comments

@gbaraldi
Copy link
Member

@Moelf found something in stack overflow where lowering/inference is doing something weird where we box a variable that doesn't need to be boxed, then check if it exists even if it existing isn't conditional which then leads to bad inference.

julia> function testMaxWeird!(x,X,β)
           xmax = 0.0
           @inbounds for i  eachindex(x)
               x[i] = X[i,2] * β[1] + X[i,2] * β[2]
               if x[i] > xmax 
                   xmax = x[i]
               end 
           end 
           y = sum(exp(xj-xmax) for xj  x)
           return xmax
       end

julia> function testMaxWeird2!(x,X,β)
           xmax = 0.0
           @inbounds for i  eachindex(x)
               x[i] = X[i,2] * β[1] + X[i,2] * β[2]
               if x[i] > xmax 
                   xmax = x[i]
               end 
           end 
           # y = sum(exp(xj-xmax) for xj ∈ x)
           return xmax
       end

julia> H = 10000;

julia> X = rand(H,2);

julia> β = rand(2);

julia> x = zeros(H);

julia> @code_warntype testMaxWeird!(x,X,β)
MethodInstance for testMaxWeird!(::Vector{Float64}, ::Matrix{Float64}, ::Vector{Float64})
  from testMaxWeird!(x, X, β) in Main at REPL[13]:1
Arguments
  #self#::Core.Const(testMaxWeird!)
  x::Vector{Float64}
  X::Matrix{Float64}
  β::Vector{Float64}
Locals
  #7::var"#7#8"
  @_6::Union{Nothing, Tuple{Int64, Int64}}
  val::Nothing
  y::Any
  xmax@_9::Core.Box
  i::Int64
  xmax@_11::Union{}
  xmax@_12::Union{}
Body::Any
1 ──       Core.NewvarNode(:(#7))
│          Core.NewvarNode(:(val))
│          Core.NewvarNode(:(y))
│          (xmax@_9 = Core.Box())
│          Core.setfield!(xmax@_9, :contents, 0.0)
│          nothing%7  = Main.eachindex(x)::Base.OneTo{Int64}
│          (@_6 = Base.iterate(%7))
│    %9  = (@_6 === nothing)::Bool%10 = Base.not_int(%9)::Bool
└───       goto #9 if not %10
2 ┄─ %12 = @_6::Tuple{Int64, Int64}
│          (i = Core.getfield(%12, 1))
│    %14 = Core.getfield(%12, 2)::Int64%15 = Base.getindex(X, i, 2)::Float64%16 = Base.getindex(β, 1)::Float64%17 = (%15 * %16)::Float64%18 = Base.getindex(X, i, 2)::Float64%19 = Base.getindex(β, 2)::Float64%20 = (%18 * %19)::Float64%21 = (%17 + %20)::Float64
│          Base.setindex!(x, %21, i)
│    %23 = Base.getindex(x, i)::Float64%24 = Core.isdefined(xmax@_9, :contents)::Bool
└───       goto #4 if not %24
3 ──       goto #5
4 ──       Core.NewvarNode(:(xmax@_11))
└───       xmax@_11
5 ┄─ %29 = Core.getfield(xmax@_9, :contents)::Any%30 = (%23 > %29)::Any
└───       goto #7 if not %30
6 ── %32 = Base.getindex(x, i)::Float64
└───       Core.setfield!(xmax@_9, :contents, %32)
7 ┄─       (@_6 = Base.iterate(%7, %14))
│    %35 = (@_6 === nothing)::Bool%36 = Base.not_int(%35)::Bool
└───       goto #9 if not %36
8 ──       goto #2
9 ┄─       (val = nothing)
│          nothing
│          val
│          (#7 = %new(Main.:(var"#7#8"), xmax@_9))%43 = #7::var"#7#8"%44 = Base.Generator(%43, x)::Base.Generator{Vector{Float64}, var"#7#8"}
│          (y = Main.sum(%44))
│    %46 = Core.isdefined(xmax@_9, :contents)::Bool
└───       goto #11 if not %46
10 ─       goto #12
11 ─       Core.NewvarNode(:(xmax@_12))
└───       xmax@_12
12%51 = Core.getfield(xmax@_9, :contents)::Any
└───       return %5

julia> @code_warntype testMaxWeird2!(x,X,β)
MethodInstance for testMaxWeird2!(::Vector{Float64}, ::Matrix{Float64}, ::Vector{Float64})
  from testMaxWeird2!(x, X, β) in Main at REPL[2]:1
Arguments
  #self#::Core.Const(testMaxWeird2!)
  x::Vector{Float64}
  X::Matrix{Float64}
  β::Vector{Float64}
Locals
  @_5::Union{Nothing, Tuple{Int64, Int64}}
  val::Nothing
  xmax::Float64
  i::Int64
Body::Float64
1 ─       Core.NewvarNode(:(val))
│         (xmax = 0.0)
│         nothing%4  = Main.eachindex(x)::Base.OneTo{Int64}
│         (@_5 = Base.iterate(%4))
│   %6  = (@_5 === nothing)::Bool%7  = Base.not_int(%6)::Bool
└──       goto #6 if not %7
2%9  = @_5::Tuple{Int64, Int64}
│         (i = Core.getfield(%9, 1))
│   %11 = Core.getfield(%9, 2)::Int64%12 = Base.getindex(X, i, 2)::Float64%13 = Base.getindex(β, 1)::Float64%14 = (%12 * %13)::Float64%15 = Base.getindex(X, i, 2)::Float64%16 = Base.getindex(β, 2)::Float64%17 = (%15 * %16)::Float64%18 = (%14 + %17)::Float64
│         Base.setindex!(x, %18, i)
│   %20 = Base.getindex(x, i)::Float64%21 = (%20 > xmax)::Bool
└──       goto #4 if not %21
3 ─       (xmax = Base.getindex(x, i))
4 ┄       (@_5 = Base.iterate(%4, %11))
│   %25 = (@_5 === nothing)::Bool%26 = Base.not_int(%25)::Bool
└──       goto #6 if not %26
5 ─       goto #2
6 ┄       (val = nothing)
│         nothing
│         val
└──       return xmax
@KristofferC
Copy link
Sponsor Member

Dup of #15276?

@gbaraldi
Copy link
Member Author

It might be that. Adding a type to xmax doesn't help also for reference, it makes it a bit better but it still boxes the variable which makes it bad. This issue is always rearing it's head, on super innocent code even.

@gbaraldi
Copy link
Member Author

Closing as it's a dup of #15276 again

This issue was closed.
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