Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[SLPVectorizer] Operand reordering across multiple instructions.
Needs ReviewPublic

Authored by vporpo on Jun 21 2019, 11:50 AM.



This patch implements operand reordering across trees of Add/Subs, referred to as "SuperNodes".
For example, it helps vectorize code like this:
S[0] = (A[0] - B[0]) - C[0]
S[1] = (B[1] + C[1]) + A[1]
A more detailed description of the technique can be found here:

The two main changes introduced by this patch are:
i. The SuperNode class that represents the Add/Sub trees and their immediate operands that we are reordering.
ii. A local change in the buildTree_rec() recursion that forms the SuperNode.

Please note that this patch depends on the scheduler changes of D62432.

Diff Detail

Event Timeline

vporpo created this revision.Jun 21 2019, 11:50 AM

Please can you precommit the tests?

bjope added a subscriber: bjope.Jun 24 2019, 1:44 AM
rcorcs added a subscriber: rcorcs.Jun 24 2019, 3:22 AM
xbolva00 added inline comments.



This looks quite generic so maybe it can be reused. Reassoc pass should have similar code too?



This is same code as the code added just above ? Please move it to own function.

xbolva00 added inline comments.Jun 25 2019, 8:02 AM


RKSimon added inline comments.Jun 25 2019, 8:47 AM

Tweak the brackets to make the ordering more readable:

return (OpIdx == 0) ? UserAPO : (IsUserInverse ? !UserAPO : UserAPO);

Don't use auto unless the type is obvious - there's a number of these in the patch that fixing.

vporpo updated this revision to Diff 206487.Jun 25 2019, 11:05 AM
vporpo marked 5 inline comments as done.

Addressed comments and rebased.


I am not sure what you mean. The only repetition I could move to its own function is the checks in the if condition.
If you are referring to the whole if-else block, then I agree there is a lot of repetition in buildTree_rec(), but it should be part of a separate cleanup patch.

@vporpo Are you still looking at this please?

@RKSimon Yes, I will update this patch in a day or two.

vporpo updated this revision to Diff 229662.Nov 15 2019, 4:22 PM


RKSimon resigned from this revision.Jul 8 2021, 2:10 PM