[PATCH] D22878: [LSV] Don't assume that bitcast ops are Instructions.
Alina Sbirlea via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 27 11:01:36 PDT 2016
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
More information about the llvm-commits
mailing list