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

Union handling of ifelse #26134

Open
nalimilan opened this issue Feb 20, 2018 · 6 comments
Open

Union handling of ifelse #26134

nalimilan opened this issue Feb 20, 2018 · 6 comments
Labels
compiler:inference Type inference compiler:simd instruction-level vectorization performance Must go faster

Comments

@nalimilan
Copy link
Member

In the following example, SIMD is enabled when x is of the element type of A (here Int8), but not when it is of a different type (e.g. Int). I imagine this can be due to overflow checks, but the compiler should ideally be able to check this before entering the loop. This also happens when x is hardcoded as a constant inside the function body instead of being passed as an argument, in which case the check could happen statically.

julia> using BenchmarkTools

julia> function test!(A::AbstractArray, x)
           @inbounds for i in eachindex(A)
               Ai = A[i]
               A[i] = ifelse(Ai == 1, Ai, x)
           end
           A
       end
test! (generic function with 1 method)

julia> A = rand(Int8, 10_000);

julia> @btime test!(A, 100);
  77.495 μs (0 allocations: 0 bytes)

julia> @btime test!(A, Int8(100));
  208.595 ns (0 allocations: 0 bytes)
@nalimilan nalimilan added performance Must go faster compiler:codegen Generation of LLVM IR and native code labels Feb 20, 2018
@yuyichao yuyichao changed the title SIMD not enabled due to integer type difference SIMD not enabled due to type instability Feb 20, 2018
@yuyichao yuyichao changed the title SIMD not enabled due to type instability Union handling of ifelse Feb 20, 2018
@yuyichao
Copy link
Contributor

The overflow check isn't causing any problem. With the ifelse replaced with branch, the inlining is still not agressive enough for the setindex! though.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Feb 21, 2018

but not when it is of a different type

In general, you should probably stick to using actual conditionals (?:), ifelse is likely to continue to always play second-fiddle. #25934 will probably help, but after Keno's PR, I strongly suspect we should just delete this intrinsic and define it simply as ifelse(b, x, y) = b ? x : y.

@yuyichao yuyichao added compiler:inference Type inference and removed compiler:codegen Generation of LLVM IR and native code labels Feb 21, 2018
@vchuravy
Copy link
Sponsor Member

vchuravy commented Feb 21, 2018 via email

@yuyichao
Copy link
Contributor

Is LLVM not smart enough?

@quinnj
Copy link
Member

quinnj commented Feb 21, 2018

@vtjnash, but are there cases where LLVM wouldn't change ?: to select, but a user might want to force it explicitly for cases like SIMD?

@yuyichao
Copy link
Contributor

yuyichao commented Feb 21, 2018

Note that I believe the hardest (and sometimes impossible) thing for the compiler to prove is that it is safe/beneficial for the two cases be eagerly evaluated. As long as it's clear to LLVM that it's a branch that has no sideeffect, I think it should be pretty trivial for it to do the transformation and any cases where it fails to do so will probably be a LLVM bug that shouldn't be too hard to fix. (edit: in another word, my "Is LLVM not smart enough?" was a serious question about the state of the GPU backend).

It's worth noting that the eager evaluation semantics is actually not provided by select but by the fact that ifelse is a function and not syntax. In another word, I don't think we should remove the function API (i.e. ifelse) but the lower level intrinsics can probably be removed.

@mbauman mbauman added the compiler:simd instruction-level vectorization label Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference compiler:simd instruction-level vectorization performance Must go faster
Projects
None yet
Development

No branches or pull requests

6 participants