This is an archive of the discontinued LLVM Phabricator instance.

[LoopVectorizer] NFC: Return ElementCount from compute[Feasible]MaxVF
ClosedPublic

Authored by sdesmalen on Nov 5 2020, 1:40 PM.

Details

Summary

Interfaces changed to return ElementCount:

  • LoopVectorizationCostModel::computeMaxVF
  • LoopVectorizationCostModel::computeFeasibleMaxVF

This is NFC for fixed-width vectors.

Diff Detail

Event Timeline

sdesmalen created this revision.Nov 5 2020, 1:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 5 2020, 1:40 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
sdesmalen requested review of this revision.Nov 5 2020, 1:40 PM
ctetreau accepted this revision.Nov 9 2020, 9:38 AM

looks good to me

This revision is now accepted and ready to land.Nov 9 2020, 9:38 AM

Is expected to make anything work yet for scalable vectors, or is it just updating interfaces? (i.e. will the code be checked somehow to make sure it will work for scalable vectors).

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
5283

Would this TC mod check still be valid for scalable vectors? If not does it need an assert?

sdesmalen updated this revision to Diff 304164.Nov 10 2020, 6:19 AM

Added assert

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
5283

Good spot, you're right that this shouldn't work yet for scalable vectors. I've added an assert here.

Is expected to make anything work yet for scalable vectors, or is it just updating interfaces? (i.e. will the code be checked somehow to make sure it will work for scalable vectors).

For this patch, it is just a matter of updating the interfaces. It will be up to follow-up patches to make the code actually work for scalable vectors (such as D91077).
That's why adding the assert on line 5271 that scalable vectors are not yet supported is good enough for this patch.

dmgreen accepted this revision.Nov 10 2020, 6:35 AM

Makes sense, thanks