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.

Sign up for GitHub

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

Jump to bottom

Bug in scalar reduction? #251

Closed
mcabbott opened this issue Apr 26, 2021 · 3 comments
Closed

Bug in scalar reduction? #251

mcabbott opened this issue Apr 26, 2021 · 3 comments

Comments

@mcabbott
Copy link

One more! Again from CI, not sure why this case should be problematic:

function fwd(r22::AbstractArray{T}, x, r312, ax_b=axes(x,2), ax_β=axes(r312,2), ax_a=axes(r312,2), ax_c=axes(r312,1)) where T
        acc = zero(T)
        # for c in ax_c  # works fine without @avx
        @avx for c in ax_c
            for a in ax_a
                for β in ax_β
                    for b in ax_b
                        acc = acc + r22[b, β] * x[a, b, c] * r312[c, a, β]
                    end
                end
            end
        end
        acc
end
r22, x, r312 = rand(2,2), rand(1,2,3), rand(3,1,2)
fwd(r22, x, r312) # UndefVarError: ####op#568_0 not defined

fwd(rand(3,3), rand(3,3,3), rand(3,3,3)) # similar, so not about size=1 dimensions
@chriselrod
Copy link
Member

chriselrod commented Apr 26, 2021 •
edited
Loading

There's the joke about cache invalidation and naming things being the two hardest things in programming.
I can't speak about the former, but LoopVectorizaiton.jl doesn't get the joke and certainly thinks it is the latter (a huge proportion of bugs are just because it named a variable differently in different parts of the code)...

The rewrite will handle that totally differently.

@mcabbott
Copy link
Author

Proof that gensym() doesn't solve all naming difficulties!

But sure, no rush, this one's skipped in testing & didn't bite anyone in real life.

This was referenced Apr 27, 2021
Closed
Merged
@chriselrod
Copy link
Member

Fixed on master, releasing soon.
With AVX512, it is also much faster now than it was when/if it worked earlier, due to adding a workaround for a bug in LLVM where it starts spilling registers to an absurd degree (when it shouldn't be spilling at all) when there are too many phi nodes, or something of that sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@chriselrod @mcabbott