Don't use negative multiplier strides for comparing loop bounds, fixes #350.

Author: chriselrod

Project.toml

@@ -1,7 +1,7 @@
 name = "LoopVectorization"
 uuid = "bdcacae8-1622-11e9-2a5c-532679323890"
 authors = ["Chris Elrod <[email protected]>"]
-version = "0.12.91"
+version = "0.12.92"
 
 [deps]
 ArrayInterface = "4fba245c-0d91-5ea0-9b3e-6abc04ee57a9"
@@ -38,5 +38,5 @@ SLEEFPirates = "0.6.23"
 Static = "0.3.3, 0.4"
 ThreadingUtilities = "0.4.5"
 UnPack = "1"
-VectorizationBase = "0.21.19"
+VectorizationBase = "0.21.20"
 julia = "1.5"

src/codegen/loopstartstopmanager.jl

@@ -61,11 +61,14 @@ function uniquearrayrefs(ls::LoopSet)
   uniquerefs, includeinlet
 end
 
-otherindexunrolled(loopsym::Symbol, ind::Symbol, loopdeps::Vector{Symbol}) = (loopsym !== ind) && (loopsym ∈ loopdeps)
+otherindexunrolled(loopsym::Symbol, ind::Symbol, loopdeps::Vector{Symbol}) = ((loopsym ≢ ind) & (loopsym ≢ Symbol("##undefined##"))) && (loopsym ∈ loopdeps)
 function otherindexunrolled(ls::LoopSet, ind::Symbol, ref::ArrayReferenceMeta)
   us = ls.unrollspecification
-  u₁sym = names(ls)[us.u₁loopnum]
-  u₂sym = us.u₂loopnum > 0 ? names(ls)[us.u₂loopnum] : Symbol("##undefined##")
+  @unpack u₁loopnum, u₂loopnum, u₁, u₂ = us
+  u₁sym = u₁ > 1 ? names(ls)[u₁loopnum] : Symbol("##undefined##")
+  u₂sym = ((u₂ > 1) & (u₂loopnum > 0)) ? names(ls)[u₂loopnum] : Symbol("##undefined##")
+  # u₁sym = names(ls)[u₁loopnum]
+  # u₂sym = ((u₂loopnum > 0)) ? names(ls)[u₂loopnum] : Symbol("##undefined##")
   otherindexunrolled(u₁sym, ind, loopdependencies(ref)) || otherindexunrolled(u₂sym, ind, loopdependencies(ref))
 end
 function multiple_with_name(n::Symbol, v::Vector{ArrayReferenceMeta})
@@ -86,19 +89,21 @@ function indices_calculated_by_pointer_offsets(ls::LoopSet, ar::ArrayReferenceMe
   offset = isdiscontiguous(ar)
   gespinds = Expr(:tuple)
   out = Vector{Bool}(undef, length(indices))
+  strds = getstrides(ar)
   li = ar.loopedindex
   for i ∈ eachindex(li)
     ii = i + offset
     ind = indices[ii]
     if (!li[i]) || (ind === CONSTANTZEROINDEX) || multiple_with_name(vptr(ar), ls.lssm.uniquearrayrefs) ||
-      (iszero(ls.vector_width) && isstaticloop(getloop(ls, ind)))# ||
+      (iszero(ls.vector_width) && isstaticloop(getloop(ls, ind))) || (strds[i] ≤ 0)
       out[i] = false
     elseif (isone(ii) && (first(looporder) === ind))
       out[i] = otherindexunrolled(ls, ind, ar)
     else
       out[i] = true
     end
   end
+  # @show ar out
   out
 end
 
@@ -677,6 +682,7 @@ function use_loop_induct_var!(
   looporder = reversenames(ls)
   uliv = Vector{Int}(undef, length(li))
   indices = getindices(ar)
+  strds = getstrides(ar)
   offset = first(indices) === DISCONTIGUOUS
   if length(indices) != offset + length(li)
     println(ar)
@@ -711,7 +717,7 @@ function use_loop_induct_var!(
     elseif isbroadcast ||
       ((isone(ii) && (last(looporder) === ind)) && !(otherindexunrolled(ls, ind, ar)) ||
       multiple_with_name(vptrar, allarrayrefs)) ||
-      (iszero(ls.vector_width) && isstaticloop(getloop(ls, ind)))# ||
+      (iszero(ls.vector_width) && isstaticloop(getloop(ls, ind))) || (strds[i] ≤ 0)
       # Not doing normal offset indexing
       uliv[i] = -findfirst(Base.Fix2(===,ind), looporder)::Int
       push!(offsetprecalc_descript.args, 0) # not doing offset indexing, so push 0

src/codegen/lowering.jl

@@ -614,10 +614,12 @@ end
 ## This performs reduction to one `Vec`
 function reduce_expr!(q::Expr, ls::LoopSet, U::Int)
   us = ls.unrollspecification
-  u₁f, u₂f = if us.u₂ == -1
-    ifelse(U == -1, us.u₁, U), -1
+  if us.u₂ == -1
+    u₁f = ifelse(U == -1, us.u₁, U)
+    u₂f = -1
   else
-    us.u₁, U
+    u₁f = us.u₁
+    u₂f = U
   end
   # u₁loop, u₂loop = getunrolled(ls)
   u₁loop = getloop(ls, us.u₁loopnum).itersymbol

src/codegen/split_loops.jl

@@ -109,6 +109,8 @@ function lower_and_split_loops(ls::LoopSet, inline::Int)
   order_fused, unrolled_fused, tiled_fused, vectorized_fused, U_fused, T_fused, cost_fused, shouldinline_fused = choose_order_cost(ls)
   remaining_ops = Vector{Int}(undef, length(split_candidates) - 1); split_1 = Int[0];
   # for (ind,i) ∈ enumerate(split_candidates)
+  looplenpen = 0.05
+  ls_looplen = looplengthprod(ls)*looplenpen
   for (ind,i) ∈ enumerate(split_candidates)
     split_1[1] = i
     ls_1 = split_loopset(ls, split_1, false)
@@ -118,7 +120,7 @@ function lower_and_split_loops(ls::LoopSet, inline::Int)
     order_2, unrolled_2, tiled_2, vectorized_2, U_2, T_2, cost_2, shouldinline_2 = choose_order_cost(ls_2)
     # U_1 = T_1 = U_2 = T_2 = 2
     # return ls_1, ls_2
-    if cost_1 + cost_2 ≤ 0.9cost_fused
+    if cost_1 + cost_2 + looplenpen*(looplengthprod(ls_1) + looplengthprod(ls_2)) ≤ muladd(0.9, cost_fused, ls_looplen)
       ls_2_lowered = if length(remaining_ops) > 1
         inline = iszero(inline) ? (shouldinline_1 % Int) : inline
         lower_and_split_loops(ls_2, inline)

src/precompile.jl

@@ -263,4 +263,6 @@ function _precompile_()
     Base.precompile(Tuple{typeof(choose_order),LoopSet})   # time: 0.001016861
     Base.precompile(Tuple{typeof(capture_a_muladd),Expr,Nothing})   # time: 0.001010088
     Base.precompile(Tuple{typeof(canonicalize_range),CartesianIndices{4, NTuple{4, Base.OneTo{Int}}}})   # time: 0.001000169
+
+  Base.precompile(reduce_expr!, (Expr, LoopSet, Int))
 end

test/shuffleloadstores.jl

@@ -380,7 +380,26 @@ function issue348_v1!(hi, lo)
     end
   end
 end
-
+function reverse_part(n1,n2)
+  A = zeros(n1,n2)
+  @turbo for i=1:n1÷2, j = 1:n2
+    c = 1.0
+    A[i, j] = c
+    r = n1 + 1 - i
+    A[r, j] = c
+  end
+  return A
+end
+function reverse_part_ref(n1,n2)
+  A = zeros(n1,n2)
+  @inbounds for i=1:n1÷2; @simd for j = 1:n2
+    c = 1.0
+    A[i, j] = c
+    r = n1 + 1 - i
+    A[r, j] = c
+  end; end
+  return A
+end
 
 @testset "shuffles load/stores" begin
   @show @__LINE__
@@ -474,5 +493,7 @@ end
     @test a_hi_tmp_ref == a_hi_tmp1
     @turbo a_hi_tmp1 .= 0;
     @test all(iszero, parent(a_hi_tmp1))
+
+    @test reverse_part(n_hi,4) == reverse_part_ref(n_hi,4)
   end
 end