-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
eliminate the dead iterate
branch in _unsafe_(get)setindex!
.
#52809
Conversation
I mean, |
julia> a = zeros(Int, 100, 100);
julia> @btime $a[:,:] = $(1:10000);
1.410 μs (0 allocations: 0 bytes) #master: 3.350 μs (0 allocations: 0 bytes)
julia> @btime $a[:,:] = $(view(LinearIndices(a), 1:100, 1:100));
7.475 μs (0 allocations: 0 bytes) #master: 11.000 μs (0 allocations: 0 bytes) |
@nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. |
There appears to be a mild regression on this PR: julia> using LinearAlgebra
julia> U = UpperTriangular(rand(100,100)); C = similar(U);
julia> @btime $C .= 2.0 .* $U;
1.867 μs (0 allocations: 0 bytes) # master, commit 2afc20c18a8
2.166 μs (0 allocations: 0 bytes) # PR, N5N3:simd_index/83cb107971f
julia> versioninfo()
Julia Version 1.11.0-DEV.1233
Commit 83cb107971f (2024-01-08 14:47 UTC)
Build Info:
Official https://julialang.org/ release
Platform Info:
OS: Linux (x86_64-linux-gnu)
CPU: 8 × 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
WORD_SIZE: 64
LLVM: libLLVM-15.0.7 (ORCJIT, tigerlake)
Threads: 1 default, 0 interactive, 1 GC (on 8 virtual cores)
Environment:
LD_LIBRARY_PATH = :/usr/lib/x86_64-linux-gnu/gtk-3.0/modules
JULIA_EDITOR = subl |
IIUC, this pr didn't touch the broadcast (and scalar indexing) so this looks like a bench noise, or an improvement comes from LLVM upgrade (assuming you use the latest master) |
Gentle bump, is there work that remains in this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Should we call this _unchecked_iterate
though, to indicate that it doesn't check for termination, or _prechecked_iterate
to indicate the caller is supposed to have checked, instead of _checked_iterate
?
by eliminating the dead branch in `iterate` manually.
renaming to `_prechecked_iterate` Co-Authored-By: Jameson Nash <[email protected]>
It's sad that compiler can't do this automatically.
Some benchmark with
setindex!
:So at leastIndexLinear
case get vectorized, which should be good forgetindex
as most of it results areIndexLinear
.Edit: Oops, just found thatsetindex!
use anAbstractArray
-based iteration. So the acceleration is quite limited.Since #51071 we can't assuming all
AbstractArray
supports indexing, so switch toeachindex
based iteration might be breaking.BTW optimization for
FastSubArray
introduced in #45371 still work after this change as the parent array might have their owncopyto!
optimization.