Page MenuHomePhabricator

[SVE] Remove calls to VectorType::getNumElements from CodeGen
ClosedPublic

Authored by ctetreau on Jun 19 2020, 10:14 AM.

Diff Detail

Event Timeline

ctetreau created this revision.Jun 19 2020, 10:14 AM
Herald added a project: Restricted Project. · View Herald Transcript
RKSimon accepted this revision.Jul 1 2020, 1:24 AM

LGTM with one minor

llvm/lib/CodeGen/ScalarizeMaskedMemIntrin.cpp
522–523

I know you're being cautious about changes but I think this assertion can use isa<FixedVectorType>? Or drop the assert entirely - cast<> implicitly asserts for the correct type.

This revision is now accepted and ready to land.Jul 1 2020, 1:24 AM
ctetreau updated this revision to Diff 274848.Jul 1 2020, 9:39 AM

address code review issues

@sdesmalen Would you like me to hold off on this one until after the branch?

@sdesmalen Would you like me to hold off on this one until after the branch?

Thanks for checking @ctetreau. I actually think most of the changes are fine to go in now, with the exception of:

  • the cast to FixedVectorType in llvm/lib/CodeGen/ValueTypes.cpp.
  • the changes to InterleavedAccessPass, because this pass is not yet guarded by bailing out early.

The other changes are fine because:

  • The ACLE uses custom target-specific intrinsics instead of llvm.masked.load/store/gather/scatter intrinsics.
  • The ACLE uses custom target-specific intrinsics instead of the llvm.experimental.reduction intrinsics.
  • GlobalISel already falls back to DAGISel if the function uses scalable vectors (D81557 and D82524)
  • The InterleavedLoadCombinePass was guarded in D79700 to bail out early for scalable vectors.
  • shouldConvertSplatType returns nullptr for AArch64 so the change in CodeGenPrepare::optimizeShuffleVectorInst is never hit.

If you can add a check to bail out early for the InterleavedAccessPass for scalable vectors and move the change to EVT::getExtendedVectorNumElements into a separate patch, then I'm happy with most of this to go in now.

ctetreau updated this revision to Diff 276506.Jul 8 2020, 11:51 AM

address code review issues

@sdesmalen I went ahead and made the requested changes. In ValueType.cpp, rather than leaving the code alone, I went ahead and returned getElementCount().Min along with adding a warning if the vector was scalable. This way getNumElement() can be removed in the future without having to touch this file again.

ctetreau updated this revision to Diff 276810.Jul 9 2020, 12:43 PM

account for abandoned D82329

This revision was automatically updated to reflect the committed changes.