slightly speed up cartesian indexing (#51606)

This is a minor simplification to the multidimensional iteration code
that behaves slightly better in the presence of possible overflow (at
least according to LLVM. This was found when looking into performance
regressions caused by #51319
since that makes `Array` rely more on the regular multidimensional code
rather than being special cased.

This PR does remove/alter a few tests, but they are all tests of
iteration when given an invalid iterator state which we specifically
document as a thing that does not work.

Co-authored-by: Jameson Nash <[email protected]>

base/multidimensional.jl

@@ -424,29 +424,19 @@ module IteratorsMD
  @inline function __inc(state::Tuple{Int}, indices::Tuple{OrdinalRangeInt})
  rng = indices[1]
  I = state[1] + step(rng)
- valid = __is_valid_range(I, rng) && state[1] != last(rng)
- return valid, (I, )
+ valid = state[1] != last(rng)
+ return valid, (I,)
  end
  @inline function __inc(state::Tuple{Int,Int,Vararg{Int}}, indices::Tuple{OrdinalRangeInt,OrdinalRangeInt,Vararg{OrdinalRangeInt}})
  rng = indices[1]
  I = state[1] + step(rng)
- if __is_valid_range(I, rng) && state[1] != last(rng)
+ if state[1] != last(rng)
  return true, (I, tail(state)...)
  end
  valid, I = __inc(tail(state), tail(indices))
  return valid, (first(rng), I...)
  end
 
- @inline __is_valid_range(I, rng::AbstractUnitRange) = I in rng
- @inline function __is_valid_range(I, rng::OrdinalRange)
- if step(rng) > 0
- lo, hi = first(rng), last(rng)
- else
- lo, hi = last(rng), first(rng)
- end
- lo <= I <= hi
- end
-
  # 0-d cartesian ranges are special-cased to iterate once and only once
  iterate(iter::CartesianIndices{0}, done=false) = done ? nothing : (CartesianIndex(), true)
 
@@ -556,13 +546,13 @@ module IteratorsMD
  @inline function __dec(state::Tuple{Int}, indices::Tuple{OrdinalRangeInt})
  rng = indices[1]
  I = state[1] - step(rng)
- valid = __is_valid_range(I, rng) && state[1] != first(rng)
+ valid = state[1] != first(rng)
  return valid, (I,)
  end
  @inline function __dec(state::Tuple{Int,Int,Vararg{Int}}, indices::Tuple{OrdinalRangeInt,OrdinalRangeInt,Vararg{OrdinalRangeInt}})
  rng = indices[1]
  I = state[1] - step(rng)
- if __is_valid_range(I, rng) && state[1] != first(rng)
+ if state[1] != first(rng)
  return true, (I, tail(state)...)
  end
  valid, I = __dec(tail(state), tail(indices))

test/cartesian.jl

@@ -296,8 +296,7 @@ end
  R = CartesianIndex(1, 1):CartesianIndex(2, 3):CartesianIndex(4, 5)
  @test R.indices == (1:2:3, 1:3:4)
  i = CartesianIndex(4, 1)
- i_next = CartesianIndex(1, 4)
- @test !(i in R) && iterate(R, i) == (i_next, i_next)
+ @test !(i in R)
 
  for R in [
  CartesianIndices((1:-1:-1, 1:2:5)),
@@ -393,37 +392,38 @@ end
 
 @testset "CartesianIndices overflow" begin
  @testset "incremental steps" begin
+ # n.b. typemax is an odd number
  I = CartesianIndices((1:typemax(Int),))
  i = last(I)
  @test iterate(I, i) === nothing
 
  I = CartesianIndices((1:2:typemax(Int), ))
- i = CartesianIndex(typemax(Int)-1)
+ i = CartesianIndex(typemax(Int))
  @test iterate(I, i) === nothing
 
  I = CartesianIndices((1:(typemax(Int)-1),))
- i = CartesianIndex(typemax(Int))
+ i = CartesianIndex(typemax(Int)-1)
  @test iterate(I, i) === nothing
 
- I = CartesianIndices((1:2:typemax(Int)-1, ))
+ I = CartesianIndices((2:2:typemax(Int)-1, ))
  i = CartesianIndex(typemax(Int)-1)
  @test iterate(I, i) === nothing
 
  I = CartesianIndices((1:typemax(Int), 1:typemax(Int)))
  i = last(I)
  @test iterate(I, i) === nothing
 
- I = CartesianIndices((1:2:typemax(Int), 1:2:typemax(Int)))
+ I = CartesianIndices((2:2:typemax(Int), 2:2:typemax(Int)))
  i = CartesianIndex(typemax(Int)-1, typemax(Int)-1)
  @test iterate(I, i) === nothing
 
  I = CartesianIndices((1:typemax(Int), 1:typemax(Int)))
  i = CartesianIndex(typemax(Int), 1)
  @test iterate(I, i) === (CartesianIndex(1, 2), CartesianIndex(1,2))
 
- I = CartesianIndices((1:2:typemax(Int), 1:2:typemax(Int)))
+ I = CartesianIndices((2:2:typemax(Int), 2:2:typemax(Int)))
  i = CartesianIndex(typemax(Int)-1, 1)
- @test iterate(I, i) === (CartesianIndex(1, 3), CartesianIndex(1, 3))
+ @test iterate(I, i) === (CartesianIndex(2, 3), CartesianIndex(2, 3))
 
  I = CartesianIndices((typemin(Int):(typemin(Int)+3),))
  i = last(I)
@@ -493,15 +493,6 @@ end
  end
  @test length(I) == length(indices)
  @test vec(collect(I)) == indices
-
- # test invalid state
- I = CartesianIndices((2:4, 3:5))
- @test iterate(I, CartesianIndex(typemax(Int), 3))[1] == CartesianIndex(2,4)
- @test iterate(I, CartesianIndex(typemax(Int), 4))[1] == CartesianIndex(2,5)
- @test iterate(I, CartesianIndex(typemax(Int), 5)) === nothing
-
- @test iterate(I, CartesianIndex(3, typemax(Int)))[1] == CartesianIndex(4,typemax(Int))
- @test iterate(I, CartesianIndex(4, typemax(Int))) === nothing
 end
 
 @testset "CartesianIndices operations" begin