Currently bottom-to-top reordering analysis counts orders of the
operands and then adds natural order counts for the operand users. It is
very conservative, this the user nodes themselves may require
reordering. Patch improves bottom-to-top analysis by checking for the
user nodes if they require/allows the reordring. If the user node must
be reordered, has reused scalars, is an alternate op vectorization node,
is a non-ordered gather node or may allow reordering because of the
reordered operands, such node is considered as the node that allows
reodring and is not counted as a node with the natural order.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | ||
---|---|---|
3478–3479 | Please rename Data to something more descriptive. I think it contains a pair of the TreeEntry and a vector of its uses along with their indexes, so something like TEAndUses would be fine. Also it is probably better to use the actual type here instead of auto, ti would make it easier to read. | |
3478–3479 | I think that this needs a better name, perhaps something like CanReorderOperands. | |
3478–3479 | If I understand correctly, this block of code could be a separate member function of the TreeEntry struct that returns the operand TreeEntry: TreeEntry *getOperandTE(unsigned OpIdx). Wdyt ? | |
3481 | It is hard to remember what Data and Data.first refers to at this point. I think it would be better to use a variable to hold the value like TreeEntry *TE = Data.first. I think Data.first also appears earlier in the code. | |
3486 | I am a bit confused with what is going on here. I was under the impression that the Users map holds the edges towards the operands (which btw should probably be part of TreeEntry and built during buildTree?). Why are we adding edges here? | |
3568 | I am not sure what Res refers to. Please use a better name and/or add a comment ? | |
3569 | Perhaps move the comment from line 3554 here? | |
3583 | Could you add a brief comment explaining what this loop does? |
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | ||
---|---|---|
3486 | Oririginally, Users contains edges between reorderable operands only (vector operands with reordering data or loads/extractelements and/or gathers of extractselements that require reordering). Here we add some extra vectorizable operands, if they were not added, because they did not have the reordering data. |
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | ||
---|---|---|
1910 | There is no NonVectorized parameter. Should be ReorderableGathers? |
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp | ||
---|---|---|
1910 | Yep, will fix it |
There is no NonVectorized parameter. Should be ReorderableGathers?