[clang][AArch64] Correct return type of Neon vqmovun intrinsics

[DavidSpickett]

Summary

Neon intrinsics vqmovunh_s16, vqmovuns_s32, vqmovund_s64
should have unsigned return types.

See https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqmovun

Fixes https://bugs.llvm.org/show_bug.cgi?id=46840

[efriedma]

Would it be worth looking into improving our general test coverage here? The fact that we don't have any test that verifies the actual signatures of the NEON intrinsics (which could be used to compare against other compilers) seems bad.

[DavidSpickett]

I was surprised too. Perhaps there is a way to write the test file such that any type mismatch is a warning and compile with -Werror (maybe a new file, the IR might get messy). I will spend some time looking into this.

[efriedma]

Probably the simplest way to verify return types would be using decltype. (static_assert(std::is_same_v<uint32_t, decltype(vqmovund_s64(0))>);.)

For arguments, maybe more complicated. I guess you could write a C++ class with conversion operators for every integer type, mark all of them except one "= delete", and pass it as an argument.

[DavidSpickett]

Thanks for the pointer, I found more mistakes this way, e.g.: https://godbolt.org/z/f6n95z
(I don't think I can use <type_traits> in a test but there is clang/test/Headers/x86-64-apple-macosx-types.cpp which uses is_same with it's own implementation so I can use/copy that)

There are ~5 more with incorrect return types so I'll add those to this patch as well as the return type check.
The arguments check is complicated by some intrinsics being macros but we can check the majority, I'll do that in a separate patch.

[miyuki]

For intrinsics that are plain functions (not macros) you could just check the signatures using assignments to function pointers, e.g.:

#include <arm_neon.h>
uint16_t (*fp1)(int32_t) = &vqmovuns_s32;

$ clang -target=aarch64-arm-none-eabi -march=armv8-a+simd -c neon.c -fsyntax-only -Werror=incompatible-function-pointer-types
neon.c:2:12: error: incompatible function pointer types initializing 'uint16_t (*)(int32_t)' (aka 'unsigned short (*)(int)') with an expression of type 'int16_t (*)(int32_t)' (aka 'short (*)(int)') [-Werror,-Wincompatible-function-pointer-types]
uint16_t (*fp1)(int32_t) = &vqmovuns_s32;
           ^               ~~~~~~~~~~~~~
1 error generated.

[DavidSpickett]

Updated to fix all the vqmovun that have incorrect types.

I'm working on a test specifically for intrinic types, using the function pointer idea (which is how I confirmed these). That will be a separate change.

[efriedma]

LGTM

[DavidSpickett]

Rebased

clang/include/clang/Basic/arm_neon.td

 def RBIT : IInst<"vrbit", "..", "cUcPcQcQUcQPc">;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Integer extract and narrow to high
 def XTN2 : SOpInst<"vmovn_high", "(<Q)<Q", "silUsUiUl", OP_XTN>;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Signed integer saturating extract and unsigned narrow to high
-def SQXTUN2 : SOpInst<"vqmovun_high", "(<U)(<q).", "HsHiHl", OP_SQXTUN>;
+def SQXTUN2 : SOpInst<"vqmovun_high", "(<U)(<Uq).", "HsHiHl", OP_SQXTUN>;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Integer saturating extract and narrow to high
 def QXTN2 : SOpInst<"vqmovn_high", "(<Q)<Q", "silUsUiUl", OP_QXTN>;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Converting vectors
 

 def SCALAR_SQDMLSL : SInst<"vqdmlsl", "(1>)(1>)11", "SsSi">;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Signed Saturating Doubling Multiply Long
 def SCALAR_SQDMULL : SInst<"vqdmull", "(1>)11", "SsSi">;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Scalar Signed Saturating Extract Unsigned Narrow
-def SCALAR_SQXTUN : SInst<"vqmovun", "(1<)1", "SsSiSl">;
+def SCALAR_SQXTUN : SInst<"vqmovun", "(U1<)1", "SsSiSl">;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Scalar Signed Saturating Extract Narrow
 def SCALAR_SQXTN : SInst<"vqmovn", "(1<)1", "SsSiSl">;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Scalar Unsigned Saturating Extract Narrow
 def SCALAR_UQXTN : SInst<"vqmovn", "(1<)1", "SUsSUiSUl">;

clang/test/CodeGen/aarch64-neon-intrinsics.c

int64_t test_vqdmulls_s32(int32_t a, int32_t b) {
   return (int64_t)vqdmulls_s32(a, b);
 }
 
 // CHECK-LABEL: @test_vqmovunh_s16(
 // CHECK:   [[TMP0:%.*]] = insertelement <8 x i16> undef, i16 %a, i64 0
 // CHECK:   [[VQMOVUNH_S16_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.sqxtun.v8i8(<8 x i16> [[TMP0]])
 // CHECK:   [[TMP1:%.*]] = extractelement <8 x i8> [[VQMOVUNH_S16_I]], i64 0
 // CHECK:   ret i8 [[TMP1]]
-int8_t test_vqmovunh_s16(int16_t a) {
-  return (int8_t)vqmovunh_s16(a);
+uint8_t test_vqmovunh_s16(int16_t a) {
+  return (uint8_t)vqmovunh_s16(a);
 }
 
 // CHECK-LABEL: @test_vqmovuns_s32(
 // CHECK:   [[TMP0:%.*]] = insertelement <4 x i32> undef, i32 %a, i64 0
 // CHECK:   [[VQMOVUNS_S32_I:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqxtun.v4i16(<4 x i32> [[TMP0]])
 // CHECK:   [[TMP1:%.*]] = extractelement <4 x i16> [[VQMOVUNS_S32_I]], i64 0
 // CHECK:   ret i16 [[TMP1]]
-int16_t test_vqmovuns_s32(int32_t a) {
-  return (int16_t)vqmovuns_s32(a);
+uint16_t test_vqmovuns_s32(int32_t a) {
+  return (uint16_t)vqmovuns_s32(a);
 }
 
 // CHECK-LABEL: @test_vqmovund_s64(
 // CHECK:   [[VQMOVUND_S64_I:%.*]] = call i32 @llvm.aarch64.neon.scalar.sqxtun.i32.i64(i64 %a)
 // CHECK:   ret i32 [[VQMOVUND_S64_I]]
-int32_t test_vqmovund_s64(int64_t a) {
-  return (int32_t)vqmovund_s64(a);
+uint32_t test_vqmovund_s64(int64_t a) {
+  return (uint32_t)vqmovund_s64(a);
 }
 
 // CHECK-LABEL: @test_vqmovnh_s16(
 // CHECK:   [[TMP0:%.*]] = insertelement <8 x i16> undef, i16 %a, i64 0
 // CHECK:   [[VQMOVNH_S16_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.sqxtn.v8i8(<8 x i16> [[TMP0]])
 // CHECK:   [[TMP1:%.*]] = extractelement <8 x i8> [[VQMOVNH_S16_I]], i64 0
 // CHECK:   ret i8 [[TMP1]]
 int8_t test_vqmovnh_s16(int16_t a) {

clang/test/CodeGen/aarch64-neon-misc.c

int32x2_t test_vqmovun_s64(int64x2_t a) {
   return vqmovun_s64(a);
 }
 
 // CHECK-LABEL: @test_vqmovun_high_s16(
 // CHECK:   [[TMP0:%.*]] = bitcast <8 x i16> %b to <16 x i8>
 // CHECK:   [[VQMOVUN_V1_I_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.sqxtun.v8i8(<8 x i16> %b)
 // CHECK:   [[SHUFFLE_I_I:%.*]] = shufflevector <8 x i8> %a, <8 x i8> [[VQMOVUN_V1_I_I]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15>
 // CHECK:   ret <16 x i8> [[SHUFFLE_I_I]]
-int8x16_t test_vqmovun_high_s16(int8x8_t a, int16x8_t b) {
+uint8x16_t test_vqmovun_high_s16(uint8x8_t a, int16x8_t b) {
   return vqmovun_high_s16(a, b);
 }
 
 // CHECK-LABEL: @test_vqmovun_high_s32(
 // CHECK:   [[TMP0:%.*]] = bitcast <4 x i32> %b to <16 x i8>
 // CHECK:   [[VQMOVUN_V1_I_I:%.*]] = call <4 x i16> @llvm.aarch64.neon.sqxtun.v4i16(<4 x i32> %b)
 // CHECK:   [[VQMOVUN_V2_I_I:%.*]] = bitcast <4 x i16> [[VQMOVUN_V1_I_I]] to <8 x i8>
 // CHECK:   [[SHUFFLE_I_I:%.*]] = shufflevector <4 x i16> %a, <4 x i16> [[VQMOVUN_V1_I_I]], <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
 // CHECK:   ret <8 x i16> [[SHUFFLE_I_I]]
-int16x8_t test_vqmovun_high_s32(int16x4_t a, int32x4_t b) {
+uint16x8_t test_vqmovun_high_s32(uint16x4_t a, int32x4_t b) {
   return vqmovun_high_s32(a, b);
 }
 
 // CHECK-LABEL: @test_vqmovun_high_s64(
 // CHECK:   [[TMP0:%.*]] = bitcast <2 x i64> %b to <16 x i8>
 // CHECK:   [[VQMOVUN_V1_I_I:%.*]] = call <2 x i32> @llvm.aarch64.neon.sqxtun.v2i32(<2 x i64> %b)
 // CHECK:   [[VQMOVUN_V2_I_I:%.*]] = bitcast <2 x i32> [[VQMOVUN_V1_I_I]] to <8 x i8>
 // CHECK:   [[SHUFFLE_I_I:%.*]] = shufflevector <2 x i32> %a, <2 x i32> [[VQMOVUN_V1_I_I]], <4 x i32> <i32 0, i32 1, i32 2, i32 3>
 // CHECK:   ret <4 x i32> [[SHUFFLE_I_I]]
-int32x4_t test_vqmovun_high_s64(int32x2_t a, int64x2_t b) {
+uint32x4_t test_vqmovun_high_s64(uint32x2_t a, int64x2_t b) {
   return vqmovun_high_s64(a, b);
 }
 
 // CHECK-LABEL: @test_vqmovn_s16(
 // CHECK:   [[TMP0:%.*]] = bitcast <8 x i16> %a to <16 x i8>
 // CHECK:   [[VQMOVN_V1_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.sqxtn.v8i8(<8 x i16> %a)
 // CHECK:   ret <8 x i8> [[VQMOVN_V1_I]]
 int8x8_t test_vqmovn_s16(int16x8_t a) {

clang/test/Sema/arm64-neon-header.c

 // RUN: %clang_cc1 -triple arm64-apple-darwin -target-feature +neon -Wvector-conversion -fsyntax-only -ffreestanding -verify %s
 
 #include <arm_neon.h>
 
-int16x8_t foo(int8x8_t p0, int16x8_t p1) {
+int16x8_t foo(uint8x8_t p0, int16x8_t p1) {
   return vqmovun_high_s16(p0, p1); // expected-warning {{incompatible vector types returning 'uint8x16_t'}}
 }