[PATCH] D37648: [SLPVectorizer] Fix PR21780 Expansion of 256 bit vector loads fails to fold into shuffles

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Some more style comment as with https://reviews.llvm.org/D37579



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4220
+        CI->dropAllReferences();
+     }
+  }
----------------
clang-format - braces


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:5573
+  } else
+    return nullptr;
+
----------------
It'd be much clearer to early out.
```
if (!C || C->getZExtValue() > MaxOffset)
  return nullptr;

// We found the first InsertElem in this chain.
uint64_t Offset = C->getZExtValue();
...
```


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:5626
+
+  for (unsigned i = 0; i < MaxOffset + 1; i++)
+    if (Inserts[i] && !Inserts[i]->use_empty()) {
----------------
```
for (unsigned i = 0, e = MaxOffset + 1; i < e; i++)
```


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:5637
+
+  if ((!Use) || Use->getParent() != BB)
+    return nullptr;
----------------
```
if (!Use || Use->getParent() != BB)
```


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:5661
+  }
+  for (unsigned i = 1; i < MaxOffset + 1; i++) {
+    // Building GEP, Load, Insert for
----------------
```
for (unsigned i = 0, e = MaxOffset + 1; i < e; i++)
```


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:5671
+        GetElementPtrInst *GEP = GEPs[i - 1];
+        Builder.SetInsertPoint(GEP->getNextNode());
+      }
----------------
You're accessing GEPs[i - 1] multiple times - pull out?


https://reviews.llvm.org/D37648