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

Improve aliasing detection of wrappers of SubArrays #29546

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1128,8 +1128,33 @@ Perform a conservative test to check if arrays `A` and `B` might share the same
By default, this simply checks if either of the arrays reference the same memory
regions, as identified by their [`Base.dataids`](@ref).
"""
mightalias(A::AbstractArray, B::AbstractArray) = !isbits(A) && !isbits(B) && !_isdisjoint(dataids(A), dataids(B))
mightalias(x, y) = false
mightalias(A, B) = _anymightalias(aliasingroots(A), aliasingroots(B))

_anymightalias(::Tuple{}, ::Tuple) = false
_anymightalias(as::Tuple, bs::Tuple) =
any(b -> _mightalias(as[1], b), bs) || _anymightalias(tail(as), bs)

_mightalias(A::AbstractArray, B::AbstractArray) =
!isbits(A) && !isbits(B) && !_isdisjoint(dataids(A), dataids(B))

"""
Base.aliasingroots(A) -> Tuple

Return a tuple of "parent" arrays in which the mutable data for `A` is stored.

Custom arrays that would like to opt-in to aliasing detection of their component
parts can specialize this method to return the concatenation of the
`aliasingroots` of their component parts. A typical definition for an array
that wraps a parent is
`Base.aliasingroots(C::CustomArray) = Base.aliasingroots(C.parent)`.

See also [`Base.dataids`](@ref).
"""
aliasingroots(A::AbstractArray) = (A,)

# For non-array object, we suppose that it has no mutable data. It then ensures
# that `mightalias` returns `false` if one of its argument is not an array.
aliasingroots(::Any) = ()

_isdisjoint(as::Tuple{}, bs::Tuple{}) = true
_isdisjoint(as::Tuple{}, bs::Tuple{Any}) = true
Expand All @@ -1146,10 +1171,8 @@ _isdisjoint(as::Tuple, bs::Tuple) = !(as[1] in bs) && _isdisjoint(tail(as), bs)

Return a tuple of `UInt`s that represent the mutable data segments of an array.

Custom arrays that would like to opt-in to aliasing detection of their component
parts can specialize this method to return the concatenation of the `dataids` of
their component parts. A typical definition for an array that wraps a parent is
`Base.dataids(C::CustomArray) = dataids(C.parent)`.
For custom array types wrapping other array types (e.g., `ReshapedArray`), it is
recommended to define [`Base.aliasingroots`](@ref) instead of `Base.dataids`.
"""
dataids(A::AbstractArray) = (UInt(objectid(A)),)
dataids(A::Array) = (UInt(pointer(A)),)
Expand Down
2 changes: 1 addition & 1 deletion base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ end

# In the common case where we have two views into the same parent, aliasing checks
# are _much_ easier and more important to get right
function mightalias(A::SubArray{T,<:Any,P}, B::SubArray{T,<:Any,P}) where {T,P}
function _mightalias(A::SubArray{T,<:Any,P}, B::SubArray{T,<:Any,P}) where {T,P}
if !_parentsmatch(A.parent, B.parent)
# We cannot do any better than the usual dataids check
return !_isdisjoint(dataids(A), dataids(B))
Expand Down
1 change: 1 addition & 0 deletions base/reinterpretarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ IndexStyle(a::ReinterpretArray) = IndexStyle(a.parent)

parent(a::ReinterpretArray) = a.parent
dataids(a::ReinterpretArray) = dataids(a.parent)
aliasingroots(a::ReinterpretArray) = aliasingroots(a.parent)

function size(a::ReinterpretArray{T,N,S} where {N}) where {T,S}
psize = size(a.parent)
Expand Down
1 change: 1 addition & 0 deletions base/reshapedarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ elsize(::Type{<:ReshapedArray{<:Any,<:Any,P}}) where {P} = elsize(P)

unaliascopy(A::ReshapedArray) = typeof(A)(unaliascopy(A.parent), A.dims, A.mi)
dataids(A::ReshapedArray) = dataids(A.parent)
aliasingroots(a::ReshapedArray) = aliasingroots(a.parent)

@inline ind2sub_rs(ax, ::Tuple{}, i::Int) = (i,)
@inline ind2sub_rs(ax, strds, i) = _ind2sub_rs(ax, strds, i - 1)
Expand Down
2 changes: 2 additions & 0 deletions base/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ julia> parentindices(V)
parentindices(a::AbstractArray) = map(OneTo, size(a))

## Aliasing detection
# Note: Not defining `aliasingroots(::SubArray)` since `SubArray` needs to act
# as a root for the custom `mightalias` check to work.
dataids(A::SubArray) = (dataids(A.parent)..., _splatmap(dataids, A.indices)...)
_splatmap(f, ::Tuple{}) = ()
_splatmap(f, t::Tuple) = (f(t[1])..., _splatmap(f, tail(t))...)
Expand Down
20 changes: 20 additions & 0 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ end
@test @views mightalias(A[:], A[3,1:1])
@test @views mightalias(A[:,:], A[3,1:1])

# view of reshape
B = reshape(A,10,2)
@test mightalias(A, A)
@test mightalias(A, B)
Expand All @@ -1158,6 +1159,25 @@ end
@test @views mightalias(B[1:2], A[1:2])
@test @views !mightalias(B[1:end÷2], A[end÷2+1:end])

# reshape of view
A1 = @view A[:, 1]
A2 = @view A[:, 2]
@test !mightalias(reshape(A1, (:, 1)), A2)
@test mightalias(reshape(A1, (:, 1)), A1)
@test !mightalias(reshape(A1, (:, 1)), reshape(A2, (1, :)))
@test mightalias(reshape(A1, (:, 1)), reshape(A1, (1, :)))

# reinterpret of view
buffer = zeros(UInt8, 4 * sizeof(Int))
x1 = reinterpret(Int, @view buffer[1:end÷2])
x2 = reinterpret(Int, @view buffer[end÷2+1:end])
@test Base.mightalias(x1, x1)
@test !Base.mightalias(x1, x2)
@test @views Base.mightalias(x1[1:1], x1[1:end])
@test @views !Base.mightalias(x1[1:1], x1[2:end])
@test Base.mightalias(reshape(x1, (1, :)), reshape(x1, (:, 1)))
@test !Base.mightalias(reshape(x1, (1, :)), reshape(x2, (:, 1)))

AA = [[1],[2]]
@test @views mightalias(AA, AA[:])
@test @views mightalias(AA[:], AA[:])
Expand Down
11 changes: 11 additions & 0 deletions test/reinterpretarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,14 @@ let a = [0.1 0.2; 0.3 0.4], at = reshape([(i,i+1) for i = 1:2:8], 2, 2)
r = reinterpret(Int, vt)
@test r == OffsetArray(reshape(1:8, 2, 2, 2), (0, offsetvt...))
end

@testset "broadcast" begin
# https://github.com/JuliaLang/julia/issues/29545
buffer = zeros(UInt8, 4 * sizeof(Int))
mid = length(buffer) ÷ 2
x1 = reinterpret(Int, @view buffer[1:mid])
x2 = reinterpret(Int, @view buffer[mid+1:end])
@test !Base.mightalias(x1, x2)
x1 .= x2
@test x1 == x2
end