[PATCH] D108136: [LoopVectorize] Permit vectorisation of more select(cmp(), X, Y) reduction patterns

[Kerry McLaughlin via Phabricator via llvm-commits]

kmclaughlin added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:896
+    StartVal = Builder.CreateVectorSplat(
+        cast<VectorType>(Left->getType())->getElementCount(), StartVal);
+  }
----------------
Could you use dyn_cast instead of isa & cast here?


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1012
+  assert(RecurrenceDescriptor::isSelectCmpRecurrenceKind(
+      Desc.getRecurrenceKind()));
+  Value *InitVal = Desc.getRecurrenceStartValue();
----------------
Can you add a message to this assert, maybe something like "Unexpected reduction kind"?


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1100
+    return createSimpleTargetReduction(B, TTI, Src, RK);
+  }
 }
----------------
Is it possible to use isSelectCmpRecurrenceKind here? e.g.

```
if (isSelectCmpRecurrenceKind(Desc.getRecurrenceKind())
  return createSelectCmpTargetReduction(B, TTI, Src, Desc, OrigPhi);

return createSimpleTargetReduction(B, TTI, Src, RK);
```


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/sve-select-cmp.ll:18
+
+; CHECK-VF4IC4:      vector.body:
+; CHECK-VF4IC4:        [[VEC_PHI1:%.*]] = phi <vscale x 4 x i32> [ shufflevector (<vscale x 4 x i32> insertelement (<vscale x 4 x i32> poison, i32 3, i32 0), <vscale x 4 x i32> poison, <vscale x 4 x i32> zeroinitializer), %vector.ph ], [ [[VEC_SEL1:%.*]], %vector.body ]
----------------
I could be wrong, but I think it's necessary to check for the correct label before the CHECK lines for each prefix? (e.g. `CHECK-VF4IC4-LABEL` above here)


================
Comment at: llvm/test/Transforms/LoopVectorize/select-cmp.ll:214
+
+; Negative tests
+
----------------
Could you please add a small comment above each of the negative tests to explain why they shouldn't vectorise?


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

https://reviews.llvm.org/D108136