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

Loop disappears in Julia v0.7 #1

Closed
chriselrod opened this issue Dec 1, 2017 · 4 comments
Closed

Loop disappears in Julia v0.7 #1

chriselrod opened this issue Dec 1, 2017 · 4 comments

Comments

@chriselrod
Copy link

I'm not yet sure what the problem is. On 0.7:

julia> using Unrolled

julia> @unroll function my_sum(seq)
                  # More on why we need @unroll twice later.
           total = zero(eltype(seq))
           @unroll for x in seq
               total += x
           end
           return total
       end
my_sum_unrolled_expansion_ (generic function with 1 method)

julia> my_sum((1, 2, 3))
0

Perhaps the problem is associated with the contents of the quoted loop body also being wrapped in an expression?

julia> my_sum_unrolled_expansion_((1,2,3))
quote  # /home/chris/.julia/v0.6/Unrolled/src/Unrolled.jl, line 105:
    total = zero(eltype(seq))
    Unrolled.@unroll_loop (1, 2, 3) for x = seq # /home/chris/.julia/v0.6/Unrolled/src/Unrolled.jl, line 94:
            total += x
        end
    return total
end

and v0.7:

julia> my_sum_unrolled_expansion_((1,2,3))
quote
    #= /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:106 =#
    total = zero(eltype(seq))
    #= /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:94 =# Unrolled.@unroll_loop (1, 2, 3) for x = seq
            #= /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:95 =#
            (:(total += x),)
        end
    return total
end

An example with two lines in the loop body, in v0.7:

julia> @unroll function sum_and_prod(seq)
           # More on why we need @unroll twice later.
           total = zero(eltype(seq))
           product = one(eltype(seq))
           @unroll for x in seq
               total += x
               product *= x
           end
           total, product
       end
sum_and_prod_unrolled_expansion_ (generic function with 1 method)

julia> sum_and_prod((1,2,3))
(0, 1)

julia> sum_and_prod_unrolled_expansion_((1,2,3))
quote
    #= /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:106 =#
    total = zero(eltype(seq))
    product = one(eltype(seq))
    #= /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:94 =# Unrolled.@unroll_loop (1, 2, 3) for x = seq
            #= /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:95 =#
            (:(total += x), :(product *= x))
        end
    (total, product)
end

In v0.6:

julia> sum_and_prod((1,2,3))
(6, 6)

julia> sum_and_prod_unrolled_expansion_((1,2,3))
quote  # /home/chris/.julia/v0.6/Unrolled/src/Unrolled.jl, line 105:
    total = zero(eltype(seq))
    product = one(eltype(seq))
    Unrolled.@unroll_loop (1, 2, 3) for x = seq # /home/chris/.julia/v0.6/Unrolled/src/Unrolled.jl, line 94:
            total += x
            product *= x
        end
    (total, product)
end

A couple more issues:

  • @unroll currently doesn't support where T syntax.
  • It's easy enough to add oneself, but I find it helpful to support unrolling based on passed Vals: Unrolled.type_length(::Type{Val{N}}) where N = N; perhaps this is something people would find generally useful.
@cstjean
Copy link
Owner

cstjean commented Dec 2, 2017

Thank you for the nicely detailed bug report. I'm not sure what's going on, but I'll try to have a look when I get a chance. Sounds like a bug in 0.7.

Generated functions have changed a lot in 0.7. This will allow us to get rid of the ugly @unroll in front of function, and only keep it in front of for. This will also solve the where T syntax problem. I'm not sure that I'll have time for all that, but PRs are very welcome. We could drop support for 0.6, since it'll be awkward to support both at the same time.

It's easy enough to add oneself, but I find it helpful to support unrolling based on passed Vals: Unrolled.type_length(::Type{Val{N}}) where N = N; perhaps this is something people would find generally useful.

That's already supported (... essentially) by passing @fixed_range(1:N). I just neglected to advertise it in the README.

@chriselrod
Copy link
Author

chriselrod commented Dec 3, 2017

Great, @fixed_range seems to be what I was looking for.

Just realized that I had already made some changes to my above examples. On the current master, I get:

julia> using Unrolled
INFO: Recompiling stale cache file /home/chris/.julia/lib/v0.7/Unrolled.ji for module Unrolled.
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)
'znver1' is not a recognized processor for this target (ignoring processor)

WARNING: deprecated syntax "(loopbody...)" at /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:33.
Use "(loopbody...,)" instead.
ERROR: LoadError: syntax: expected ")"
Stacktrace:
 [1] include_relative(::Module, ::String) at ./loading.jl:509
 [2] include(::Module, ::String) at ./sysimg.jl:15
 [3] top-level scope
 [4] top-level scope at ./<missing>:2
in expression starting at /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:33
ERROR: Failed to precompile Unrolled to /home/chris/.julia/lib/v0.7/Unrolled.ji.
Stacktrace:
 [1] compilecache(::String) at ./loading.jl:637
 [2] _require(::Symbol) at ./loading.jl:426
 [3] require(::Symbol) at ./loading.jl:300

Example change that lets us precompile the module, but then gives us the incorrect _unrolled_expansion_ function:

macro unroll_loop(niter_type::Type, loop)
    local niter   # necessary on 0.5
    @assert(@capture(loop, for var_ in seq_ loopbody__ end),
            "Internal error in @unroll_loop")
    try
        niter = type_length(niter_type)
    catch e
        # Don't unroll the loop, we can't figure out its length
        if isa(e, MethodError)
            return esc(loop)
        else rethrow() end
    end
    esc(
        quote
            $Unrolled.@unroll_loop $niter for $var in $seq
          $(loopbody...) end
        end
        )
end

Could we fix this by replace the splatting of loopbody with a loop?
I'm starting to read your link, to see if I can learn anything suggesting a fix. I'm unfamiliar with metaprogramming, but when classes end in a couple weeks I will be eager to learn more.
EDIT: But getting to avoid using @unroll in front of the function definition sounds like you'll probably want to rework things a lot anyway to take advantage of the changes. Not too much point in just patching things.

@fixed_range(1:N) looks great, but it currently doesn't support working around the no-parameterized functions issue:

julia> @unroll function my_sum(x, ::Val{N}) where N
           out = zero(eltype(x))
           @unroll for i  @fixed_range(1:N)
               out += x[i]
           end
           out
       end
ERROR: LoadError: AssertionError: `@unroll` must precede a function definition
Stacktrace:
 [1] @unroll(::LineNumberNode, ::Module, ::Any) at /home/chris/.julia/v0.7/Unrolled/src/Unrolled.jl:92
in expression starting at REPL[16]:1

while defining a type_length for val does:

julia> Unrolled.type_length(::Type{Val{N}}) where N = N

julia> @unroll function my_sum(x, vn)
           out = zero(eltype(x))
           @unroll for i  1:length(vn)
               out += x[i]
           end
           out
       end
my_sum_unrolled_expansion_ (generic function with 1 method)

julia> my_sum(rand(32), Val(32))
15.151042302254876

@cstjean
Copy link
Owner

cstjean commented Dec 4, 2017

I'm starting to read your link, to see if I can learn anything suggesting a fix. I'm unfamiliar with metaprogramming

Then fixing @unroll might be a bit out reach :). It's a macro that expands into a generated function that explicitly unrolls the loop. In 0.7, it will be simplified significantly. I'll look into the details this week, I hope.

With regards to your example, @fixed_range is meant to be passed as an argument to an @unroll function. Something like:

@unroll function my_sum(x, range)
           out = zero(eltype(x))
           @unroll for i  range
               out += x[i]
           end
           out
       end

You may call my_sum([1,2,3], @fixed_range(1:3)), or if you want to pass Val, use:

my_sum(x, ::Val{N}) where N = my_sum(x, @fixed_range(1:N))

@cstjean
Copy link
Owner

cstjean commented Jun 12, 2018

Superseded by #3

@cstjean cstjean closed this as completed Jun 12, 2018
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