[PATCH] D64700: [SLPVectorizer] [NFC] Avoid repetitive calls to getSameOpcode().

Alexey Bataev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 09:39:07 PDT 2019


ABataev added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2976
           // vectorized tree.
-          if (areAllUsersVectorized(cast<Instruction>(V)) &&
-              !ScalarToTreeEntry.count(V)) {
+          auto *I = cast<Instruction>(V);
+          if (areAllUsersVectorized(I) && !ScalarToTreeEntry.count(I)) {
----------------
vporpo wrote:
> This also looks unrelated to this patch.
Not fixed


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1264
+    bool isAltShuffle() const {
+      return getOpcode() != getAltOpcode();;
+    }
----------------
Remove extra `;`


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1283
+
+    void setOperations(InstructionsState *S) {
+      MainOp = S->MainOp;
----------------
const reference?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1305
+
+    bool updateStateIfReorder() {
+      if (!ReorderIndices.empty()) {
----------------
Description?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1356
   TreeEntry *newTreeEntry(ArrayRef<Value *> VL, bool Vectorized,
-                          const EdgeInfo &UserTreeIdx,
+                          InstructionsState *S, const EdgeInfo &UserTreeIdx,
                           ArrayRef<unsigned> ReuseShuffleIndices = None,
----------------
const reference?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3836-3837
       // sink them all the way down past store instructions.
       bool IsReorder = !E->ReorderIndices.empty();
-      if (IsReorder) {
-        S = getSameOpcode(E->Scalars, E->ReorderIndices.front());
-        VL0 = cast<Instruction>(S.OpValue);
-      }
-      setInsertPointAfterBundle(E->Scalars, S);
+      if (E->updateStateIfReorder())
+        VL0 = E->getMainOp();
----------------
Better to keep `IsReorder` in the condition and change its initial value to `E->updateStateIfReorder()`


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

https://reviews.llvm.org/D64700





More information about the llvm-commits mailing list