Skip to content

Commit ada01d1

Browse files
statham-armstatham-arm
committed
[clang] New __attribute__((__clang_arm_mve_strict_polymorphism)).
This is applied to the vector types defined in <arm_mve.h> for use with the intrinsics for the ARM MVE vector architecture. Its purpose is to inhibit lax vector conversions, but only in the context of overload resolution of the MVE polymorphic intrinsic functions. This solves an ambiguity problem with polymorphic MVE intrinsics that take a vector and a scalar argument: the scalar argument can often have the wrong integer type due to default integer promotions or unsuffixed literals, and therefore, the type of the vector argument should be considered trustworthy when resolving MVE polymorphism. As part of the same change, I've added the new attribute to the declarations generated by the MveEmitter Tablegen backend (and corrected a namespace issue with the other attribute while I was there). Reviewers: aaron.ballman, dmgreen Reviewed By: aaron.ballman Subscribers: kristof.beyls, JDevlieghere, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D72518
1 parent 44f9c7a commit ada01d1

File tree

8 files changed

+197
-3
lines changed

8 files changed

+197
-3
lines changed

‎clang/include/clang/Basic/Attr.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,11 @@ def NeonVectorType : TypeAttr {
14791479
let ASTNode = 0;
14801480
}
14811481

1482+
def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr<TargetARM> {
1483+
let Spellings = [Clang<"__clang_arm_mve_strict_polymorphism">];
1484+
let Documentation = [ArmMveStrictPolymorphismDocs];
1485+
}
1486+
14821487
def NoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> {
14831488
let Spellings = [CXX11<"", "no_unique_address", 201803>];
14841489
let Subjects = SubjectList<[NonBitField], ErrorDiag>;

‎clang/include/clang/Basic/AttrDocs.td

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4789,3 +4789,45 @@ close the handle. It is also assumed to require an open handle to work with.
47894789
zx_status_t zx_handle_close(zx_handle_t handle [[clang::release_handle]]);
47904790
}];
47914791
}
4792+
4793+
def ArmMveStrictPolymorphismDocs : Documentation {
4794+
let Category = DocCatType;
4795+
let Content = [{
4796+
This attribute is used in the implementation of the ACLE intrinsics for the Arm
4797+
MVE instruction set. It is used to define the vector types used by the MVE
4798+
intrinsics.
4799+
4800+
Its effect is to modify the behavior of a vector type with respect to function
4801+
overloading. If a candidate function for overload resolution has a parameter
4802+
type with this attribute, then the selection of that candidate function will be
4803+
disallowed if the actual argument can only be converted via a lax vector
4804+
conversion. The aim is to prevent spurious ambiguity in ARM MVE polymorphic
4805+
intrinsics.
4806+
4807+
.. code-block:: c++
4808+
4809+
void overloaded(uint16x8_t vector, uint16_t scalar);
4810+
void overloaded(int32x4_t vector, int32_t scalar);
4811+
uint16x8_t myVector;
4812+
uint16_t myScalar;
4813+
4814+
// myScalar is promoted to int32_t as a side effect of the addition,
4815+
// so if lax vector conversions are considered for myVector, then
4816+
// the two overloads are equally good (one argument conversion
4817+
// each). But if the vector has the __clang_arm_mve_strict_polymorphism
4818+
// attribute, only the uint16x8_t,uint16_t overload will match.
4819+
overloaded(myVector, myScalar + 1);
4820+
4821+
However, this attribute does not prohibit lax vector conversions in contexts
4822+
other than overloading.
4823+
4824+
.. code-block:: c++
4825+
4826+
uint16x8_t function();
4827+
4828+
// This is still permitted with lax vector conversion enabled, even
4829+
// if the vector types have __clang_arm_mve_strict_polymorphism
4830+
int32x4_t result = function();
4831+
4832+
}];
4833+
}

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6593,6 +6593,8 @@ def note_objc_unsafe_perform_selector_method_declared_here : Note<
65936593
"method %0 that returns %1 declared here">;
65946594
def err_attribute_arm_mve_alias : Error<
65956595
"'__clang_arm_mve_alias' attribute can only be applied to an ARM MVE builtin">;
6596+
def err_attribute_arm_mve_polymorphism : Error<
6597+
"'__clang_arm_mve_strict_polymorphism' attribute can only be applied to an MVE/NEON vector type">;
65966598

65976599
def warn_setter_getter_impl_required : Warning<
65986600
"property %0 requires method %1 to be defined - "

‎clang/lib/AST/TypePrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
15581558
case attr::AcquireHandle:
15591559
OS << "acquire_handle";
15601560
break;
1561+
case attr::ArmMveStrictPolymorphism:
1562+
OS << "__clang_arm_mve_strict_polymorphism";
1563+
break;
15611564
}
15621565
OS << "))";
15631566
}

‎clang/lib/Sema/SemaOverload.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1653,9 +1653,13 @@ static bool IsVectorConversion(Sema &S, QualType FromType,
16531653
// 1)vector types are equivalent AltiVec and GCC vector types
16541654
// 2)lax vector conversions are permitted and the vector types are of the
16551655
// same size
1656+
// 3)the destination type does not have the ARM MVE strict-polymorphism
1657+
// attribute, which inhibits lax vector conversion for overload resolution
1658+
// only
16561659
if (ToType->isVectorType() && FromType->isVectorType()) {
16571660
if (S.Context.areCompatibleVectorTypes(FromType, ToType) ||
1658-
S.isLaxVectorConversion(FromType, ToType)) {
1661+
(S.isLaxVectorConversion(FromType, ToType) &&
1662+
!ToType->hasAttr(attr::ArmMveStrictPolymorphism))) {
16591663
ICK = ICK_Vector_Conversion;
16601664
return true;
16611665
}

‎clang/lib/Sema/SemaType.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7360,6 +7360,23 @@ static void HandleNeonVectorTypeAttr(QualType &CurType, const ParsedAttr &Attr,
73607360
CurType = S.Context.getVectorType(CurType, numElts, VecKind);
73617361
}
73627362

7363+
static void HandleArmMveStrictPolymorphismAttr(TypeProcessingState &State,
7364+
QualType &CurType,
7365+
ParsedAttr &Attr) {
7366+
const VectorType *VT = dyn_cast<VectorType>(CurType);
7367+
if (!VT || VT->getVectorKind() != VectorType::NeonVector) {
7368+
State.getSema().Diag(Attr.getLoc(),
7369+
diag::err_attribute_arm_mve_polymorphism);
7370+
Attr.setInvalid();
7371+
return;
7372+
}
7373+
7374+
CurType =
7375+
State.getAttributedType(createSimpleAttr<ArmMveStrictPolymorphismAttr>(
7376+
State.getSema().Context, Attr),
7377+
CurType, CurType);
7378+
}
7379+
73637380
/// Handle OpenCL Access Qualifier Attribute.
73647381
static void HandleOpenCLAccessAttr(QualType &CurType, const ParsedAttr &Attr,
73657382
Sema &S) {
@@ -7544,6 +7561,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
75447561
VectorType::NeonPolyVector);
75457562
attr.setUsedAsTypeAttr();
75467563
break;
7564+
case ParsedAttr::AT_ArmMveStrictPolymorphism: {
7565+
HandleArmMveStrictPolymorphismAttr(state, type, attr);
7566+
attr.setUsedAsTypeAttr();
7567+
break;
7568+
}
75477569
case ParsedAttr::AT_OpenCLAccess:
75487570
HandleOpenCLAccessAttr(type, attr, state.getSema());
75497571
attr.setUsedAsTypeAttr();

‎clang/test/Sema/overload-arm-mve.c

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature +mve.fp -flax-vector-conversions=all -Werror -emit-llvm -o - %s | FileCheck %s
2+
// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature +mve.fp -flax-vector-conversions=all -verify -fsyntax-only -DERROR_CHECK %s
3+
4+
typedef signed short int16_t;
5+
typedef signed int int32_t;
6+
typedef signed long long int64_t;
7+
typedef unsigned short uint16_t;
8+
typedef unsigned int uint32_t;
9+
typedef unsigned long long uint64_t;
10+
11+
typedef __attribute__((neon_vector_type(8), __clang_arm_mve_strict_polymorphism)) int16_t int16x8_t;
12+
typedef __attribute__((neon_vector_type(4), __clang_arm_mve_strict_polymorphism)) int32_t int32x4_t;
13+
typedef __attribute__((neon_vector_type(2), __clang_arm_mve_strict_polymorphism)) int64_t int64x2_t;
14+
typedef __attribute__((neon_vector_type(8), __clang_arm_mve_strict_polymorphism)) uint16_t uint16x8_t;
15+
typedef __attribute__((neon_vector_type(4), __clang_arm_mve_strict_polymorphism)) uint32_t uint32x4_t;
16+
typedef __attribute__((neon_vector_type(2), __clang_arm_mve_strict_polymorphism)) uint64_t uint64x2_t;
17+
18+
__attribute__((overloadable))
19+
int overload(int16x8_t x, int16_t y); // expected-note {{candidate function}}
20+
__attribute__((overloadable))
21+
int overload(int32x4_t x, int32_t y); // expected-note {{candidate function}}
22+
__attribute__((overloadable))
23+
int overload(uint16x8_t x, uint16_t y); // expected-note {{candidate function}}
24+
__attribute__((overloadable))
25+
int overload(uint32x4_t x, uint32_t y); // expected-note {{candidate function}}
26+
27+
int16_t s16;
28+
int32_t s32;
29+
uint16_t u16;
30+
uint32_t u32;
31+
32+
int16x8_t vs16;
33+
int32x4_t vs32;
34+
uint16x8_t vu16;
35+
uint32x4_t vu32;
36+
37+
// ----------------------------------------------------------------------
38+
// Simple cases where the types are correctly matched
39+
40+
// CHECK-LABEL: @test_easy_s16(
41+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int16
42+
int test_easy_s16(void) { return overload(vs16, s16); }
43+
44+
// CHECK-LABEL: @test_easy_u16(
45+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint16
46+
int test_easy_u16(void) { return overload(vu16, u16); }
47+
48+
// CHECK-LABEL: @test_easy_s32(
49+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int32
50+
int test_easy_s32(void) { return overload(vs32, s32); }
51+
52+
// CHECK-LABEL: @test_easy_u32(
53+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint32
54+
int test_easy_u32(void) { return overload(vu32, u32); }
55+
56+
// ----------------------------------------------------------------------
57+
// Do arithmetic on the scalar, and it may get promoted. We still expect the
58+
// same overloads to be selected if that happens.
59+
60+
// CHECK-LABEL: @test_promote_s16(
61+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int16
62+
int test_promote_s16(void) { return overload(vs16, s16 + 1); }
63+
64+
// CHECK-LABEL: @test_promote_u16(
65+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint16
66+
int test_promote_u16(void) { return overload(vu16, u16 + 1); }
67+
68+
// CHECK-LABEL: @test_promote_s32(
69+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int32
70+
int test_promote_s32(void) { return overload(vs32, s32 + 1); }
71+
72+
// CHECK-LABEL: @test_promote_u32(
73+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint32
74+
int test_promote_u32(void) { return overload(vu32, u32 + 1); }
75+
76+
// ----------------------------------------------------------------------
77+
// Write a simple integer literal without qualification, and expect
78+
// the vector type to make it unambiguous which integer type you meant
79+
// the literal to be.
80+
81+
// CHECK-LABEL: @test_literal_s16(
82+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int16
83+
int test_literal_s16(void) { return overload(vs16, 1); }
84+
85+
// CHECK-LABEL: @test_literal_u16(
86+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint16
87+
int test_literal_u16(void) { return overload(vu16, 1); }
88+
89+
// CHECK-LABEL: @test_literal_s32(
90+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int32
91+
int test_literal_s32(void) { return overload(vs32, 1); }
92+
93+
// CHECK-LABEL: @test_literal_u32(
94+
// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint32
95+
int test_literal_u32(void) { return overload(vu32, 1); }
96+
97+
// ----------------------------------------------------------------------
98+
// All of those overload resolutions are supposed to be unambiguous even when
99+
// lax vector conversion is enabled. Check here that a lax conversion in a
100+
// different context still works.
101+
int16x8_t lax_conversion(void) { return vu32; }
102+
103+
// ----------------------------------------------------------------------
104+
// Use a vector type that there really _isn't_ any overload for, and
105+
// make sure that we get a fatal compile error.
106+
107+
#ifdef ERROR_CHECK
108+
int expect_error(uint64x2_t v) {
109+
return overload(v, 2); // expected-error {{no matching function for call to 'overload'}}
110+
}
111+
112+
typedef __attribute__((__clang_arm_mve_strict_polymorphism)) int i; // expected-error {{'__clang_arm_mve_strict_polymorphism' attribute can only be applied to an MVE/NEON vector type}}
113+
typedef __attribute__((__clang_arm_mve_strict_polymorphism)) int f(); // expected-error {{'__clang_arm_mve_strict_polymorphism' attribute can only be applied to an MVE/NEON vector type}}
114+
typedef __attribute__((__clang_arm_mve_strict_polymorphism)) struct { uint16x8_t v; } s; // expected-error {{'__clang_arm_mve_strict_polymorphism' attribute can only be applied to an MVE/NEON vector type}}
115+
#endif

‎clang/utils/TableGen/MveEmitter.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,8 +1454,9 @@ void MveEmitter::EmitHeader(raw_ostream &OS) {
14541454
raw_ostream &OS = parts[ST->requiresFloat() ? Float : 0];
14551455
const VectorType *VT = getVectorType(ST);
14561456

1457-
OS << "typedef __attribute__((neon_vector_type(" << VT->lanes() << "))) "
1458-
<< ST->cName() << " " << VT->cName() << ";\n";
1457+
OS << "typedef __attribute__((__neon_vector_type__(" << VT->lanes()
1458+
<< "), __clang_arm_mve_strict_polymorphism)) " << ST->cName() << " "
1459+
<< VT->cName() << ";\n";
14591460

14601461
// Every vector type also comes with a pair of multi-vector types for
14611462
// the VLD2 and VLD4 instructions.

0 commit comments

Comments
 (0)