Page MenuHomePhabricator

[AArch64][SelectionDAG] Support passing/returning scalable vectors with unusual types.
ClosedPublic

Authored by efriedma on Jul 7 2021, 1:54 PM.

Details

Summary

This adds handling for two cases:

  1. A scalable vector where the element type is promoted.
  2. A scalable vector where the element count is odd (or more generally, not divisble by the element count of the part type).

(Some element types still don't work; for example, <vscale x 2 x i128>, or <vscale x 2 x fp128>.)

The change to SplitVecRes_INSERT_SUBVECTOR needs to be considered in the context of the discussion of "widening" a scalable vector.

Diff Detail

Event Timeline

efriedma created this revision.Jul 7 2021, 1:54 PM
efriedma requested review of this revision.Jul 7 2021, 1:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2021, 1:54 PM
Matt added a subscriber: Matt.Jul 20 2021, 6:46 AM
sdesmalen accepted this revision.Aug 2 2021, 1:51 PM

The patch is a bit tricky to follow because it has many changing parts, so it's not always clear that all the parts are covered by the tests, but I trust you've made all these changes for a reason.
The changes themselves seem sensible to me, so I'm happy to accept it. As the title suggests, the types are all quite unusual, so if there's parts that aren't bullet proof we can fix those when we run into them.

Does D106028 depend on this patch? (I'm thinking mostly about the widening)

This revision is now accepted and ready to land.Aug 2 2021, 1:51 PM

The patch is a bit tricky to follow because it has many changing parts, so it's not always clear that all the parts are covered by the tests, but I trust you've made all these changes for a reason.

I'll experiment a bit to see if there's some way to separate out the legalization changes; the llvm.experimental.vector.extract tricks I figured out for D106028 might be usable for that. The rest is sort of tied together.

Does D106028 depend on this patch? (I'm thinking mostly about the widening)

There isn't an explicit dependency. The SplitVecRes_INSERT_SUBVECTOR here is a less general version of the one in D106028.

The patch is a bit tricky to follow because it has many changing parts, so it's not always clear that all the parts are covered by the tests, but I trust you've made all these changes for a reason.

I'll experiment a bit to see if there's some way to separate out the legalization changes; the llvm.experimental.vector.extract tricks I figured out for D106028 might be usable for that. The rest is sort of tied together.

Oh, right... if I try that, I get into issues with the extract/insert themselves, which end up needing more code to make them work.

I think it's reasonable to commit this as a single unit, even if it might theoretically be possible to split it more.

Oh, oops, just realized the LegalizeVectorTypes aren't necessary here, at least for the most basic cases. I'll commit without them.

This revision was landed with ongoing or failed builds.Aug 2 2021, 3:53 PM
This revision was automatically updated to reflect the committed changes.