Page MenuHomePhabricator

[AArch64][SVE] Allow lax conversion between VLATs and GNU vectors
ClosedPublic

Authored by joechrisellis on Nov 18 2020, 3:51 AM.

Details

Summary

Previously, lax conversions were only allowed between SVE vector-length
agnostic types and vector-length specific types. This meant that code
such as the following:

#include <arm_sve.h>
#define N __ARM_FEATURE_SVE_BITS
#define FIXED_ATTR __attribute__ ((vector_size (N/8)))
typedef float fixed_float32_t FIXED_ATTR;

void foo() {
    fixed_float32_t fs32;
    svfloat64_t s64;
    fs32 = s64;
}

was not allowed.

This patch makes a minor change to areLaxCompatibleSveTypes to allow for
lax conversions to be performed between SVE vector-length agnostic types
and GNU vectors.

Diff Detail

Event Timeline

joechrisellis created this revision.Nov 18 2020, 3:51 AM
joechrisellis requested review of this revision.Nov 18 2020, 3:51 AM
willlovett added a subscriber: willlovett.EditedNov 18 2020, 4:02 AM

I found this one. The fix looks sensible, and I believe the code being tested is consistent with the spec, but I'll leave it to someone else to accept (once the tests are passing)

Remove redundant tests from clang/test/Sema/attr-arm-sve-vector-bits.c.

fpetrogalli accepted this revision.Nov 18 2020, 4:46 AM

This LGTM. May I ask to extend the commit message to add a reference to the paragraph in section "3.7.3.3", item 2 on page 23 of the specs version 00bet6?

This revision is now accepted and ready to land.Nov 18 2020, 4:46 AM
c-rhodes added inline comments.
clang/test/Sema/attr-arm-sve-vector-bits.c
278–283

I don't think this can be removed. The ACLE states "Whenever __ARM_FEATURE_SVE_BITS==N, GNUT implicitly converts to VLAT and VLAT implicitly converts to GNUT.".

AFAIK lax vector conversions only apply to vectors of the same width, with GNU vectors for example the following is invalid regardless of lax vector conversions:

typedef int8_t int8x16_t __attribute__((vector_size(16)));
typedef int8_t int8x64_t __attribute__((vector_size(64)));

int8x16_t foo(int8x64_t x) { return x; }
joechrisellis marked an inline comment as done.

Address @c-rhodes's comments regarding lax conversion when __ARM_FEATURE_SVE_BITS != N for GNU vectors.

clang/test/Sema/attr-arm-sve-vector-bits.c
278–283

Great spot, didn't see that in the spec. I've re-added the test and added an extra condition to check if __ARM_FEATURE_SVE_BITS == N before allowing the lax conversion for GNU vectors. Not sure if this is the best way to do it, though. :)

c-rhodes accepted this revision.Nov 20 2020, 4:00 AM

Address @c-rhodes's comments regarding lax conversion when __ARM_FEATURE_SVE_BITS != N for GNU vectors.

Cheers, LGTM

This revision was landed with ongoing or failed builds.Nov 23 2020, 2:47 AM
This revision was automatically updated to reflect the committed changes.