[AArch64] vqmovun* return types should be unsigned

[llvmbot]

Bugzilla Link	46840
Resolution	FIXED
Resolved on	Aug 16, 2021 07:59
Version	11.0
OS	Windows NT
Reporter	LLVM Bugzilla Contributor
CC	@Arnaud-de-Grandmaison-ARM,@DavidSpickett,@efriedma-quic,@sdesmalen-arm,@smithp35

Extended Description

clang has signed return values for the vqmovun* functions, but they should be unsigned. See https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vqmovun

Trivial test case:

include

uint16_t foo(int32_t v);

uint16_t foo(int32_t v) { return vqmovuns_s32(v); }

On Compiler Explorer for convenience: https://godbolt.org/z/qYPP3j

[llvmbot]

Hm, you're right I can confirm that with the latest development version. Maybe I was looking at an old build… not sure, but regardless this is fixed, thank you!

[DavidSpickett]

Whether through my change or someone else's, the types are correct: $ grep vqmovun_high ./lib/clang/14.0.0/include/arm_neon.h ai uint16x8_t vqmovun_high_s32(uint16x4_t p0, int32x4_t p1) { ai uint16x8_t vqmovun_high_s32(uint16x4_t p0, int32x4_t p1) { ai uint32x4_t vqmovun_high_s64(uint32x2_t __p0, int64x2_t p1) { ai uint32x4_t vqmovun_high_s64(uint32x2_t p0, int64x2_t p1) { ai uint8x16_t vqmovun_high_s16(uint8x8_t p0, int16x8_t p1) { ai uint8x16_t vqmovun_high_s16(uint8x8_t __p0, int16x8_t p1) {

[efriedma-quic]

I'm not sure what you think the issue is?

I mean, it's a known issue that -flax-vector-conversions is on by default, but that's a different problem.

[llvmbot]

Sorry I missed this.

The patch helps, thanks, but the vqmovunhigh* functions are still wrong; the first argument should be unsigned. Arm's documentation (where they're unigned): https://developer.arm.com/architectures/instruction-sets/intrinsics/

Also, GCC's implementation: https://raw.githubusercontent.com/gcc-mirror/gcc/master/gcc/config/aarch64/arm_neon.h

[DavidSpickett]

Fixed by 349af8054218017a2ac0c4bfeddd63e6ccbf4a21.

[DavidSpickett]

Sorry for the delay. While testing this I found a bunch of other mistakes that I intend to fix in a larger patch soon. (hopefully with extra testing for types)

[DavidSpickett]

Posted https://reviews.llvm.org/D85118

[DavidSpickett]

I think it's just one set that are incorrect: ai int8_t vqmovunh_s16(int16_t p0) { ai int16_t vqmovuns_s32(int32_t p0) { __ai int32_t vqmovund_s64(int64_t __p0) {

Rest look good to me: ai uint8x8_t vqmovun_s16(int16x8_t p0) { ai uint16x4_t vqmovun_s32(int32x4_t p0) { ai uint32x2_t vqmovun_s64(int64x2_t p0) { ai uint8x16_t vqmovun_high_s16(int8x8_t __p0, int16x8_t p1) { ai uint16x8_t vqmovun_high_s32(int16x4_t p0, int32x4_t p1) { ai uint32x4_t vqmovun_high_s64(int32x2_t __p0, int64x2_t __p1)

(tell me if I'm missing something, I don't deal with intrinsics very often)

Should be a simple fix I'll have something posted shortly.