Page MenuHomePhabricator

Unpack array of all sizes in InstCombine

Authored by deadalnix on Jan 5 2016, 6:22 AM.



This is another step toward improving fca support. This unpack load of array in a series of load to array's elements.

Diff Detail

Event Timeline

deadalnix updated this revision to Diff 43993.Jan 5 2016, 6:22 AM
deadalnix retitled this revision from to Unpack array of all sizes in InstCombine.
deadalnix updated this object.
deadalnix added a subscriber: llvm-commits.
mgrang added a subscriber: mgrang.Feb 1 2016, 3:48 PM
mgrang added inline comments.

Why can't we simply call LI.getName() wherever it is required rather than maintaining the variable "Name"?


Same here. Do we really need to maintain NumElements? Can we simply call ST->getNumElements() instead?


I think these could be written better as:

SmallString<16> LoadName = LI->getName() + ".unpack";
SmallString<16> EltName = LI->getName() + ".elt";

deadalnix added inline comments.Feb 1 2016, 5:37 PM

Is that a


Is that a problem ? I generally avoid calling the same thing again and again. In this case, it probably doesn't matter much.

error: no viable conversion from 'llvm::Twine' to 'SmallString<16>'
deadalnix updated this revision to Diff 48160.Feb 17 2016, 12:59 AM

Fix alignement on generated loads.

deadalnix updated this revision to Diff 48220.Feb 17 2016, 11:25 AM

clang format

majnemer edited edge metadata.Mar 1 2016, 5:32 PM

Does this do the right thing with wacky stuff like arrays of i3?


Arrays can have more than 2**32 elements.


You should use uint64_t here.

deadalnix updated this revision to Diff 49573.Mar 1 2016, 5:51 PM
deadalnix edited edge metadata.

Use 64 bits indices

majnemer accepted this revision.Mar 2 2016, 1:21 PM
majnemer edited edge metadata.


This revision is now accepted and ready to land.Mar 2 2016, 1:21 PM
This revision was automatically updated to reflect the committed changes.