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

Can we have type Box{T} instead just Box? #16431

Closed
ninegua opened this issue May 18, 2016 · 4 comments
Closed

Can we have type Box{T} instead just Box? #16431

ninegua opened this issue May 18, 2016 · 4 comments
Assignees

Comments

@ninegua
Copy link

ninegua commented May 18, 2016

Problem: In Julia 0.5, the Box type is too opaque, which prevents more accurate type inference.

Example:

julia> f(x) = let z = x * 2; @noinline g(y) = begin z = z + y; y + x end; z * g(x) end
f (generic function with 1 method)

julia> x=code_typed(f,(Int,))[1]
LambdaInfo for f
:(begin  # REPL[270], line 1:
        z = $(Expr(:new, :(Core.Box)))
        SSAValue(0) = (Base.box)(Int64,(Base.mul_int)(x,2))
        (Core.setfield!)(z,:contents,SSAValue(0))::Int64 # REPL[270], line 1:
        g = $(Expr(:new, #g#3{Int64}, :(x), :(z)))
        g # REPL[270], line 1:
        return (Core.getfield)(z,:contents) * (g)(x)::Int64
    end)

julia> x.slottypes
4-element Array{Any,1}:
 #f         
 Int64      
 Core.Box   
 #g#3{Int64}

julia> x.ssavaluetypes
1-element Array{Any,1}:
 Int64

julia> t=x.slottypes[4]
#g#3{Int64}

julia> y=Core.Inference.typeinf_uncached(t.name.mt.defs.func, Tuple{t,Int}, Base.svec(t), true)[1]
LambdaInfo for g
:(begin  # REPL[270], line 1: # REPL[270], line 1:
        SSAValue(0) = (Core.getfield)((Core.getfield)(#self#,:z),:contents) + y
        (Core.setfield!)((Core.getfield)(#self#,:z),:contents,SSAValue(0)) # REPL[270], line 1:
        return (Base.box)(Int64,(Base.add_int)(y,(Core.getfield)(#self#,:x)::Int64))
    end::Int64)

julia> y.slottypes
2-element Array{Any,1}:
 #g#3{Int64}
 Int64      

julia> y.ssavaluetypes
1-element Array{Any,1}:
 Any

Function f is purposely constructed so that variable z is modified by inner function g, while x is not. We can see from g's generated code that indeed z is passed as a Box, while x is not.

However, variable SSAValue(0) in g is inferred to have Any type because, well, the :content of z is inferred to have Any type. The implication is that all further arithmetic operations or values that are computed from z will be given Any type.

Had we given z a type of Box{Int64}, we will not have this kind of problem.

Since z is mutable, and could be modified by inner closures to values other than Int, so we really do not have sufficient type information when inferencing f. Yes, this is a valid concern, so in this case we should give it type Box{Any}. However, , if we write f as:

f(x) = let z::Int = x * 2; @noinline g(y) = begin z = z + y; y + x end; z * g(x) end

Then we should be able to safely infer z to be Box{Int64} instead of Box{Any}. Any runtime assignment of its content to values other than Int64 would trigger an error, like what we already do.

@JeffBezanson
Copy link
Sponsor Member

Yes, this is planned.

@ninegua
Copy link
Author

ninegua commented May 18, 2016

Thanks @JeffBezanson . Will it go into 0.5 before release? We've mostly ported ParallelAccelerator to work with current 0.5 master, but this issue is preventing several important workloads from working.

@JeffBezanson
Copy link
Sponsor Member

Yes, we will need some fix for this in 0.5.

I realized another approach is possible too: instead of Box{T}, we could annotate each use of the variable with a type assert. One reason to do this is that in Box{Int} there is no way for the data field to give an error if it is used uninitialized. Would that be sufficient?

@ninegua
Copy link
Author

ninegua commented May 18, 2016

What we really want is that the type inference should give SSAValue(0) in function g in the above example an Int type. Whether it is done through Box{T} or some other means is up to your decision.

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