[PATCH] D115268: [SLP]Fix comparator for cmp instruction vectorization.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:9574
+      // Compare operands.
+      for (int I = 0, E = CI1->getNumOperands(); I < E; ++I) {
+        auto *Op0 = CI1->getOperand(I);
----------------
vporpo wrote:
> ABataev wrote:
> > vporpo wrote:
> > > Perhaps it is worth moving the operand comparison code in a separate function as it shares a lot with the operand comparison code in `CompareSorter()` above. It could return an enum like LT, GT, EQ (corresponding to return true, return false and continue).
> > No, the logic is different
> My main concern is that if we make changes in the logic of `CompareSorter()` we should also make changes here too, and if we don't I think the compare instructions will be silently skipped. Perhaps it is worth mentioning this in a comment?
That’s inevitable here. We have different logic for sorting comparator and for compatible instructions. They are similar but different, have different number of instructions. I will try but no promises. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115268