[clang] ada01d1 - [clang] New __attribute__((__clang_arm_mve_strict_polymorphism)).

Simon Tatham via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 15 07:05:17 PST 2020


Author: Simon Tatham
Date: 2020-01-15T15:04:10Z
New Revision: ada01d1b869763f7d5d3438dcfce02066b06ab0a

URL: https://github.com/llvm/llvm-project/commit/ada01d1b869763f7d5d3438dcfce02066b06ab0a
DIFF: https://github.com/llvm/llvm-project/commit/ada01d1b869763f7d5d3438dcfce02066b06ab0a.diff

LOG: [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

Added: 
    clang/test/Sema/overload-arm-mve.c

Modified: 
    clang/include/clang/Basic/Attr.td
    clang/include/clang/Basic/AttrDocs.td
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/AST/TypePrinter.cpp
    clang/lib/Sema/SemaOverload.cpp
    clang/lib/Sema/SemaType.cpp
    clang/utils/TableGen/MveEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 16556b5f0745..10db2a868dce 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1479,6 +1479,11 @@ def NeonVectorType : TypeAttr {
   let ASTNode = 0;
 }
 
+def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr<TargetARM> {
+  let Spellings = [Clang<"__clang_arm_mve_strict_polymorphism">];
+  let Documentation = [ArmMveStrictPolymorphismDocs];
+}
+
 def NoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;

diff  --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 03d36ae7ab32..456edd1daafc 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4789,3 +4789,45 @@ close the handle. It is also assumed to require an open handle to work with.
   zx_status_t zx_handle_close(zx_handle_t handle [[clang::release_handle]]);
   }];
 }
+
+def ArmMveStrictPolymorphismDocs : Documentation {
+    let Category = DocCatType;
+    let Content = [{
+This attribute is used in the implementation of the ACLE intrinsics for the Arm
+MVE instruction set. It is used to define the vector types used by the MVE
+intrinsics.
+
+Its effect is to modify the behavior of a vector type with respect to function
+overloading. If a candidate function for overload resolution has a parameter
+type with this attribute, then the selection of that candidate function will be
+disallowed if the actual argument can only be converted via a lax vector
+conversion. The aim is to prevent spurious ambiguity in ARM MVE polymorphic
+intrinsics.
+
+.. code-block:: c++
+
+  void overloaded(uint16x8_t vector, uint16_t scalar);
+  void overloaded(int32x4_t vector, int32_t scalar);
+  uint16x8_t myVector;
+  uint16_t myScalar;
+
+  // myScalar is promoted to int32_t as a side effect of the addition,
+  // so if lax vector conversions are considered for myVector, then
+  // the two overloads are equally good (one argument conversion
+  // each). But if the vector has the __clang_arm_mve_strict_polymorphism
+  // attribute, only the uint16x8_t,uint16_t overload will match.
+  overloaded(myVector, myScalar + 1);
+
+However, this attribute does not prohibit lax vector conversions in contexts
+other than overloading.
+
+.. code-block:: c++
+
+  uint16x8_t function();
+
+  // This is still permitted with lax vector conversion enabled, even
+  // if the vector types have __clang_arm_mve_strict_polymorphism
+  int32x4_t result = function();
+
+    }];
+}

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7d8231d140e4..ffa326932a1c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6593,6 +6593,8 @@ def note_objc_unsafe_perform_selector_method_declared_here :  Note<
   "method %0 that returns %1 declared here">;
 def err_attribute_arm_mve_alias : Error<
   "'__clang_arm_mve_alias' attribute can only be applied to an ARM MVE builtin">;
+def err_attribute_arm_mve_polymorphism : Error<
+  "'__clang_arm_mve_strict_polymorphism' attribute can only be applied to an MVE/NEON vector type">;
 
 def warn_setter_getter_impl_required : Warning<
   "property %0 requires method %1 to be defined - "

diff  --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index c2f4baec989e..3a00a6c11ddb 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -1558,6 +1558,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
   case attr::AcquireHandle:
     OS << "acquire_handle";
     break;
+  case attr::ArmMveStrictPolymorphism:
+    OS << "__clang_arm_mve_strict_polymorphism";
+    break;
   }
   OS << "))";
 }

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0fd932fac970..8bb4c5312c17 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1653,9 +1653,13 @@ static bool IsVectorConversion(Sema &S, QualType FromType,
   // 1)vector types are equivalent AltiVec and GCC vector types
   // 2)lax vector conversions are permitted and the vector types are of the
   //   same size
+  // 3)the destination type does not have the ARM MVE strict-polymorphism
+  //   attribute, which inhibits lax vector conversion for overload resolution
+  //   only
   if (ToType->isVectorType() && FromType->isVectorType()) {
     if (S.Context.areCompatibleVectorTypes(FromType, ToType) ||
-        S.isLaxVectorConversion(FromType, ToType)) {
+        (S.isLaxVectorConversion(FromType, ToType) &&
+         !ToType->hasAttr(attr::ArmMveStrictPolymorphism))) {
       ICK = ICK_Vector_Conversion;
       return true;
     }

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 3884fdae8fe7..a860640444e3 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7360,6 +7360,23 @@ static void HandleNeonVectorTypeAttr(QualType &CurType, const ParsedAttr &Attr,
   CurType = S.Context.getVectorType(CurType, numElts, VecKind);
 }
 
+static void HandleArmMveStrictPolymorphismAttr(TypeProcessingState &State,
+                                               QualType &CurType,
+                                               ParsedAttr &Attr) {
+  const VectorType *VT = dyn_cast<VectorType>(CurType);
+  if (!VT || VT->getVectorKind() != VectorType::NeonVector) {
+    State.getSema().Diag(Attr.getLoc(),
+                         diag::err_attribute_arm_mve_polymorphism);
+    Attr.setInvalid();
+    return;
+  }
+
+  CurType =
+      State.getAttributedType(createSimpleAttr<ArmMveStrictPolymorphismAttr>(
+                                  State.getSema().Context, Attr),
+                              CurType, CurType);
+}
+
 /// Handle OpenCL Access Qualifier Attribute.
 static void HandleOpenCLAccessAttr(QualType &CurType, const ParsedAttr &Attr,
                                    Sema &S) {
@@ -7544,6 +7561,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
                                VectorType::NeonPolyVector);
       attr.setUsedAsTypeAttr();
       break;
+    case ParsedAttr::AT_ArmMveStrictPolymorphism: {
+      HandleArmMveStrictPolymorphismAttr(state, type, attr);
+      attr.setUsedAsTypeAttr();
+      break;
+    }
     case ParsedAttr::AT_OpenCLAccess:
       HandleOpenCLAccessAttr(type, attr, state.getSema());
       attr.setUsedAsTypeAttr();

diff  --git a/clang/test/Sema/overload-arm-mve.c b/clang/test/Sema/overload-arm-mve.c
new file mode 100644
index 000000000000..0ce2dac0aded
--- /dev/null
+++ b/clang/test/Sema/overload-arm-mve.c
@@ -0,0 +1,115 @@
+// 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
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature +mve.fp -flax-vector-conversions=all -verify -fsyntax-only -DERROR_CHECK %s
+
+typedef   signed short      int16_t;
+typedef   signed int        int32_t;
+typedef   signed long long  int64_t;
+typedef unsigned short     uint16_t;
+typedef unsigned int       uint32_t;
+typedef unsigned long long uint64_t;
+
+typedef __attribute__((neon_vector_type(8), __clang_arm_mve_strict_polymorphism))  int16_t  int16x8_t;
+typedef __attribute__((neon_vector_type(4), __clang_arm_mve_strict_polymorphism))  int32_t  int32x4_t;
+typedef __attribute__((neon_vector_type(2), __clang_arm_mve_strict_polymorphism))  int64_t  int64x2_t;
+typedef __attribute__((neon_vector_type(8), __clang_arm_mve_strict_polymorphism)) uint16_t uint16x8_t;
+typedef __attribute__((neon_vector_type(4), __clang_arm_mve_strict_polymorphism)) uint32_t uint32x4_t;
+typedef __attribute__((neon_vector_type(2), __clang_arm_mve_strict_polymorphism)) uint64_t uint64x2_t;
+
+__attribute__((overloadable))
+int overload(int16x8_t x, int16_t y); // expected-note {{candidate function}}
+__attribute__((overloadable))
+int overload(int32x4_t x, int32_t y); // expected-note {{candidate function}}
+__attribute__((overloadable))
+int overload(uint16x8_t x, uint16_t y); // expected-note {{candidate function}}
+__attribute__((overloadable))
+int overload(uint32x4_t x, uint32_t y); // expected-note {{candidate function}}
+
+int16_t s16;
+int32_t s32;
+uint16_t u16;
+uint32_t u32;
+
+int16x8_t vs16;
+int32x4_t vs32;
+uint16x8_t vu16;
+uint32x4_t vu32;
+
+// ----------------------------------------------------------------------
+// Simple cases where the types are correctly matched
+
+// CHECK-LABEL: @test_easy_s16(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int16
+int test_easy_s16(void) { return overload(vs16, s16); }
+
+// CHECK-LABEL: @test_easy_u16(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint16
+int test_easy_u16(void) { return overload(vu16, u16); }
+
+// CHECK-LABEL: @test_easy_s32(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int32
+int test_easy_s32(void) { return overload(vs32, s32); }
+
+// CHECK-LABEL: @test_easy_u32(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint32
+int test_easy_u32(void) { return overload(vu32, u32); }
+
+// ----------------------------------------------------------------------
+// Do arithmetic on the scalar, and it may get promoted. We still expect the
+// same overloads to be selected if that happens.
+
+// CHECK-LABEL: @test_promote_s16(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int16
+int test_promote_s16(void) { return overload(vs16, s16 + 1); }
+
+// CHECK-LABEL: @test_promote_u16(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint16
+int test_promote_u16(void) { return overload(vu16, u16 + 1); }
+
+// CHECK-LABEL: @test_promote_s32(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int32
+int test_promote_s32(void) { return overload(vs32, s32 + 1); }
+
+// CHECK-LABEL: @test_promote_u32(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint32
+int test_promote_u32(void) { return overload(vu32, u32 + 1); }
+
+// ----------------------------------------------------------------------
+// Write a simple integer literal without qualification, and expect
+// the vector type to make it unambiguous which integer type you meant
+// the literal to be.
+
+// CHECK-LABEL: @test_literal_s16(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int16
+int test_literal_s16(void) { return overload(vs16, 1); }
+
+// CHECK-LABEL: @test_literal_u16(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint16
+int test_literal_u16(void) { return overload(vu16, 1); }
+
+// CHECK-LABEL: @test_literal_s32(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_int32
+int test_literal_s32(void) { return overload(vs32, 1); }
+
+// CHECK-LABEL: @test_literal_u32(
+// CHECK: call i32 @_Z8overload{{[a-zA-Z0-9_]+}}_uint32
+int test_literal_u32(void) { return overload(vu32, 1); }
+
+// ----------------------------------------------------------------------
+// All of those overload resolutions are supposed to be unambiguous even when
+// lax vector conversion is enabled. Check here that a lax conversion in a
+// 
diff erent context still works.
+int16x8_t lax_conversion(void) { return vu32; }
+
+// ----------------------------------------------------------------------
+// Use a vector type that there really _isn't_ any overload for, and
+// make sure that we get a fatal compile error.
+
+#ifdef ERROR_CHECK
+int expect_error(uint64x2_t v) {
+  return overload(v, 2); // expected-error {{no matching function for call to 'overload'}}
+}
+
+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}}
+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}}
+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}}
+#endif

diff  --git a/clang/utils/TableGen/MveEmitter.cpp b/clang/utils/TableGen/MveEmitter.cpp
index 431e5c477c2b..c39e41ac6e5b 100644
--- a/clang/utils/TableGen/MveEmitter.cpp
+++ b/clang/utils/TableGen/MveEmitter.cpp
@@ -1454,8 +1454,9 @@ void MveEmitter::EmitHeader(raw_ostream &OS) {
     raw_ostream &OS = parts[ST->requiresFloat() ? Float : 0];
     const VectorType *VT = getVectorType(ST);
 
-    OS << "typedef __attribute__((neon_vector_type(" << VT->lanes() << "))) "
-       << ST->cName() << " " << VT->cName() << ";\n";
+    OS << "typedef __attribute__((__neon_vector_type__(" << VT->lanes()
+       << "), __clang_arm_mve_strict_polymorphism)) " << ST->cName() << " "
+       << VT->cName() << ";\n";
 
     // Every vector type also comes with a pair of multi-vector types for
     // the VLD2 and VLD4 instructions.


        


More information about the cfe-commits mailing list