Page MenuHomePhabricator

[SVE][CodeGen] Bail out for scalable vectors in AArch64TargetLowering::ReconstructShuffle
ClosedPublic

Authored by david-arm on Jan 4 2022, 8:23 AM.

Details

Summary

Previously the code in AArch64TargetLowering::ReconstructShuffle assumed
the input vectors were always fixed-width, however this is not always
the case since you can extract elements from scalable vectors and insert
into fixed-width ones. We were hitting crashes here for two different
cases:

  1. When lowering a fixed-length vector extract from a scalable vector

with i1 element types. This happens due to the fact the i1 elements
get promoted to larger integer types for fixed-width vectors and leads
to sequences of INSERT_VECTOR_ELT and EXTRACT_VECTOR_ELT nodes. In this
case AArch64TargetLowering::ReconstructShuffle will still fail to make
a transformation, but at least it no longer crashes.

  1. When lowering a sequence of extractelement/insertelement operations

on mixed fixed-width/scalable vectors.

For now, I've just changed AArch64TargetLowering::ReconstructShuffle to
bail out if it finds a scalable vector.

Tests for both instances described above have been added here:

(1) CodeGen/AArch64/sve-extract-fixed-vector.ll
(2) CodeGen/AArch64/sve-fixed-length-reshuffle.ll

Diff Detail

Event Timeline

david-arm created this revision.Jan 4 2022, 8:23 AM
david-arm requested review of this revision.Jan 4 2022, 8:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2022, 8:23 AM
Matt added a subscriber: Matt.Jan 7 2022, 7:31 AM
david-arm updated this revision to Diff 407469.Feb 10 2022, 4:11 AM
david-arm edited the summary of this revision. (Show Details)
  • Split patch into two - first patch bails out of the optimisation, second patch will add support for it with scalable vectors.
david-arm retitled this revision from [SVE][CodeGen] Add support for scalable vectors in AArch64TargetLowering::ReconstructShuffle to [SVE][CodeGen] Bail out for scalable vectors in AArch64TargetLowering::ReconstructShuffle.Feb 10 2022, 4:13 AM
david-arm edited the summary of this revision. (Show Details)
sdesmalen added inline comments.Feb 10 2022, 4:31 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
8989–8990

You can bail out sooner I think, by adding a || V.getOperand(0).getValueType().isScalableVectorType() here.

david-arm updated this revision to Diff 407487.Feb 10 2022, 5:09 AM
david-arm marked an inline comment as done.
sdesmalen accepted this revision.Feb 10 2022, 5:39 AM

LGTM, but I'd prefer if you could move out some of the tests.

llvm/test/CodeGen/AArch64/sve-fixed-length-reshuffle.ll
27

Other than the test above, I'd suggest removing this test and the others below from this patch and submitting them as a separate NFC patch (as precursor to D119427).

This revision is now accepted and ready to land.Feb 10 2022, 5:39 AM
This revision was landed with ongoing or failed builds.Feb 10 2022, 6:19 AM
This revision was automatically updated to reflect the committed changes.