[PATCH] D21934: Address two correctness issues in LoadStoreVectorizer

[Justin Lebar via llvm-commits]

jlebar added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:422
@@ +421,3 @@
+  assert(Chain.size() == ChainInstrs.size() &&
+         "LSV: Error occured while checking if a chain is vectorizable");
+
----------------
jlebar wrote:
> arsenm wrote:
> > This message isn't descriptive of what is wrong and doesn't need the LSV
> Nit, no need for "LSV:" -- the assertion error will point to this line of code.  And, can we make this message a bit more specific?  Like, clearly if an assert fails, there's an error.  But what is the condition we're checking here, exactly?  Maybe "Range [From, To) must contain all instructions in Chain"?
Need to wrap to 80 chars.

But also this message can be tightened up, I think.  No need to say "Error checking if chain is vectorizable:" -- we can tell from the stacktrace printed when the assert fails.

And as written this doesn't make sense to me:

> All instructions in Chain should have been to ChainInsts from [From, To).

Maybe "to" is the wrong preposition?  Or do you mean "should have been added to"?  I still like my original suggestion, tbh.  :)


http://reviews.llvm.org/D21934