This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] NFC: Change LLT::vector to take ElementCount.
ClosedPublic

Authored by sdesmalen on Jun 17 2021, 5:47 AM.

Details

Summary

This also adds new interfaces for the fixed- and scalable case:

  • LLT::fixed_vector
  • LLT::scalable_vector

The strategy for migrating to the new interfaces was as follows:

  • If the new LLT is a (modified) clone of another LLT, taking the same number of elements, then use LLT::vector(OtherTy.getElementCount()) or if the number of elements is halfed/doubled, it uses .divideCoefficientBy(2) or operator*. That is because there is no reason to specifically restrict the types to 'fixed_vector'.
  • If the algorithm works on the number of elements (as unsigned), then just use fixed_vector. This will need to be fixed up in the future when modifying the algorithm to also work for scalable vectors, and will need then need additional tests to confirm the behaviour works the same for scalable vectors.
  • If the test used the '/*Scalable=*/true` flag of LLT::vector, then this is replaced by LLT::scalable_vector.

Diff Detail

Event Timeline

sdesmalen created this revision.Jun 17 2021, 5:47 AM
sdesmalen requested review of this revision.Jun 17 2021, 5:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2021, 5:47 AM
Matt added a subscriber: Matt.Jun 19 2021, 7:58 AM
arsenm added inline comments.Jun 21 2021, 8:22 AM
llvm/include/llvm/Support/LowLevelTypeImpl.h
100

Is there actually a context where you would use this which would need a scalable vector?

sdesmalen added inline comments.Jun 21 2021, 8:47 AM
llvm/include/llvm/Support/LowLevelTypeImpl.h
100

The context would be where the algorithm that uses scalarOrVector is agnostic of whether the type is fixed or scalable and just passes an ElementCount.
As an example: LLT::divide(int Factor) would return v2i64 for v4i64.divide(2), nxv2i64 for nxv4i64.divide(2), but i64 for v2i64.divide(2). And instead of having to write if conditional code, it can work on ElementCount directly (see its implementation in D104452)

aemerson accepted this revision.Jun 22 2021, 2:24 PM

Thanks for doing this. Adding scalable vector support for GISel is an important step in the effort to bring feature parity with SelectionDAG in the long term, especially on AArch64.

The rationale for the changes makes sense to me. LGTM.

This revision is now accepted and ready to land.Jun 22 2021, 2:24 PM
This revision was landed with ongoing or failed builds.Jun 24 2021, 3:27 AM
This revision was automatically updated to reflect the committed changes.