This is an archive of the discontinued LLVM Phabricator instance.

[SLP] Fix for PR32086: Count InsertElementInstr of the same elements as shuffle.
ClosedPublic

Authored by ABataev on Oct 9 2017, 11:26 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ABataev created this revision.Oct 9 2017, 11:26 AM
RKSimon edited edge metadata.Oct 10 2017, 4:37 AM

This patch seems to change the blending-shuffle.ll test case in the same way as D38693 - what is the relationship/dependency between them?

This patch seems to change the blending-shuffle.ll test case in the same way as D38693 - what is the relationship/dependency between them?

Yes, these patches do the same for this particular tests. But D38693 fixes the global problem with the handling of ExtractElementInsts, while this one can fix the only pattern used in blending-shuffle.ll. These patches can do the same, but only for limited number of patterns.

RKSimon added inline comments.Oct 10 2017, 1:09 PM
lib/Transforms/Vectorize/SLPVectorizer.cpp
658 ↗(On Diff #118241)

Spelling mistake - ShuffledIndices not ShuffledIndicies

1492 ↗(On Diff #118241)
if (Res.second)
   UniqueValues.emplace_back(V);
2003 ↗(On Diff #118241)

Is this better?

if (!E->ReuseShuffleIndicies.empty())
ABataev marked 3 inline comments as done.Oct 11 2017, 7:05 AM
ABataev updated this revision to Diff 118611.Oct 11 2017, 7:20 AM

Address comments.

D38693 and D38697 appear to be touching much of the same code - should this one be given priority and D38693 rebased as dependent upon it?

ABataev updated this revision to Diff 130443.Jan 18 2018, 10:19 AM

Fixed hoisting + correct generation of shuffle for gather sequence.

RKSimon accepted this revision.Jan 23 2018, 6:51 AM

LGTM

This revision is now accepted and ready to land.Jan 23 2018, 6:51 AM
This revision was automatically updated to reflect the committed changes.