[PATCH] D59973: [SLP] Refactoring of the operand reordering code.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a subscriber: spatel.
RKSimon added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:640
+    Constant, // Matching constants
+    Splat,    // Matching the same instruction multiple times (broadcast)
+    Failed,   // We failed to create a vectorizable group
----------------
vporpo wrote:
> RKSimon wrote:
> > Matching for extractelement is also an issue: https://bugs.llvm.org/show_bug.cgi?id=41304
> Good point, but let's implement it in a separate patch.
@spatel has done this in instcombine which should be good enough.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:683
+  LLVM_DUMP_METHOD static void
+  dumpModeVec(const SmallVectorImpl<ReorderingMode> &ReorderingModes) {
+    for (unsigned OpIdx = 0, E = ReorderingModes.size(); OpIdx != E; ++OpIdx)
----------------
This isn't used yet - can it wait until you need it? Also - should you use ArrayRef?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:749
+      // the inverse operations by checking commutativity.
+      return !isCommutative(I);
+    }
----------------
If this is now the only use of isCommutative then maybe merge them? Not sure if it needs to be inside VLOperands


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

https://reviews.llvm.org/D59973