[SVE] Don't consider scalable vector types in SLPVectorizerPass::vectorizeChainsInBlock

[david-arm]

Summary

In vectorizeChainsInBlock we try to collect chains of PHI nodes
that have the same element type, but the code is relying upon
the implicit conversion from TypeSize -> uint64_t. For now, I have
modified the code to ignore PHI nodes with scalable types.

[sdesmalen]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7405

Should the check for scalable (+ corresponding code to bail out) be hoisted out of the if (EltTy->isSized()) condition/block?

[david-arm]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7405

I'm not sure really. This change fixes the case I was interested, but not sure what it means if a type is not sized *and* scalable? I assumed that if a type wasn't sized then you couldn't call getTypeSizeInBits and hope to get a sensible answer. We only know if the type is scalable or not by asking for the TypeSize.

[david-arm]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7405

Any further thoughts on this @sdesmalen ? If it's not sized then I can't ask the question if it's scalable or not.

[efriedma]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7405

The isSized() check here is nonsense: an instruction can't have an unsized result or operand type.

[david-arm]

Hi @efriedma I tried removing the isSized() check and added an assert instead. I tested it and none of our tests broke! Or would you prefer me to just remove the assert completely?

[efriedma]

LGTM

The assertion is probably a little excessive, but someone thought it was worth checking for at some point, so it's okay.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

while (HaveVectorizedPhiNodes) {
     // Try to vectorize elements base on their type.
     for (SmallVector<Value *, 4>::iterator IncIt = Incoming.begin(),
                                            E = Incoming.end();
          IncIt != E;) {
 
       // Look for the next elements with the same type.
       SmallVector<Value *, 4>::iterator SameTypeIt = IncIt;
       Type *EltTy = (*IncIt)->getType();
-      unsigned EltSize = EltTy->isSized() ? DL->getTypeSizeInBits(EltTy)
-                                          : MaxVecRegSize;
+
+      assert(EltTy->isSized() &&
+             "Instructions should all be sized at this point");
+      TypeSize EltTS = DL->getTypeSizeInBits(EltTy);
+      if (EltTS.isScalable()) {
+        // For now, just ignore vectorizing scalable types.
+        ++IncIt;
+        continue;
+      }
+
+      unsigned EltSize = EltTS.getFixedSize();
       unsigned MaxNumElts = MaxVecRegSize / EltSize;
       if (MaxNumElts < 2) {
         ++IncIt;
         continue;
       }
 
       while (SameTypeIt != E &&
              (*SameTypeIt)->getType() == EltTy &&

llvm/test/Transforms/SLPVectorizer/AArch64/scalable-vector.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -slp-vectorizer -S | FileCheck %s
+; RUN: opt < %s -slp-vectorizer -S 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-unknown-linux-gnu"
 
 ; This test check that we are not crashing or changing the code.
 
 define void @test() {
 ; CHECK-LABEL: @test(
 ; CHECK-NEXT:    [[LOAD0:%.*]] = tail call <vscale x 16 x i8> @llvm.masked.load.nxv16i8.p0nxv16i8(<vscale x 16 x i8>* undef, i32 1, <vscale x 16 x i1> undef, <vscale x 16 x i8> undef)
 ; CHECK-NEXT:    [[LOAD1:%.*]] = tail call <vscale x 16 x i8> @llvm.masked.load.nxv16i8.p0nxv16i8(<vscale x 16 x i8>* undef, i32 1, <vscale x 16 x i1> undef, <vscale x 16 x i8> undef)
 ; CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[LOAD1]], [[LOAD0]]
 ; CHECK-NEXT:    tail call void @llvm.masked.store.nxv16i8.p0nxv16i8(<vscale x 16 x i8> [[ADD]], <vscale x 16 x i8>* undef, i32 1, <vscale x 16 x i1> undef)
 ; CHECK-NEXT:    ret void
 ;
   %load0 = tail call <vscale x 16 x i8> @llvm.masked.load.nxv16i8.p0nxv16i8(<vscale x 16 x i8> *undef, i32 1, <vscale x 16 x i1> undef, <vscale x 16 x i8> undef)
   %load1 = tail call <vscale x 16 x i8> @llvm.masked.load.nxv16i8.p0nxv16i8(<vscale x 16 x i8> *undef, i32 1, <vscale x 16 x i1> undef, <vscale x 16 x i8> undef)
   %add = add <vscale x 16 x i8> %load1, %load0
   tail call void @llvm.masked.store.nxv16i8.p0nxv16i8(<vscale x 16 x i8> %add, <vscale x 16 x i8>* undef, i32 1, <vscale x 16 x i1> undef)
   ret void
 }
 
+define <vscale x 4 x i32> @scalable_phi(<vscale x 4 x i32> %a, i32 %b) {
+; CHECK-LABEL: @scalable_phi(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[B:%.*]], 0
+; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[END:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[END]]
+; CHECK:       end:
+; CHECK-NEXT:    [[RETVAL:%.*]] = phi <vscale x 4 x i32> [ [[A:%.*]], [[ENTRY:%.*]] ], [ zeroinitializer, [[IF_THEN]] ]
+; CHECK-NEXT:    ret <vscale x 4 x i32> [[RETVAL]]
+;
+entry:
+  %cmp = icmp eq i32 %b, 0
+  br i1 %cmp, label %if.then, label %end
+
+if.then:
+  br label %end
+
+end:
+  %retval = phi <vscale x 4 x i32> [ %a, %entry ], [ zeroinitializer, %if.then ]
+  ret <vscale x 4 x i32> %retval
+}
+
 declare <vscale x 16 x i8> @llvm.masked.load.nxv16i8.p0nxv16i8(<vscale x 16 x i8>*, i32 immarg, <vscale x 16 x i1>, <vscale x 16 x i8>)
 declare void @llvm.masked.store.nxv16i8.p0nxv16i8(<vscale x 16 x i8>, <vscale x 16 x i8>*, i32 immarg, <vscale x 16 x i1>)