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

Force avx to "avx = false" when use LHS scattering, unless the index is checked to be unique. #27

Closed
N5N3 opened this issue Aug 18, 2020 · 3 comments

Comments

@N5N3
Copy link

N5N3 commented Aug 18, 2020

using Tullio
using LoopVectorization
J = [1,3,1,1]
A = [i^2 for i in 1:10]

Example 1(wrong):

@tullio B[J[i],k] := A[k]
@tullio B[J[i],k] += A[k]
# 3×10 Array{Int64,2}:
#  1  4  9  16  25  36  49  64  81  100
#  0  0  0   0   0   0   0   0   0    0
#  1  4  9  16  25  36  49  64  81  100

Example 2(right):

@tullio B[J[i],k] := A[k]
@tullio avx=false B[J[i],k] += A[k]
# 3×10 Array{Int64,2}:
#  4  16  36  64  100  144  196  256  324  400
#  0   0   0   0    0    0    0    0    0    0
#  2   8  18  32   50   72   98  128  162  200

As @avx can't tell the user all the misusing, maybe the default set should be avx = false.

@N5N3 N5N3 changed the title The LHS scatter operation has bug when avx = true. Switch the default set of "avx to avx = false" Aug 18, 2020
@N5N3 N5N3 changed the title Switch the default set of "avx to avx = false" Switch the default set of avx to "avx = false" Aug 18, 2020
@chriselrod
Copy link
Contributor

LoopVectorization issue:
JuliaSIMD/LoopVectorization.jl#145

The problem there is that if you try to write to the same memory location multiple times simultaneously, the answer is going to be undefined (but probably equal to one of those simultaneous write attempts).
This could also be considered a cumsum-like example, in that correct value of memory-reads are also changing between loop iterations.

I'd say this is a problem in documentation. @N5N3 should've been able to figure out that this is expected behavior.

@N5N3 N5N3 changed the title Switch the default set of avx to "avx = false" Force avx to "avx = false" when use LHS scattering, unless the index is checked to be unique. Aug 18, 2020
@mcabbott
Copy link
Owner

mcabbott commented Aug 18, 2020

Thanks for finding this. There is some logic to prevent multi-threading from messing this up, with += it sets unsafeind = [:i] which hides i from threader. It's possible this ought to also disable @avx entirely, I just need to remember how things work. The goal is certainly to make operations safe by default.

@N5N3
Copy link
Author

N5N3 commented Aug 18, 2020

So the SubArray ver is correct only because the size is small and the Multi-threads is not open. If true, I think this might be warned somewhere in the document. As it seem's impossible to check the indices of the SubArray.

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

3 participants