Page MenuHomePhabricator

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

[PowerPC] Improvements for BUILD_VECTOR Vol. 3

Authored by nemanjai on Oct 27 2016, 1:27 AM.



This is the third (and penultimate) patch that is split into.
The test case will be added with the final patch.

In this patch, support for converting a vector of loads into a single load if the loads are consecutive (in either direction).
There is currently a limitation in that consecutive loads are not recognized if one of them is a pre-increment load.

Diff Detail


Event Timeline

nemanjai updated this revision to Diff 75989.Oct 27 2016, 1:27 AM
nemanjai retitled this revision from to [PowerPC] Improvements for BUILD_VECTOR Vol. 3.
nemanjai updated this object.
nemanjai set the repository for this revision to rL LLVM.
nemanjai added a subscriber: llvm-commits.
kbarton requested changes to this revision.Oct 31 2016, 6:37 PM
kbarton edited edge metadata.

I think the loop on line 10619 needs to be reworked a bit (or I don't understand what it's trying to do).


Please convert to doxygen-style comments.




Haven't you already checked the first operand? Can this loop not start at 1 to ensure the remainder of the operands conform to the necessary conditions? That would eliminate the if (i==0) check below also.

This revision now requires changes to proceed.Oct 31 2016, 6:37 PM
nemanjai added inline comments.Nov 1 2016, 2:49 AM



Good point. This is a remnant of a previous incarnation of this logic which got refactored but obviously incompletely. I'll simplify the loop.

nemanjai updated this revision to Diff 76828.Nov 3 2016, 3:32 AM
nemanjai edited edge metadata.

Fixed the weird loop and added doxygen comments.

kbarton accepted this revision.Nov 9 2016, 12:32 PM
kbarton edited edge metadata.

One minor question, more for my own curiosity, that we can likely address offline. Aside from that, LGTM.


Any reason for a std::vector vs a SmallVector?

This revision is now accepted and ready to land.Nov 9 2016, 12:32 PM
nemanjai added inline comments.Nov 10 2016, 7:30 AM

That's a good point. The SmallVector is a better choice here. I'll change it prior to comitting.

nemanjai closed this revision.Nov 29 2016, 4:08 PM

Committed revision 288219.