This is an archive of the discontinued LLVM Phabricator instance.

[clang][AArch64][SVE] Avoid going through memory for coerced VLST arguments
ClosedPublic

Authored by joechrisellis on Dec 7 2020, 6:44 AM.

Details

Summary

VLST arguments are coerced to VLATs at the function boundary for
consistency with the VLAT ABI. They are then bitcast back to VLSTs in
the function prolog. Previously, this conversion is done through memory.
With the introduction of the llvm.vector.{insert,extract} intrinsic, we
can avoid going through memory here.

Depends on D92761

Diff Detail

Event Timeline

joechrisellis created this revision.Dec 7 2020, 6:44 AM
joechrisellis requested review of this revision.Dec 7 2020, 6:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 7 2020, 6:44 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Do not try to generate llvm.experimental.vector.extract instructions if the vector element types differ.

c-rhodes added inline comments.Dec 18 2020, 7:50 AM
clang/lib/CodeGen/CGCall.cpp
2691–2693

this is slightly confusing since the coercion done in TargetInfo is from fixed -> scalable so VLSTs are represented as scalable vectors in functions args/return, yet this is casting back to fixed in the function prolog using llvm.experimental.vector.extract like you mention in the commit message, could this comment clarify that?

2694

Do we want to check VT->getVectorKind() == VectorType::SveFixedLengthDataVector and isa<llvm::ScalableVectorType>(Coerced->getType());?

2701

this-> can be dropped?

joechrisellis marked 2 inline comments as done.

Address @c-rhodes's comments.

  • Make comment clearer.
  • Change structure of the if statements for better readability.
  • Drop superfluous this->.
c-rhodes accepted this revision.Jan 4 2021, 5:01 AM
c-rhodes added inline comments.
clang/lib/CodeGen/CGCall.cpp
2706

nit: I know we've used cast<Fixed|Scalable>Sve for Name in a couple of places already and it's not very descriptive, but I think it describes the type being cast to, in which case this should be castFixedSve.

This revision is now accepted and ready to land.Jan 4 2021, 5:01 AM

Address @c-rhodes's comment.

  • Use name castFixedSve instead of castScalableSve.
joechrisellis marked an inline comment as done.Jan 4 2021, 5:10 AM
joechrisellis added inline comments.
clang/lib/CodeGen/CGCall.cpp
2691–2693

I am a bit unsure what you mean in this comment, but I have tried to reword the comment to be a bit clearer about what's happening. Let me know if you still think it needs clarifying. 😄

2694

Do we not also want to account for predicate vectors here?

c-rhodes added inline comments.Jan 4 2021, 5:38 AM
clang/lib/CodeGen/CGCall.cpp
2694

Do we not also want to account for predicate vectors here?

I think so, but as mentioned on D92761 it'll require more thought since the insert/extract intrinsics require the element type to be identical which they aren't for predicates (i8 for VLST and i1 for scalable).