[PATCH] [SLPVectorizer] Reorder operands of shufflevector if it can result in a vectorized code.
mzolotukhin at apple.com
Fri Jan 16 12:32:01 PST 2015
Thanks for the fixes, the patch mostly looks ok, but I still have a bad feeling regarding `reorderInputsAccordingToOpcode`.
Now we have two checks for `if(!(LeftBroadcast || RightBroadcast))` in a row, which doesn't look good. And the overall logic of the function becomes really fuzzy.
You're right that it resembles the original approach. However, I don't think `needToReorder` flag solves the problem in a good way either. To solve it properly, we need to set our priorities first by answering simple questions:
1. If the original operands formed a broadcast, do we want to touch them?
2. What do we prefer: get `AllSameOpcode` operands, or operands, some of which are consecutive?
3. What if after swapping to create consecutive operands we lose `AllSameOpcode` property?
4. What if operands are `AllSameOpcode`, but not consecutive?
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?
I guess that we actually only care about cases in which we can make operands both consecutive, and 'AllSameOpcode' - if either of these two conditions is false, we most probably end up with no-vectorization. Thus, we want to maintain both properties at the same time, bailing out if that's not possible.
As for the broadcasting, I think it's always bad to break it, so once we discover it, we want to keep it.
Do you agree with these general strategies, or do you have another opinion?
If we agree on them, it'll be easy to efficiently align the code around them, like the following:
1. Sort the operands as we do right now.
2. Check for broadcasts, if isSplat(Left) or isSplat(Right) - return.
3. Try to reorder things to create as many consecutive loads as possible.
4. Check if we have AllSameOpcode in either left, or right operands. If yes, return Left and Right, otherwise - LeftOrig and RightOrig.
More information about the llvm-commits