[PATCH] D63661: [SLPVectorizer] Operand reordering across multiple instructions.

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1608
+    bool hasCompatibleOpcodesWithSuperNode(ArrayRef<Value *> VL) const {
+      return std::all_of(VL.begin(), VL.end(), [this](Value *V) {
+        return isa<Instruction>(V) &&
----------------
llvm::all_of


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1622
+    /// \returns true if the binary operator \p BO allows reassociation.
+    static bool canReassociate(BinaryOperator *BO) {
+      // If fmath, then check the fast-math flags.
----------------
This looks quite generic so maybe it can be reused. Reassoc pass should have similar code too?

@spatel


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3223
 
-      // Reorder operands if reordering would enable vectorization.
-      if (isa<BinaryOperator>(VL0)) {
-        ValueList Left, Right;
-        reorderInputsAccordingToOpcode(VL, Left, Right, *DL, *SE, *this);
-        TE->setOperand(0, Left);
-        TE->setOperand(1, Right);
-        buildTree_rec(Left, Depth + 1, {TE, 0});
-        buildTree_rec(Right, Depth + 1, {TE, 1});
-        return;
-      }
+      if (EnableSuperNode &&
+          // We are either in progress, or we can create a new one.
----------------
This is same code as the code added just above ? Please move it to own function.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63661