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

regression in codegen of array constructors #15044

Closed
JeffBezanson opened this issue Feb 12, 2016 · 2 comments
Closed

regression in codegen of array constructors #15044

JeffBezanson opened this issue Feb 12, 2016 · 2 comments
Labels
performance Must go faster regression Regression in behavior compared to a previous version
Milestone

Comments

@JeffBezanson
Copy link
Member

julia> @code_typed ones(1)
1-element Array{Any,1}:
 :($(Expr(:lambda, Any[symbol("#self#"),:(dims::Any...)], Any[Any[Any[symbol("#self#"),Base.#ones,0],Any[:dims,Tuple{Int64},0]],Any[],Any[Any]], :(begin  # array.jl, line 169: # boot.jl, line 345: # boot.jl, line 331:
        GenSym(0) = (Core.convert)(Core.Int,(top(getfield))(dims::Tuple{Int64},1)::Int64)
        return (Base.fill!)((top(ccall))(:jl_alloc_array_1d,(top(apply_type))(Core.Array,Float64,1)::Type{Array{Float64,1}},(top(svec))(Core.Any,Core.Int)::SimpleVector,Array{Float64,1},0,(top(unsafe_convert))(Core.Int,GenSym(0))::Int64,GenSym(0))::Array{Float64,1},(Base.box)(Float64,(Base.sitofp)(Float64,1)))::Array{Float64,1}
    end::Array{Float64,1}))))

Although we infer the type OK, there is still a call to Core.convert and we generate a call to jl_apply_generic. Most likely a bootstrap artifact due to moving some array constructors to Core.

@JeffBezanson JeffBezanson added performance Must go faster regression Regression in behavior compared to a previous version labels Feb 12, 2016
@mbauman
Copy link
Member

mbauman commented Feb 12, 2016

Looks like I just need to rebase #14441.

@vtjnash
Copy link
Member

vtjnash commented Feb 28, 2016

I suspect this is because Inference is prohibited from inferring code in Core (unless it has sparams). as such, the following works around the issue. afaict, there is no downside either?

diff --git a/base/boot.jl b/base/boot.jl
index d60941b..cb8e21f 100644
--- a/base/boot.jl
+++ b/base/boot.jl
@@ -322,7 +322,7 @@ Task(f::ANY) = ccall(:jl_new_task, Any, (Any, Int), f, 0)::Task
 # so the methods and ccall's in Core aren't permitted to use convert
 convert(::Type{Any}, x::ANY) = x
 convert{T}(::Type{T}, x::T) = x
-cconvert(T::Type, x) = convert(T, x)
+cconvert{T}(::Type{T}, x) = convert(T, x)
 unsafe_convert{T}(::Type{T}, x::T) = x

 # primitive array constructors

@yuyichao yuyichao added this to the 0.5.0 milestone Apr 15, 2016
yuyichao added a commit that referenced this issue Apr 16, 2016
In order to help type inference when calling array constructor.

Fixes #15044
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

4 participants