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

add static codegen for jl_is_leaf_type and jl_subtype #7088

Merged
merged 1 commit into from
Jun 3, 2014
Merged

add static codegen for jl_is_leaf_type and jl_subtype #7088

merged 1 commit into from
Jun 3, 2014

Conversation

carnaval
Copy link
Contributor

@carnaval carnaval commented Jun 2, 2014

solves some part of #7060. I'm not so sure about method_exists because you can add methods after codegen.

@JeffBezanson
Copy link
Member

Failure is linalg-test-threshold-related.

@JeffBezanson
Copy link
Member

Nice, I was hoping somebody would get around to adding this.

JeffBezanson added a commit that referenced this pull request Jun 3, 2014
add static codegen for jl_is_leaf_type and jl_subtype
@JeffBezanson JeffBezanson merged commit 3b94b84 into JuliaLang:master Jun 3, 2014
@stevengj
Copy link
Member

stevengj commented Jun 3, 2014

Regarding method_exists, I'm not sure I see the issue with methods being added after codegen. Adding methods after codegen does not affect other previously generated method calls, after all, e.g.:

f(x) = x + 1
g(x) = f(x) + 2
println(g(1))
f(x::Int) = x + 7
println(g(1))

prints

4
4

So why should method_exists be any different?

@stevengj
Copy link
Member

stevengj commented Jun 3, 2014

Is the isleaftype codegen really done statically now? It doesn't seem to be:

g{T}(x::Array{T}) = isleaftype(T) ? 1 : 2
code_native(g, (Array{Int},))

still spits out a lot more code than it should.

@jiahao
Copy link
Member

jiahao commented Jun 3, 2014

OT: test failure should be addressed by 454d307.

@JeffBezanson
Copy link
Member

You will need to try code_native(g, (Array{Int,1},)).

@JeffBezanson
Copy link
Member

But yes, we are being too conservative in the Array{Int} case.

@timholy
Copy link
Member

timholy commented Jun 3, 2014

Sweet!

Hanging out with Julians makes me feel like I've got a whole flock of fairy godmothers making my life better. Let's see, what have they done today? Faster regex, better static codegen, improved SIMD, more special functions, lifetime supply of jellybeans, better reduction functions...hmm, all in all, a pretty good day.

@ViralBShah
Copy link
Member

You should tweet that!

@timholy
Copy link
Member

timholy commented Jun 3, 2014

What's a tweet? 😄 When it comes to social media, email is about as sophisticated as I get.

@carnaval carnaval deleted the few_known_calls branch June 3, 2014 05:45
@carnaval
Copy link
Contributor Author

carnaval commented Jun 3, 2014

@stevengj For method_exists, I guess it probably wouldn't hurt to evaluate it at compile time, but it does change current behavior which some programs may rely on. If it is not deemed too breaking, I can probably make that change too.
The Array{Int} case seems to be codegen hitting a fallback path because the type is not concrete. No other known_call works in that case either IIUC.

@timholy Am I the only one not aware of the jellybeans thing ?! :-)

@timholy
Copy link
Member

timholy commented Jun 3, 2014

@carnaval, just checking to see if anyone noticed.

@stevengj
Copy link
Member

stevengj commented Jun 3, 2014

One option would be to evaluate method_exists at compile time and only substitute the value into the codegen if it is true, and otherwise leave it as a runtime call.

@stevengj
Copy link
Member

stevengj commented Jun 3, 2014

Also for

g(x::Array) = isleaftype(eltype(x)) ? 1 : 2

the code_native(g, (Array{Int,1},)) output is more than it needs to be.

@KristofferC
Copy link
Member

Was this meant to "fix" this?

julia> f{T}(x::T) = T <: Real ? 1 : 2.0

julia> @code_warntype f(1.0)
Variables:
  x::Float64

Body:
  begin  # none, line 1:
      unless T <: Main.AbstractFloat::Bool goto 0
      return 1
      0: 
      return 2.0
  end::Union{Float64,Int64}

Is there any good way to do what I am trying to do here? I am wrapping a library where the function calls will look very similar except that I need to change the type of a parameter depending on the type of the input. Basically I want to do :

for t in (:StridedVector, :StridedMatrix)
    @eval begin
        function f{T}(B::$t(){T})
            # bunch of stuff same for vector and matrix
            if T <: StridedVector
                 C = zeros(10) # vector
            else
                 C = zeros(10, 10) # matrix
            end
            ccall(blah blah, C, ..)
        end
    end
end

and have it be type stable.

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

Successfully merging this pull request may close these issues.

7 participants