[SLP] Honor min/max regsize and min/max VF in vectorizeStores

[bjope]

Summary

Make sure we use PowerOf2Floor instead of PowerOf2Ceil when
calculating max number of elements that fits inside a vector
register (otherwise we could end up creating vectors larger
than the maximum vector register size).

Also make sure we honor the min/max VF (as given by TTI or
cmd line parameters) when doing vectorizeStores.

[anton-afanasyev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

6076

Good fix, how did we missed this? Looks like ceil and floor are equal in real life here.

6081

Since llvm::PowerOf2Floor() could return 0 for zero argument, I believe this check could be actually removed.

[anton-afanasyev]

LGTM except for the comment. I think MaxVecRegSize % EltSize != 0 check could be removed (together with comment).

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

 //===- SLPVectorizer.cpp - A bottom up SLP Vectorizer ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
for (int Cnt = E; Cnt > 0; --Cnt) {
     BoUpSLP::ValueList Operands;
     // Collect the chain into a list.
     while (I != E + 1 && !VectorizedStores.count(Stores[I])) {
       Operands.push_back(Stores[I]);
       // Move to the next value in the chain.
       I = ConsecutiveChain[I];
     }
 
-    // If a vector register can't hold 1 element, we are done.
     unsigned MaxVecRegSize = R.getMaxVecRegSize();
     unsigned EltSize = R.getVectorElementSize(Operands[0]);
+    unsigned MaxElts = llvm::PowerOf2Floor(MaxVecRegSize / EltSize);
+
+    // For legacy reasons we check that MaxVecRegSize is evenly divisible by the
+    // element size. This check used to be described as checking that at least
+    // one element fits in a vector register, but that did not match the
+    // code. Maybe this check can be removed?
     if (MaxVecRegSize % EltSize != 0)
       continue;
 
-    unsigned MaxElts = MaxVecRegSize / EltSize;
+    unsigned MinVF = std::max(2U, R.getMinVecRegSize() / EltSize);
+    unsigned MaxVF = std::min(R.getMaximumVF(EltSize, Instruction::Store),
+                              MaxElts);
+
     // FIXME: Is division-by-2 the correct step? Should we assert that the
     // register size is a power-of-2?
     unsigned StartIdx = 0;
-    for (unsigned Size = llvm::PowerOf2Ceil(MaxElts); Size >= 2; Size /= 2) {
+    for (unsigned Size = MaxVF; Size >= MinVF; Size /= 2) {
       for (unsigned Cnt = StartIdx, E = Operands.size(); Cnt + Size <= E;) {
         ArrayRef<Value *> Slice = makeArrayRef(Operands).slice(Cnt, Size);
         if (!VectorizedStores.count(Slice.front()) &&
             !VectorizedStores.count(Slice.back()) &&
             vectorizeStoreChain(Slice, R, Cnt)) {
           // Mark the vectorized stores so that we don't vectorize them again.
           VectorizedStores.insert(Slice.begin(), Slice.end());
           Changed = true;

llvm/test/Transforms/SLPVectorizer/slp-max-reg-size.ll

 
 ; This test case is checking if max register size (and max VF) is honored when
 ; vecorizing store chains in SLPVectorizer.
 
 @X = external global [8 x i32], align 1
 
 define void @foo() {
 ; CHECK-VF8-160-LABEL: @foo(
-; CHECK-VF8-160-NEXT:    store <8 x i32> <i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8>, <8 x i32>* bitcast ([8 x i32]* @X to <8 x i32>*), align 1
+; CHECK-VF8-160-NEXT:    store <4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32>* bitcast ([8 x i32]* @X to <4 x i32>*), align 1
+; CHECK-VF8-160-NEXT:    store <4 x i32> <i32 5, i32 6, i32 7, i32 8>, <4 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <4 x i32>*), align 1
 ; CHECK-VF8-160-NEXT:    ret void
 ;
 ; CHECK-VF4-160-LABEL: @foo(
-; CHECK-VF4-160-NEXT:    store <8 x i32> <i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8>, <8 x i32>* bitcast ([8 x i32]* @X to <8 x i32>*), align 1
+; CHECK-VF4-160-NEXT:    store <4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32>* bitcast ([8 x i32]* @X to <4 x i32>*), align 1
+; CHECK-VF4-160-NEXT:    store <4 x i32> <i32 5, i32 6, i32 7, i32 8>, <4 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <4 x i32>*), align 1
 ; CHECK-VF4-160-NEXT:    ret void
 ;
 ; CHECK-VF2-160-LABEL: @foo(
-; CHECK-VF2-160-NEXT:    store <8 x i32> <i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8>, <8 x i32>* bitcast ([8 x i32]* @X to <8 x i32>*), align 1
+; CHECK-VF2-160-NEXT:    store <2 x i32> <i32 1, i32 2>, <2 x i32>* bitcast ([8 x i32]* @X to <2 x i32>*), align 1
+; CHECK-VF2-160-NEXT:    store <2 x i32> <i32 3, i32 4>, <2 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 2) to <2 x i32>*), align 1
+; CHECK-VF2-160-NEXT:    store <2 x i32> <i32 5, i32 6>, <2 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <2 x i32>*), align 1
+; CHECK-VF2-160-NEXT:    store <2 x i32> <i32 7, i32 8>, <2 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 6) to <2 x i32>*), align 1
 ; CHECK-VF2-160-NEXT:    ret void
 ;
 ; CHECK-VF8-128-LABEL: @foo(
 ; CHECK-VF8-128-NEXT:    store <4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32>* bitcast ([8 x i32]* @X to <4 x i32>*), align 1
 ; CHECK-VF8-128-NEXT:    store <4 x i32> <i32 5, i32 6, i32 7, i32 8>, <4 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <4 x i32>*), align 1
 ; CHECK-VF8-128-NEXT:    ret void
 ;
 ; CHECK-VF4-128-LABEL: @foo(
 ; CHECK-VF4-128-NEXT:    store <4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32>* bitcast ([8 x i32]* @X to <4 x i32>*), align 1
 ; CHECK-VF4-128-NEXT:    store <4 x i32> <i32 5, i32 6, i32 7, i32 8>, <4 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <4 x i32>*), align 1
 ; CHECK-VF4-128-NEXT:    ret void
 ;
 ; CHECK-VF2-128-LABEL: @foo(
-; CHECK-VF2-128-NEXT:    store <4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32>* bitcast ([8 x i32]* @X to <4 x i32>*), align 1
-; CHECK-VF2-128-NEXT:    store <4 x i32> <i32 5, i32 6, i32 7, i32 8>, <4 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <4 x i32>*), align 1
+; CHECK-VF2-128-NEXT:    store <2 x i32> <i32 1, i32 2>, <2 x i32>* bitcast ([8 x i32]* @X to <2 x i32>*), align 1
+; CHECK-VF2-128-NEXT:    store <2 x i32> <i32 3, i32 4>, <2 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 2) to <2 x i32>*), align 1
+; CHECK-VF2-128-NEXT:    store <2 x i32> <i32 5, i32 6>, <2 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <2 x i32>*), align 1
+; CHECK-VF2-128-NEXT:    store <2 x i32> <i32 7, i32 8>, <2 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 6) to <2 x i32>*), align 1
 ; CHECK-VF2-128-NEXT:    ret void
 ;
 ; CHECK-VF8-256-LABEL: @foo(
 ; CHECK-VF8-256-NEXT:    store <8 x i32> <i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8>, <8 x i32>* bitcast ([8 x i32]* @X to <8 x i32>*), align 1
 ; CHECK-VF8-256-NEXT:    ret void
 ;
 ; CHECK-VF2-128-128-LABEL: @foo(
-; CHECK-VF2-128-128-NEXT:    store <4 x i32> <i32 1, i32 2, i32 3, i32 4>, <4 x i32>* bitcast ([8 x i32]* @X to <4 x i32>*), align 1
-; CHECK-VF2-128-128-NEXT:    store <4 x i32> <i32 5, i32 6, i32 7, i32 8>, <4 x i32>* bitcast (i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4) to <4 x i32>*), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 1, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 0), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 2, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 1), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 3, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 2), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 4, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 3), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 5, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 6, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 5), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 7, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 6), align 1
+; CHECK-VF2-128-128-NEXT:    store i32 8, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 7), align 1
 ; CHECK-VF2-128-128-NEXT:    ret void
 ;
   store i32 1, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 0), align 1
   store i32 2, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 1), align 1
   store i32 3, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 2), align 1
   store i32 4, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 3), align 1
   store i32 5, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 4), align 1
   store i32 6, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 5), align 1
   store i32 7, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 6), align 1
   store i32 8, i32* getelementptr inbounds ([8 x i32], [8 x i32]* @X, i16 0, i16 7), align 1
   ret void
 }