[PATCH] [SLPVectorizer] Reorder operands of shufflevector if it can result in a vectorized code.

[Karthik Bhat]

Hi Michael,
Thanks for the review. Addressed review comments after slight modification in code flow. This version does minimal changes in reorderInputsAccordingToOpcode and to make logic clear i have added appropriate comments in the code. 
The only change is the check we have added at the end of reorderInputsAccordingToOpcode to reorder operands to create a longer vectorizable chain without effecting AllSameOpcode property.

I hope this makes things clearer in reorderInputsAccordingToOpcode?

Please find the updated patch and comments inline-

1. If the original operands formed a broadcast, do we want to touch them? > No. As you mentioned if we detect a broadcast we do not reorder them. We can return as soon as we detect a broadcast.

2. What do we prefer: get AllSameOpcode operands, or operands, some of which are consecutive? > If we have AllSameOpcode operands reodering it so that consecutive loads are grouped together will not change AllSameOpcode property. > This case (i.e AllSameOpcode and code hits our swap logic) is only possible when we have all loads in left and right side of the binary > operation.)

3. What if after swapping to create consecutive operands we lose AllSameOpcode property? > This is not possible as far as i can tell. > If we have AllSameOpcode and if we enter in to the condition to swap to create consecutive operands. It means that we have all loads in > left and right lane. Hence swaping will retain AllSameOpcode property as we will still have loads on either side after swap.

4. What if operands are AllSameOpcode, but not consecutive? > Our logic should not alter anything in this scenario.

5. What if left operands are broadcast, but if we swap one of them with a right one, we'll get consecutive access in the right operands? > As you mentioned we don't want to disturb boardcast. Hence we do not do anything here.

Comments on code flow suggested -

1. Sort the operands as we do right now. > OK
2. Check for broadcasts, if isSplat(Left) or isSplat(Right) - return. > OK
3. Try to reorder things to create as many consecutive loads as possible. > OK
4. Check if we have AllSameOpcode in either left, or right operands. If yes, return Left and Right, otherwise - LeftOrig and RightOrig. > We will have a problem here. As mentioned above in case we have AllSameOpcode there are 2 cases which we have to handle- > 1) If operands are AllSameOpcode but reordering will result in consecutive loads and retain *AllSameOpcode* property. > 2) If operands are AllSameOpcode but not consecutive loads. > So in the 1st case above we would like to reorder operands but in the second case we return the LeftOrig and RightOrig. > In both these case AllSameOpcode is the same but in one case we want to reorder and other case we want to return LeftOrig and RightOrig. > So if we follow this code flow any operands reordered in step 3 will be discarded and we will return LeftOrig and RightOrig preventing > vectorization.

So the final  code flow will be something like -

1. Sort the operands as we do right now. (Same as in original code)
2. If broadcast then return. (Same as in original code but we return now as you suggested)
3. Check if we have AllSameOpcode if yes retain the original left/right order (same as original code)
4. Check if we can reorder the opcodes without disturbing good operand order if yes reorder the same. (Our logic)

Thanks
Karthik Bhat


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D6677

Files:
  lib/Transforms/Vectorize/SLPVectorizer.cpp
  test/Transforms/SLPVectorizer/X86/addsub.ll
  test/Transforms/SLPVectorizer/X86/operandorder.ll

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D6677.18366.patch
Type: text/x-patch
Size: 23763 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150119/f63be48c/attachment.bin>