[ARM] Fix AArch32 and pre-v8 poly types to be unsigned

[bsmith]

Summary

All poly types should be of unsigned type, this is true for all ARM ISAs, AArch64 currently has this behavior however AArch32 and pre-v8 do not.

Also update test/CodeGenCXX/mangle-neon-vectors.cpp to test as thoroughly as the AArch64 version.

[t.p.northover]

Hi Bradley,

[bsmith]

Hi Bradley,

Hi Tim,

On 2 October 2013 16:09, Bradley Smith <[e-mail]> wrote:

All poly types should be of unsigned type,

I proposed this back in January, and I think we reached the conclusion
that this change should be coordinated with GCC since it's potentially
ABI-breaking.

Do you know if there's been any progress on the GCC side? I think they
(Richard Earnshaw & Tejas) were intending to wait until 4.8.0 had been
released before proposing the change there.

Unfortunately not much it seems, it's still very much on their TODO list
for 4.8 so will happen at some point, we're just not sure exactly when.
Given this is now specified in the ACLE as unsigned so has to change in GCC,
it might be nice for clang to take the lead on this one rather than waiting
for GCC.

Also as you stated in your previous thread, which I just found, it's
extraordinarily unlikely that many people if anybody will actually be
affected
by this change differing across clang and GCC until GCC changes it.

Regards,
Bradley Smith

[bsmith]

Hi Bob,

Since I believe it was you who objected somewhat to this last time around,
do you have any opinion on this? Thanks.

Regards,
Bradley Smith

On 2 October 2013 16:09, Bradley Smith <[e-mail]> wrote:

All poly types should be of unsigned type,

I proposed this back in January, and I think we reached the

conclusion

that this change should be coordinated with GCC since it's

potentially

ABI-breaking.

Do you know if there's been any progress on the GCC side? I think

they

(Richard Earnshaw & Tejas) were intending to wait until 4.8.0 had

been

released before proposing the change there.

Unfortunately not much it seems, it's still very much on their TODO
list
for 4.8 so will happen at some point, we're just not sure exactly when.
Given this is now specified in the ACLE as unsigned so has to change in
GCC,
it might be nice for clang to take the lead on this one rather than
waiting
for GCC.

Also as you stated in your previous thread, which I just found, it's
extraordinarily unlikely that many people if anybody will actually be
affected
by this change differing across clang and GCC until GCC changes it.

[bsmith]

This change is very old and almost certainly out of date, therefore abandoning.

lib/AST/ItaniumMangle.cpp

 // if they are structs (to match ARM's initial implementation).  The
 // vector type must be one of the special types predefined by ARM.
 void CXXNameMangler::mangleNeonVectorType(const VectorType *T) {
   QualType EltType = T->getElementType();
   assert(EltType->isBuiltinType() && "Neon vector element not a BuiltinType");
   const char *EltName = 0;
   if (T->getVectorKind() == VectorType::NeonPolyVector) {
     switch (cast<BuiltinType>(EltType)->getKind()) {
-    case BuiltinType::SChar:     EltName = "poly8_t"; break;
-    case BuiltinType::Short:     EltName = "poly16_t"; break;
+    case BuiltinType::UChar:     EltName = "poly8_t"; break;
+    case BuiltinType::UShort:    EltName = "poly16_t"; break;
     default: llvm_unreachable("unexpected Neon polynomial vector element type");
     }
   } else {
     switch (cast<BuiltinType>(EltType)->getKind()) {
     case BuiltinType::SChar:     EltName = "int8_t"; break;
     case BuiltinType::UChar:     EltName = "uint8_t"; break;
     case BuiltinType::Short:     EltName = "int16_t"; break;
     case BuiltinType::UShort:    EltName = "uint16_t"; break;

lib/Sema/SemaType.cpp

 static bool isPermittedNeonBaseType(QualType &Ty,
                                     VectorType::VectorKind VecKind,
                                     bool IsAArch64) {
   const BuiltinType *BTy = Ty->getAs<BuiltinType>();
   if (!BTy)
     return false;
 
   if (VecKind == VectorType::NeonPolyVector) {
-    if (IsAArch64) {
-      // AArch64 polynomial vectors are unsigned
-      return BTy->getKind() == BuiltinType::UChar ||
-             BTy->getKind() == BuiltinType::UShort;
-    } else {
-      // AArch32 polynomial vector are signed.
-      return BTy->getKind() == BuiltinType::SChar ||
-             BTy->getKind() == BuiltinType::Short;
-    }
   }
 
   // Non-polynomial vector types: the usual suspects are allowed, as well as
   // float64_t on AArch64.
   if (IsAArch64 && BTy->getKind() == BuiltinType::Double)
     return true;
 
   return BTy->getKind() == BuiltinType::SChar ||

test/CodeGenCXX/mangle-neon-vectors.cpp

 // RUN: %clang_cc1 -triple arm-none-linux-gnueabi -target-feature +neon %s -emit-llvm -o - | FileCheck %s
 
 typedef float float32_t;
 typedef __fp16 float16_t;
-typedef signed char poly8_t;
-typedef short poly16_t;
+typedef unsigned char poly8_t;
+typedef unsigned short poly16_t;
+typedef unsigned char uint8_t;
+typedef unsigned short uint16_t;
+typedef signed char int8_t;
+typedef signed short int16_t;
+typedef signed long long int64_t;
 typedef unsigned long long uint64_t;
 
+typedef __attribute__((neon_vector_type(8))) int8_t int8x8_t;
+typedef __attribute__((neon_vector_type(16))) int8_t int8x16_t;
+typedef __attribute__((neon_vector_type(4))) int16_t int16x4_t;
+typedef __attribute__((neon_vector_type(8))) int16_t int16x8_t;
 typedef __attribute__((neon_vector_type(2))) int int32x2_t;
 typedef __attribute__((neon_vector_type(4))) int int32x4_t;
 typedef __attribute__((neon_vector_type(1))) uint64_t uint64x1_t;
+typedef __attribute__((neon_vector_type(2))) int64_t int64x2_t;
+typedef __attribute__((neon_vector_type(8))) uint8_t uint8x8_t;
+typedef __attribute__((neon_vector_type(16))) uint8_t uint8x16_t;
+typedef __attribute__((neon_vector_type(4))) uint16_t uint16x4_t;
+typedef __attribute__((neon_vector_type(8))) uint16_t uint16x8_t;
+typedef __attribute__((neon_vector_type(2))) unsigned int uint32x2_t;
+typedef __attribute__((neon_vector_type(4))) unsigned int uint32x4_t;
 typedef __attribute__((neon_vector_type(2))) uint64_t uint64x2_t;
 typedef __attribute__((neon_vector_type(2))) float32_t float32x2_t;
 typedef __attribute__((neon_vector_type(4))) float32_t float32x4_t;
 typedef __attribute__((neon_vector_type(4))) float16_t float16x4_t;
 typedef __attribute__((neon_vector_type(8))) float16_t float16x8_t;
+typedef __attribute__((neon_polyvector_type(8))) poly8_t poly8x8_t;
 typedef __attribute__((neon_polyvector_type(16))) poly8_t poly8x16_t;
+typedef __attribute__((neon_polyvector_type(4))) poly16_t poly16x4_t;
 typedef __attribute__((neon_polyvector_type(8))) poly16_t poly16x8_t;
 
+// CHECK: 15__simd64_int8_t
+void f1(int8x8_t) {}
+// CHECK: 16__simd64_int16_t
+void f2(int16x4_t) {}
 // CHECK: 16__simd64_int32_t
-void f1(int32x2_t v) { }
-// CHECK: 17__simd128_int32_t
-void f2(int32x4_t v) { }
-// CHECK: 17__simd64_uint64_t
-void f3(uint64x1_t v) { }
-// CHECK: 18__simd128_uint64_t
-void f4(uint64x2_t v) { }
-// CHECK: 18__simd64_float32_t
-void f5(float32x2_t v) { }
-// CHECK: 19__simd128_float32_t
-void f6(float32x4_t v) { }
+void f3(int32x2_t) {}
+// CHECK: 16__simd64_uint8_t
+void f4(uint8x8_t) {}
+// CHECK: 17__simd64_uint16_t
+void f5(uint16x4_t) {}
 // CHECK: 18__simd64_float16_t
-void f7(float16x4_t v) {}
+void f6(float16x4_t) {}
 // CHECK: 19__simd128_float16_t
-void f8(float16x8_t v) {}
+void f7(float16x8_t) {}
+// CHECK: 17__simd64_uint32_t
+void f8(uint32x2_t) {}
+// CHECK: 18__simd64_float32_t
+void f9(float32x2_t) {}
+// CHECK: 19__simd128_float32_t
+void f10(float32x4_t) {}
+// CHECK: 16__simd64_poly8_t
+void f11(poly8x8_t) {}
+// CHECK: 17__simd64_poly16_t
+void f12(poly16x4_t) {}
 // CHECK: 17__simd128_poly8_t
-void f9(poly8x16_t v) {}
+void f13(poly8x16_t) {}
 // CHECK: 18__simd128_poly16_t
-void f10(poly16x8_t v) {}
+void f14(poly16x8_t) {}
+// CHECK: 16__simd128_int8_t
+void f15(int8x16_t) {}
+// CHECK: 17__simd128_int16_t
+void f16(int16x8_t) {}
+// CHECK: 17__simd128_int32_t
+void f17(int32x4_t) {}
+// CHECK: 17__simd128_uint8_t
+void f18(uint8x16_t) {}
+// CHECK: 18__simd128_uint16_t
+void f19(uint16x8_t) {}
+// CHECK: 18__simd128_uint32_t
+void f20(uint32x4_t) {}
+// CHECK: 17__simd128_int64_t
+void f21(int64x2_t) {}
+// CHECK: 18__simd128_uint64_t
+void f22(uint64x2_t) {}

test/Sema/neon-vector-types.c

 // RUN: %clang_cc1 %s -triple armv7 -target-feature +neon -fsyntax-only -verify
 // RUN: %clang_cc1 %s -triple armv8 -target-feature +neon -fsyntax-only -verify
 
 typedef float float32_t;
-typedef signed char poly8_t;
-typedef short poly16_t;
+typedef unsigned char poly8_t;
+typedef unsigned short poly16_t;
 typedef unsigned long long uint64_t;
 
 // Define some valid Neon types.
 typedef __attribute__((neon_vector_type(2))) int int32x2_t;
 typedef __attribute__((neon_vector_type(4))) int int32x4_t;
 typedef __attribute__((neon_vector_type(1))) uint64_t uint64x1_t;
 typedef __attribute__((neon_vector_type(2))) uint64_t uint64x2_t;
 typedef __attribute__((neon_vector_type(2))) float32_t float32x2_t;

utils/TableGen/NeonEmitter.cpp

void NeonEmitter::run(raw_ostream &OS) {
   // Emit NEON-specific scalar typedefs.
   OS << "typedef float float32_t;\n";
   OS << "typedef __fp16 float16_t;\n";
 
   OS << "#ifdef __aarch64__\n";
   OS << "typedef double float64_t;\n";
   OS << "#endif\n\n";
 
-  // For now, signedness of polynomial types depends on target
-  OS << "#ifdef __aarch64__\n";
   OS << "typedef uint8_t poly8_t;\n";
   OS << "typedef uint16_t poly16_t;\n";
-  OS << "#else\n";
-  OS << "typedef int8_t poly8_t;\n";
-  OS << "typedef int16_t poly16_t;\n";
-  OS << "#endif\n";
 
   // Emit Neon vector typedefs.
   std::string TypedefTypes(
       "cQcsQsiQilQlUcQUcUsQUsUiQUiUlQUlhQhfQfQdPcQPcPsQPs");
   SmallVector<StringRef, 24> TDTypeVec;
   ParseTypes(0, TypedefTypes, TDTypeVec);
 
   // Emit vector typedefs.