This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Improvements for BUILD_VECTOR Vol. 3
ClosedPublic

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

Details

Summary

This is the third (and penultimate) patch that https://reviews.llvm.org/D25580 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

Repository
rL LLVM

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).

lib/Target/PowerPC/PPCISelLowering.cpp
10601

Please convert to doxygen-style comments.

10625

++i

10625

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
lib/Target/PowerPC/PPCISelLowering.cpp
10601

OK.

10625

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.

lib/Target/PowerPC/PPCISelLowering.cpp
10678

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
lib/Target/PowerPC/PPCISelLowering.cpp
10678

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.