[PATCH] D102253: [LV] Prevent vectorization with unsupported element types.

[Joe Ellis via Phabricator via llvm-commits]

joechrisellis added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1515-1525
+  bool canVectorizeInstructionTypes(Loop *TheLoop, ElementCount VF) {
+    bool IllegalTy = false;
+    for (BasicBlock *BB : TheLoop->blocks())
+      for (Instruction &I : BB->instructionsWithoutDebug()) {
+        auto *Ty = I.getType();
+        if (!Ty->isVoidTy() &&
+            !TTI.isLegalToVectorizeElementType(Ty, VF.isScalable()))
----------------
I think this is equivalent to:

```
bool canVectorizeInstructionTypes(Loop *TheLoop, ElementCount VF) {
  for (BasicBlock *BB : TheLoop->blocks())
    for (Instruction &I : BB->instructionsWithoutDebug()) {
      auto *Ty = I.getType();
      if (!Ty->isVoidTy() &&
          !TTI.isLegalToVectorizeElementType(Ty, VF.isScalable()))
        return false;
    }
  return true;
}
```

Has the nice side effect of getting rid of the double negative. 🙂 


================
Comment at: llvm/test/Transforms/LoopVectorize/sve-illegal-type.ll:1
+; RUN: opt < %s -loop-vectorize -mtriple aarch64-linux-gnu -force-target-supports-scalable-vectors=true -force-vector-width=4 -pass-remarks-analysis=loop-vectorize -S 2>%t | FileCheck %s
+; RUN: cat %t | FileCheck %s -check-prefix=CHECK-REMARKS
----------------
nit: I usually see the triple being set with:

```
target triple = "aarch64-linux-gnu"
```

to simplify the `RUN` line. IMO in general it makes it a little nicer to run the test as a standalone thing rather than through `llvm-lit`. 🙂 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102253/new/

https://reviews.llvm.org/D102253