[PATCH] D37880: Fix an out-of-bounds shufflevector index bug

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Sorry for taking so long to get back to this, and thanks again for the comments/reviews!

Since -- as outlined above -- I still don't think the other proposed fix is correct, I plan to commit this as-is. I'll do so in two days, so people have time to voice objections if they have them. (And I'm always happy to revert if we do find a better way to fix this.)



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14472
 
-    NearestPow2 = PowerOf2Ceil(MaxIndex);
+    NearestPow2 = PowerOf2Ceil(MaxIndex + 1);
     if (InVT.isSimple() && (NearestPow2 > 2) &&
----------------
jbhateja wrote:
> If MaxIndex is same as MaxSize of vector it will cause later split (line #14475) to be futile.
I tried adding `InVT.getVectorNumElements() != NearestPow2` into the conditional below, and it seems to substantially change the output of some test cases (e.g. one of the tests in test/CodeGen/X86/avx512-shuffles/partial_permute.ll goes from 4 vector instructions to 18).

Since I don't know this code, and it looks like your comment applies to this code even without this patch, I'd rather not have that change as a part of this.


https://reviews.llvm.org/D37880