[llvm-commits] Patch: generalize vector shuffle

[Evan Cheng]

Hi Mon Ping,

Thanks for doing this. It's an important improvement.

However, this patch is hard (at least for me) to digest. It's just a  
lot of code. :-) Some of the functions have become so big that it's  
hard to follow. For example, SplitVecRes_VECTOR_SHUFFLE. Is it  
possible to factor some stuff out?

Also, some of code seems to be replicated in several functions (please  
correct me if that's not the case). For example, the code that  
determines how an element is used.

A stylistic nitpick:

+    if (VT1NumElems*2 == NumElems && SequentialMask(Mask, 0)) {
+      setValue(&I, DAG.getNode(ISD::CONCAT_VECTORS, VT, V1, V2));
+      return;
+    }
+    else {

The else { } isn't needed because of the early exit. Same issue here:

+    if (VT1NumElems == NumElems && SequentialMask(Mask,0)) {
+      setValue(&I, V1);
+      return;
+    } else if (VT1NumElems == NumElems &&  
SequentialMask(Mask,NumElems)) {
+      setValue(&I, V2);
+      return;
+    } else {
+      // Analyze the access pattern of the vector to see if we can  
extract
+      // two subvectors and do the shuffle.

Don't forget to end comment sentences with periods. :-)

Evan

On Oct 31, 2008, at 7:56 PM, Mon Ping Wang wrote:

> <genshuffle3.patch>