From d64f74c88ed59858defe68b6a319d5b10a09a2f5 Mon Sep 17 00:00:00 2001
From: Matt Bauman <mbauman@gmail.com>
Date: Tue, 19 Mar 2019 12:08:26 -0400
Subject: [PATCH] Fix #31392, unaliasing of broadcast arguments against
 destinations with 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.
---
 base/broadcast.jl | 33 ++++++++++++++++++++++++++-------
 test/broadcast.jl | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/base/broadcast.jl b/base/broadcast.jl
index 4d5ca6b92fe19..ffdd38e083495 100644
--- a/base/broadcast.jl
+++ b/base/broadcast.jl
@@ -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
diff --git a/test/broadcast.jl b/test/broadcast.jl
index 3e39d14cfb440..6e7b8e3d95954 100644
--- a/test/broadcast.jl
+++ b/test/broadcast.jl
@@ -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)