Skip to content

Commit

Permalink
Fix #31392, unaliasing of broadcast arguments against destinations wi…
Browse files Browse the repository at this point in the history
…th repeated indices

When introducing the new broadcasting implementation (which came alongside greater
semantic guarantees with regards to aliasing source and destination), I found that
we needed a performance optimization for the common case where the destination was
`===` to an argument. This is quite common due to the `.op=` syntax and indeed is
safe in most cases as the iteration order between the destination and the source
is matched. This performance optimization is not safe, however, in the case that
the destination has multiple locations that refer to the same memory.
  • Loading branch information
mbauman authored and JeffBezanson committed May 18, 2019
1 parent 4a38e79 commit d64f74c
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 7 deletions.
33 changes: 26 additions & 7 deletions base/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -854,15 +854,34 @@ end
end
end

# For broadcasted assignments like `broadcast!(f, A, ..., A, ...)`, where `A`
# appears on both the LHS and the RHS of the `.=`, then we know we're only
# going to make one pass through the array, and even though `A` is aliasing
# against itself, the mutations won't affect the result as the indices on the
# LHS and RHS will always match. This is not true in general, but with the `.op=`
# syntax it's fairly common for an argument to be `===` a source.
broadcast_unalias(dest, src) = dest === src ? src : unalias(dest, src)
"""
broadcast_unalias(dest, src)
`broadcast_unalias` is a simple extension of `Base.unalias` to enable an important
performance optimization specific to the semantics of broadcasting.
For broadcasted assignments like `broadcast!(f, A, ..., A, ...)`, where `A`
appears on both the LHS and the RHS of the `.=`, then we know we're only
going to make one pass through the array, and even though `A` is aliasing
against itself, the mutations won't affect the result as the indices on the
LHS and RHS will always match. As long as the indices do not repeat, the
RHS does not need to be unaliased. This is particuclarly valuable with the `.op=`
syntax since that makes it fairly common for an argument to be `===` a source.
"""
broadcast_unalias(dest, src) = dest === src && !potentially_self_aliased(dest) ? src : unalias(dest, src)
broadcast_unalias(::Nothing, src) = src

"""
potentially_self_aliased(A)
Conservatively returns true if multiple locations in `A` might reference the same memory
"""
potentially_self_aliased(::DenseArray) = false
potentially_self_aliased(A::StridedArray) = any(==(0), strides(A))
potentially_self_aliased(A::SubArray) = any(!allunique, A.indices)
potentially_self_aliased(A::Union{Base.ReshapedArray,Base.ReinterpretArray}) = potentially_self_aliased(A.parent)
potentially_self_aliased(::Any) = true

# Preprocessing a `Broadcasted` does two things:
# * unaliases any arguments from `dest`
# * "extrudes" the arguments where it is advantageous to pre-compute the broadcasted indices
Expand Down
33 changes: 33 additions & 0 deletions test/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,39 @@ end
@test iszero.((big(0):big(2)) .- 1) == [false, true, false]
end

@testset "Issue #31392: In-place broadcasting with repeated indices" begin
x = [3]
@views x[[1,1]] .= x[[1,1]] .+ x[[1,1]]
@test x[] == 6
x[[1,1]] .+= 1
@test x[] == 7

x = [3]
@views x[[1,1]] .+= x[[1,1]]
@test x[] == 6
x[[1,1]] .+= 1
@test x[] == 7

for I in ([1,1], StepRangeLen(1, 0, 2))
x = [3]
@views x[I] .= x[I] .+ x[I]
@test x[] == 6

x = [3]
@views x[I] .+= x[I]
@test x[] == 6

for v in (view([3], I), reshape(view([3], I), 1, 2))
v .= v .+ v
@test v[1] == 6
v .+= v
@test v[1] == 12
v .+= 1
@test v[1] == 13
end
end
end

@testset "Issue #27775: Broadcast!ing over nested scalar operations" begin
a = zeros(2)
a .= 1 ./ (1 + 2)
Expand Down

0 comments on commit d64f74c

Please sign in to comment.