Page MenuHomePhabricator

[clang][aarch64] Address various fixed-length SVE vector operations
ClosedPublic

Authored by c-rhodes on Sep 24 2020, 8:34 AM.

Details

Summary

This patch adds tests and support for operations on SVE vectors created
by the 'arm_sve_vector_bits' attribute, described by the Arm C Language
Extensions (ACLE, version 00bet6, section 3.7.3.3) for SVE [1].

This covers the following:

  • VLSTs support the same forms of element-wise initialization as GNU vectors.
  • VLSTs support the same built-in C and C++ operators as GNU vectors.
  • Conditional and binary expressions containing GNU and SVE vectors (fixed or sizeless) are invalid since the ambiguity around the result type affects the ABI.

No functional changes were required to support vector initialization and
operators. The functional changes are to address unsupported conditional and
binary expressions.

[1] https://developer.arm.com/documentation/100987/latest

Diff Detail

Event Timeline

c-rhodes created this revision.Sep 24 2020, 8:34 AM
Herald added a project: Restricted Project. · View Herald Transcript
c-rhodes requested review of this revision.Sep 24 2020, 8:34 AM

Hi @c-rhodes, Peter asked me to take a look at this. LGTM, I only have minor stuff.

In the commit message:

Arm C Language Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE [1].

It seems that a reference to [1] is missing: https://developer.arm.com/documentation/100987/latest
Also, please note that the current version is 00bet6. Nothing seem to have changed from 00bet5 to 00bet6 in terms of this patch, but I think it is worth keeping it up to date with the specs numbering until it is merged into master.

Please fix the commit message before submitting.

Other than that, I only have a couple of nits that I'll let you decide what to do.

Thanks!

Francesco

clang/test/Sema/attr-arm-sve-vector-bits.c
137

Nit: I spent some time trying to verify that this was the right test. It would be grate if each test had some reference about the item and page stating the rule that the test is testing. Too bad that the items in the spec don't have a number that can easily referred to...

141–158

Nit: should you test more binary operators other than just +, like you have done for the vector initialization tests?

318

Nit: the define(...) seems redundant. Can this be #if __ARM_FEATURE_SVE_BITS == 256?

c-rhodes updated this revision to Diff 301603.Thu, Oct 29, 7:02 AM
c-rhodes edited the summary of this revision. (Show Details)

Address comments

c-rhodes marked 2 inline comments as done.Thu, Oct 29, 7:05 AM

Hi @c-rhodes, Peter asked me to take a look at this. LGTM, I only have minor stuff.

In the commit message:

Arm C Language Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE [1].

It seems that a reference to [1] is missing: https://developer.arm.com/documentation/100987/latest
Also, please note that the current version is 00bet6. Nothing seem to have changed from 00bet5 to 00bet6 in terms of this patch, but I think it is worth keeping it up to date with the specs numbering until it is merged into master.

Please fix the commit message before submitting.

I didn't realise 00bet6 had been released, thanks for pointing that out.

Thanks for reviewing!

clang/test/Sema/attr-arm-sve-vector-bits.c
141–158

Nit: should you test more binary operators other than just +, like you have done for the vector initialization tests?

Added tests for a couple more operators.

fpetrogalli accepted this revision.Thu, Oct 29, 10:31 AM

LGTM, thank you!

This revision is now accepted and ready to land.Thu, Oct 29, 10:31 AM
This revision was automatically updated to reflect the committed changes.
c-rhodes marked an inline comment as done.