[PATCH] D22878: [LSV] Don't assume that bitcast ops are Instructions.

[Alina Sbirlea via llvm-commits]

asbirlea added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:972
@@ -971,2 +971,3 @@
     SmallVector<Instruction *, 16> InstrsToErase;
     SmallVector<Instruction *, 16> InstrsToReorder;
+    // Bitcast might not be an Instruction, if the value being loaded is a
----------------
Looking back at this, why do we need a small vector for instructions to reorder and not just one instruction (which now may or may not exist)?

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:975
@@ +974,3 @@
+    // constant.  In that case, no need to reorder anything.
+    if (Instruction *I = dyn_cast<Instruction>(Bitcast))
+      InstrsToReorder.push_back(I);
----------------
Can you rename I to something else? It's being used right below as an unsigned iterator, and while  the two scopes don't overlap, it can be confusing.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:1003
@@ -999,2 +1002,3 @@
     SmallVector<Instruction *, 16> InstrsToReorder;
-    InstrsToReorder.push_back(cast<Instruction>(Bitcast));
+    if (Instruction *I = dyn_cast<Instruction>(Bitcast))
+      InstrsToReorder.push_back(I);
----------------
Same as above.


https://reviews.llvm.org/D22878