From 6dfea23ad054730202e0a7274529951e2d3e36e2 Mon Sep 17 00:00:00 2001 From: Pablo San-Jose Date: Sun, 28 Jan 2024 21:45:34 +0100 Subject: [PATCH 1/5] ensure elision of `require_one_based_indexing` on high-dim array views clean up has_offset_axes dispatch --- base/abstractarray.jl | 12 +++++++----- test/abstractarray.jl | 4 ++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 7248e9f8038e6..716e343e3cf8d 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -107,13 +107,15 @@ If multiple arguments are passed, equivalent to `has_offset_axes(A) | has_offset See also [`require_one_based_indexing`](@ref). """ -has_offset_axes(A) = _any_tuple(x->Int(first(x))::Int != 1, false, axes(A)...) -has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve performance of a common case (ranges) +has_offset_axes(A) = _has_offset_axes(A) # Use `_any_tuple` to avoid unneeded invoke. # note: this could call `any` directly if the compiler can infer it -has_offset_axes(As...) = _any_tuple(has_offset_axes, false, As...) -has_offset_axes(::Colon) = false -has_offset_axes(::Array) = false +has_offset_axes(As...) = _any_tuple(_has_offset_axes, false, As...) + +_has_offset_axes(A) = _any_tuple(x->Int(first(x))::Int != 1, false, axes(A)...) +_has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve performance of a common case (ranges) +_has_offset_axes(::Colon) = false +_has_offset_axes(::Array) = false """ require_one_based_indexing(A::AbstractArray) diff --git a/test/abstractarray.jl b/test/abstractarray.jl index 21b80c257872f..d32bd5794b5fc 100644 --- a/test/abstractarray.jl +++ b/test/abstractarray.jl @@ -1907,13 +1907,17 @@ end @testset "type-based offset axes check" begin a = randn(ComplexF64, 10) + b = randn(ComplexF64, 4, 4, 4, 4) ta = reinterpret(Float64, a) tb = reinterpret(Float64, view(a, 1:2:10)) tc = reinterpret(Float64, reshape(view(a, 1:3:10), 2, 2, 1)) + td = view(b, :, :, 1, 1) # Issue #44040 @test IRUtils.fully_eliminated(Base.require_one_based_indexing, Base.typesof(ta, tc)) @test IRUtils.fully_eliminated(Base.require_one_based_indexing, Base.typesof(tc, tc)) @test IRUtils.fully_eliminated(Base.require_one_based_indexing, Base.typesof(ta, tc, tb)) + # Issue #49332 + @test IRUtils.fully_eliminated(Base.require_one_based_indexing, Base.typesof(td, td, td)) # Ranges && CartesianIndices @test IRUtils.fully_eliminated(Base.require_one_based_indexing, Base.typesof(1:10, Base.OneTo(10), 1.0:2.0, LinRange(1.0, 2.0, 2), 1:2:10, CartesianIndices((1:2:10, 1:2:10)))) # Remind us to call `any` in `Base.has_offset_axes` once our compiler is ready. From 7172c20f45af3b3e6a13e56f6bb322e797efa0ed Mon Sep 17 00:00:00 2001 From: Pablo San-Jose Date: Sun, 28 Jan 2024 22:36:36 +0100 Subject: [PATCH 2/5] further simplify has_offset_axes dispatch --- base/abstractarray.jl | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 716e343e3cf8d..1ca729ebdeb05 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -107,15 +107,12 @@ If multiple arguments are passed, equivalent to `has_offset_axes(A) | has_offset See also [`require_one_based_indexing`](@ref). """ -has_offset_axes(A) = _has_offset_axes(A) -# Use `_any_tuple` to avoid unneeded invoke. -# note: this could call `any` directly if the compiler can infer it -has_offset_axes(As...) = _any_tuple(_has_offset_axes, false, As...) - -_has_offset_axes(A) = _any_tuple(x->Int(first(x))::Int != 1, false, axes(A)...) -_has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve performance of a common case (ranges) -_has_offset_axes(::Colon) = false -_has_offset_axes(::Array) = false +has_offset_axes(A, As...) = has_offset_axes(A) || has_offset_axes(As...) # note: this could call `any` directly if the compiler can infer it +has_offset_axes(A) = _any_tuple(x->Int(first(x))::Int != 1, false, axes(A)...) +has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve performance of a common case (ranges) +has_offset_axes(::Colon) = false +has_offset_axes(::Array) = false +has_offset_axes() = false """ require_one_based_indexing(A::AbstractArray) From 5240b5ad424cd39c4cd0c5be671627f630445d73 Mon Sep 17 00:00:00 2001 From: Pablo San-Jose Date: Mon, 29 Jan 2024 10:59:06 +0100 Subject: [PATCH 3/5] add comments with details --- base/abstractarray.jl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 1ca729ebdeb05..a617960ff31ef 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -107,12 +107,16 @@ If multiple arguments are passed, equivalent to `has_offset_axes(A) | has_offset See also [`require_one_based_indexing`](@ref). """ -has_offset_axes(A, As...) = has_offset_axes(A) || has_offset_axes(As...) # note: this could call `any` directly if the compiler can infer it +has_offset_axes() = false has_offset_axes(A) = _any_tuple(x->Int(first(x))::Int != 1, false, axes(A)...) has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve performance of a common case (ranges) has_offset_axes(::Colon) = false has_offset_axes(::Array) = false -has_offset_axes() = false +# note: this could call `any` directly if the compiler can infer it. We don't use _any_tuple +# here because it stops full elision in sone cases (#49332) and we don't need handling of +# `missing` (has_offset_axes(A) always returns a Bool) +has_offset_axes(A, As...) = has_offset_axes(A) || has_offset_axes(As...) + """ require_one_based_indexing(A::AbstractArray) From cc21e72972feeefa7fa8207741a45698093f9bb8 Mon Sep 17 00:00:00 2001 From: Pablo San-Jose Date: Mon, 29 Jan 2024 15:59:07 +0100 Subject: [PATCH 4/5] update docstring for has_offset_axes --- base/abstractarray.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index a617960ff31ef..88de5309c2293 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -103,7 +103,7 @@ end has_offset_axes(A, B, ...) Return `true` if the indices of `A` start with something other than 1 along any axis. -If multiple arguments are passed, equivalent to `has_offset_axes(A) | has_offset_axes(B) | ...`. +If multiple arguments are passed, equivalent to `has_offset_axes(A) || has_offset_axes(B) || ...`. See also [`require_one_based_indexing`](@ref). """ From 24c6236d15418b114d3ba3fed52762b411a95a50 Mon Sep 17 00:00:00 2001 From: Pablo San-Jose Date: Mon, 29 Jan 2024 16:41:48 +0100 Subject: [PATCH 5/5] fix typo in comment [skip CI] Co-authored-by: Denis Barucic --- base/abstractarray.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 88de5309c2293..1af8e07584d0d 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -113,7 +113,7 @@ has_offset_axes(A::AbstractVector) = Int(firstindex(A))::Int != 1 # improve perf has_offset_axes(::Colon) = false has_offset_axes(::Array) = false # note: this could call `any` directly if the compiler can infer it. We don't use _any_tuple -# here because it stops full elision in sone cases (#49332) and we don't need handling of +# here because it stops full elision in some cases (#49332) and we don't need handling of # `missing` (has_offset_axes(A) always returns a Bool) has_offset_axes(A, As...) = has_offset_axes(A) || has_offset_axes(As...)