[AArch64] Make ACLE intrinsics always available part1

[danielkiss]

Summary

A given arch feature might enabled by a pragma or a function attribute so in this cases would be nice to use intrinsics.
Today GCC offers the intrinsics without the march flag[1].
PR[2] for ACLE to clarify the intention and remove the need for -march flag for a given intrinsics.

This is going to be more useful when D127812 lands.

[1] https://godbolt.org/z/bxcMhav3z
[2] https://github.com/ARM-software/acle/pull/214

[samparker]

So, what kind of errors will the user get now if try to use an unsupported instruction? I'm assuming that it fails during isel and that doesn't sound like a good user experience.

[danielkiss]

In D133359#3776503, @samparker wrote:

So, what kind of errors will the user get now if try to use an unsupported instruction? I'm assuming that it fails during isel and that doesn't sound like a good user experience.

I'm adding target warnings. so the user will get "error: always_inline function 'foo' requires target feature 'x', but would be inlined into function 'bar' that is compiled without support for 'x'.

[danielkiss]

Armv8.3-A Javascript conversion and Armv8.5-A FP rounding don't have feature flag.
One option is to introduce feature flags or the the arch version could be used.
I'm looking comment for the arch version solution.

[dmgreen]

In D133359#3786469, @danielkiss wrote:

Armv8.3-A Javascript conversion and Armv8.5-A FP rounding don't have feature flag.
One option is to introduce feature flags or the the arch version could be used.
I'm looking comment for the arch version solution.

I'm looking into a similar patch for the parts of arm_neon.h that are not already handled, and was thinking along the same lines. The idea at current is to add support for arch= target attributes, as in https://gcc.gnu.org/onlinedocs/gcc/AArch64-Function-Attributes.html#AArch64-Function-Attributes. And then using "v8.1a" (rdma), "v8.3a" (complex) or "v8.5a" (frint) for the TARGET_BUILTIN, like you have here. I think that is fine for things internal to the compiler. The user can then use "arch=..." target attributes to specify different architecture versions. I'll try and put that part up for review.

What is you plan for AArch32? Keep it behind the same ifdef as before?

clang/lib/Basic/Targets/AArch64.cpp

529 ↗

(On Diff #459745)

I had a (completely untested) version of this function doing the same thing. I was basing it on string processing, though, so we didn't have to add new code for each new version of the architecture. I'm not sure if it was a good idea or not.

clang/test/Sema/aarch64-tme-errors.c

6 ↗

(On Diff #459745)

Does this get a "failure to inline" error now?

clang/test/Sema/builtins-arm64-mte.c

8 ↗

(On Diff #459745)

I think these should still be testing the ACLE intrinsics, not the builtins, if that is what we expect the users to use. Are the error's now different?

[dmgreen]

clang/lib/Basic/Targets/AArch64.cpp

529 ↗

(On Diff #459745)

It looked like this, by the way:

if (Enabled && Name == "v9.0a")                                              
  setFeatureEnabled(Features, "v8.5a", true);                                
else if (Enabled && Name != "v8.1a" &&                                       
         (Name.startswith("v8.") || Name.startswith("v9.")) &&               
         Name.endswith("a") &&                                               
         atoi(Name.substr(3, Name.size() - 1).data()) >= 1) {                
  SmallString<16> Storage;                                                   
  Twine NewName =                                                            
      Name.take_front(3) +                                                   
      std::to_string(atoi(Name.substr(3, Name.size() - 1).data()) - 1) + "a";
  setFeatureEnabled(Features, NewName.toStringRef(Storage), true);           
}

It is untested, apart from some very simple checks, doesn't handle errors and doesn't handle all the 9 feautres properly. It may be better not to be so recursive too if it can just enable multiple features at once.

[tschuett]

This looks like you want an enum over all versions with string2enum, enum2string, and comparison operations on the enum.

[compnerd]

clang/lib/Basic/Targets/AArch64.cpp

535–564 ↗

(On Diff #459745)

I think that we can compress some of the logic here. But I also wonder if we can write this in a different way: rather than creating a small vector and resizing it, can we actually avoid that all together and create a static constant data blob that we can iterate?

const char * const kv8Architectures[] = { "v8a", "v8.1a", "v8.2a", "v8.3a", "v8.4a", "v8.5a", "v8.6a", "v8.7a" };

unsigned major, minor;
std::tie(major, minor) = [parse_version](Name);
size_t v8Revisions = major == 8 ? minor : 4 + minor;
size_t v9Revisions = major == 9 : minor : 0;

The bit in the middle handwaves over the version parsing, but should hopefully convey the general idea.

[danielkiss]

Moved not ACLE related changes to D134349 and D134353.
This to be rebased on top of those and D134127.

clang/test/Sema/builtins-arm64-mte.c

8 ↗

(On Diff #459745)

These are custom errors see here e.g. that can't be triggered via the intrinsics anymore because we stop earlier.

[danielkiss]

Arm part will be a different patch since it is more complex to decide the support of a given feature. (e.g. ((CPUProfile != "M" && ArchVersion >= 6) || (CPUProfile == "M" && DSP))))

clang/lib/Basic/Targets/AArch64.cpp

535–564 ↗

(On Diff #459745)

solved in D134353.

clang/test/Sema/aarch64-tme-errors.c

6 ↗

(On Diff #459745)

Yes, but -verify does not go that far.

clang/test/Sema/builtins-arm64-mte.c

8 ↗

(On Diff #459745)

added test for intrinsics besides the builtins

[dmgreen]

Can you split the "mte" and "tme" parts away from the rest? I'm not sure if there are any problems with those part, but they are more involved and I am less familiar with it. It would be good to get a lot of the simpler parts in and out of the way first.

[danielkiss]

MTE/TME parts are removed, they will get their own patch.

[dmgreen]

Thanks. That's great. LGTM

clang/include/clang/Basic/BuiltinsAArch64.def

 // Memory Operations
 BUILTIN(__builtin_arm_mops_memset_tag, "v*v*iz", "")
 
 // Memory barrier
 BUILTIN(__builtin_arm_dmb, "vUi", "nc")
 BUILTIN(__builtin_arm_dsb, "vUi", "nc")
 BUILTIN(__builtin_arm_isb, "vUi", "nc")
 
-BUILTIN(__builtin_arm_jcvt, "Zid", "nc")
+TARGET_BUILTIN(__builtin_arm_jcvt, "Zid", "nc", "v8.3a")
 
 // Prefetch
 BUILTIN(__builtin_arm_prefetch, "vvC*UiUiUiUi", "nc")
 
 // System Registers
 BUILTIN(__builtin_arm_rsr, "UicC*", "nc")
 BUILTIN(__builtin_arm_rsr64, "WUicC*", "nc")
 BUILTIN(__builtin_arm_rsrp, "v*cC*", "nc")

 
 // Transactional Memory Extension
 BUILTIN(__builtin_arm_tstart, "WUi", "nj")
 BUILTIN(__builtin_arm_tcommit, "v", "n")
 BUILTIN(__builtin_arm_tcancel, "vWUIi", "n")
 BUILTIN(__builtin_arm_ttest, "WUi", "nc")
 
 // Armv8.5-A FP rounding intrinsics
-BUILTIN(__builtin_arm_rint32zf, "ff", "")
-BUILTIN(__builtin_arm_rint32z, "dd", "")
-BUILTIN(__builtin_arm_rint64zf, "ff", "")
-BUILTIN(__builtin_arm_rint64z, "dd", "")
-BUILTIN(__builtin_arm_rint32xf, "ff", "")
-BUILTIN(__builtin_arm_rint32x, "dd", "")
-BUILTIN(__builtin_arm_rint64xf, "ff", "")
-BUILTIN(__builtin_arm_rint64x, "dd", "")
+TARGET_BUILTIN(__builtin_arm_rint32zf, "ff", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint32z, "dd", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint64zf, "ff", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint64z, "dd", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint32xf, "ff", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint32x, "dd", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint64xf, "ff", "", "v8.5a")
+TARGET_BUILTIN(__builtin_arm_rint64x, "dd", "", "v8.5a")
 
 // Armv8.5-A Random number generation intrinsics
-BUILTIN(__builtin_arm_rndr,   "iWUi*", "n")
-BUILTIN(__builtin_arm_rndrrs, "iWUi*", "n")
+TARGET_BUILTIN(__builtin_arm_rndr,   "iWUi*", "n", "rand")
+TARGET_BUILTIN(__builtin_arm_rndrrs, "iWUi*", "n", "rand")
 
 // Armv8.7-A load/store 64-byte intrinsics
-BUILTIN(__builtin_arm_ld64b, "vvC*WUi*", "n")
-BUILTIN(__builtin_arm_st64b, "vv*WUiC*", "n")
-BUILTIN(__builtin_arm_st64bv, "WUiv*WUiC*", "n")
-BUILTIN(__builtin_arm_st64bv0, "WUiv*WUiC*", "n")
+TARGET_BUILTIN(__builtin_arm_ld64b, "vvC*WUi*", "n", "ls64")
+TARGET_BUILTIN(__builtin_arm_st64b, "vv*WUiC*", "n", "ls64")
+TARGET_BUILTIN(__builtin_arm_st64bv, "WUiv*WUiC*", "n", "ls64")
+TARGET_BUILTIN(__builtin_arm_st64bv0, "WUiv*WUiC*", "n", "ls64")
 
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
 TARGET_HEADER_BUILTIN(_InterlockedAdd,           "NiNiD*Ni",    "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64,         "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")

clang/lib/Headers/arm_acle.h

 }
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smusdx(int16x2_t __a, int16x2_t __b) {
   return __builtin_arm_smusdx(__a, __b);
 }
 #endif
 
 /* 9.7 CRC32 intrinsics */
-#if defined(__ARM_FEATURE_CRC32) && __ARM_FEATURE_CRC32
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+#if (defined(__ARM_FEATURE_CRC32) && __ARM_FEATURE_CRC32) ||                   \
+    (defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE)
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32b(uint32_t __a, uint8_t __b) {
   return __builtin_arm_crc32b(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32h(uint32_t __a, uint16_t __b) {
   return __builtin_arm_crc32h(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32w(uint32_t __a, uint32_t __b) {
   return __builtin_arm_crc32w(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32d(uint32_t __a, uint64_t __b) {
   return __builtin_arm_crc32d(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32cb(uint32_t __a, uint8_t __b) {
   return __builtin_arm_crc32cb(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32ch(uint32_t __a, uint16_t __b) {
   return __builtin_arm_crc32ch(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32cw(uint32_t __a, uint32_t __b) {
   return __builtin_arm_crc32cw(__a, __b);
 }
 
-static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__, target("crc")))
 __crc32cd(uint32_t __a, uint64_t __b) {
   return __builtin_arm_crc32cd(__a, __b);
 }
 #endif
 
 /* Armv8.3-A Javascript conversion intrinsic */
-#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE && defined(__ARM_FEATURE_JCVT)
-static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
+#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
+static __inline__ int32_t __attribute__((__always_inline__, __nodebug__, target("v8.3a")))
 __jcvt(double __a) {
   return __builtin_arm_jcvt(__a);
 }
 #endif
 
 /* Armv8.5-A FP rounding intrinsics */
-#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE && defined(__ARM_FEATURE_FRINT)
-static __inline__ float __attribute__((__always_inline__, __nodebug__))
+#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
+static __inline__ float __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint32zf(float __a) {
   return __builtin_arm_rint32zf(__a);
 }
 
-static __inline__ double __attribute__((__always_inline__, __nodebug__))
+static __inline__ double __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint32z(double __a) {
   return __builtin_arm_rint32z(__a);
 }
 
-static __inline__ float __attribute__((__always_inline__, __nodebug__))
+static __inline__ float __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint64zf(float __a) {
   return __builtin_arm_rint64zf(__a);
 }
 
-static __inline__ double __attribute__((__always_inline__, __nodebug__))
+static __inline__ double __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint64z(double __a) {
   return __builtin_arm_rint64z(__a);
 }
 
-static __inline__ float __attribute__((__always_inline__, __nodebug__))
+static __inline__ float __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint32xf(float __a) {
   return __builtin_arm_rint32xf(__a);
 }
 
-static __inline__ double __attribute__((__always_inline__, __nodebug__))
+static __inline__ double __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint32x(double __a) {
   return __builtin_arm_rint32x(__a);
 }
 
-static __inline__ float __attribute__((__always_inline__, __nodebug__))
+static __inline__ float __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint64xf(float __a) {
   return __builtin_arm_rint64xf(__a);
 }
 
-static __inline__ double __attribute__((__always_inline__, __nodebug__))
+static __inline__ double __attribute__((__always_inline__, __nodebug__, target("v8.5a")))
 __rint64x(double __a) {
   return __builtin_arm_rint64x(__a);
 }
 #endif
 
 /* Armv8.7-A load/store 64-byte intrinsics */
-#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE && defined(__ARM_FEATURE_LS64)
+#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
 typedef struct {
     uint64_t val[8];
 } data512_t;
 
-static __inline__ data512_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ data512_t __attribute__((__always_inline__, __nodebug__, target("ls64")))
 __arm_ld64b(const void *__addr) {
-    data512_t __value;
-    __builtin_arm_ld64b(__addr, __value.val);
-    return __value;
 }
-static __inline__ void __attribute__((__always_inline__, __nodebug__))
+static __inline__ void __attribute__((__always_inline__, __nodebug__, target("ls64")))
 __arm_st64b(void *__addr, data512_t __value) {
-    __builtin_arm_st64b(__addr, __value.val);
 }
-static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__, target("ls64")))
 __arm_st64bv(void *__addr, data512_t __value) {
-    return __builtin_arm_st64bv(__addr, __value.val);
 }
-static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
+static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__, target("ls64")))
 __arm_st64bv0(void *__addr, data512_t __value) {
-    return __builtin_arm_st64bv0(__addr, __value.val);
 }
 #endif
 
 /* 10.1 Special register intrinsics */
 #define __arm_rsr(sysreg) __builtin_arm_rsr(sysreg)
 #define __arm_rsr64(sysreg) __builtin_arm_rsr64(sysreg)
 #define __arm_rsrp(sysreg) __builtin_arm_rsrp(sysreg)
 #define __arm_rsrf(sysreg) __builtin_bit_cast(float, __arm_rsr(sysreg))

 #define __tstart()        __builtin_arm_tstart()
 #define __tcommit()       __builtin_arm_tcommit()
 #define __tcancel(__arg)  __builtin_arm_tcancel(__arg)
 #define __ttest()         __builtin_arm_ttest()
 
 #endif /* __ARM_FEATURE_TME */
 
 /* Armv8.5-A Random number generation intrinsics */
-#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE && defined(__ARM_FEATURE_RNG)
-static __inline__ int __attribute__((__always_inline__, __nodebug__))
+#if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
+static __inline__ int __attribute__((__always_inline__, __nodebug__, target("rand")))
 __rndr(uint64_t *__p) {
   return __builtin_arm_rndr(__p);
 }
-static __inline__ int __attribute__((__always_inline__, __nodebug__))
+static __inline__ int __attribute__((__always_inline__, __nodebug__, target("rand")))
 __rndrrs(uint64_t *__p) {
   return __builtin_arm_rndrrs(__p);
 }
 #endif
 
 #if defined(__cplusplus)
 }
 #endif
 
 #endif /* __ARM_ACLE_H */

clang/test/CodeGen/arm_acle.c

 // AArch64-NEXT:  entry:
 // AArch64-NEXT:    call void @llvm.aarch64.hint(i32 5)
 // AArch64-NEXT:    ret void
 //
 void test_sevl(void) {
   __sevl();
 }
 
-#if __ARM_32BIT_STATE
+#ifdef __ARM_32BIT_STATE
 // AArch32-LABEL: @test_dbg(
 // AArch32-NEXT:  entry:
 // AArch32-NEXT:    call void @llvm.arm.dbg(i32 0)
 // AArch32-NEXT:    ret void
 //
 void test_dbg(void) {
   __dbg(0);
 }

 void test_wsrf64(double v) {
 #ifdef __ARM_32BIT_STATE
   __arm_wsrf64("cp1:2:c3", v);
 #else
   __arm_wsrf64("1:2:3:4:5", v);
 #endif
 }
 
-#ifdef __ARM_64BIT_STATE
+#if defined(__ARM_64BIT_STATE) && defined(__ARM_FEATURE_JCVT)
 // AArch6483-LABEL: @test_jcvt(
 // AArch6483-NEXT:  entry:
 // AArch6483-NEXT:    [[TMP0:%.*]] = call i32 @llvm.aarch64.fjcvtzs(double [[V:%.*]])
 // AArch6483-NEXT:    ret i32 [[TMP0]]
 //
 int32_t test_jcvt(double v) {
   return __jcvt(v);
 }
 #endif
 
 
-#if __ARM_64BIT_STATE && defined(__ARM_FEATURE_RNG)
+#if defined(__ARM_64BIT_STATE) && defined(__ARM_FEATURE_RNG)
 
 // AArch6485-LABEL: @test_rndr(
 // AArch6485-NEXT:  entry:
 // AArch6485-NEXT:    [[TMP0:%.*]] = call { i64, i1 } @llvm.aarch64.rndr()
 // AArch6485-NEXT:    [[TMP1:%.*]] = extractvalue { i64, i1 } [[TMP0]], 0
 // AArch6485-NEXT:    [[TMP2:%.*]] = extractvalue { i64, i1 } [[TMP0]], 1
 // AArch6485-NEXT:    store i64 [[TMP1]], ptr [[__ADDR:%.*]], align 8
 // AArch6485-NEXT:    [[TMP3:%.*]] = zext i1 [[TMP2]] to i32

clang/test/CodeGen/builtins-arm64.c

void prefetch(void) {
 
   __builtin_arm_prefetch(0, 0, 0, 1, 1); // pldl1strm
   // CHECK: call {{.*}} @llvm.prefetch.p0(ptr null, i32 0, i32 0, i32 1)
 
   __builtin_arm_prefetch(0, 0, 0, 0, 0); // plil1keep
   // CHECK: call {{.*}} @llvm.prefetch.p0(ptr null, i32 0, i32 3, i32 0)
 }
 
+__attribute__((target("v8.5a")))
 int32_t jcvt(double v) {
   //CHECK-LABEL: @jcvt(
   //CHECK: call i32 @llvm.aarch64.fjcvtzs
   return __builtin_arm_jcvt(v);
 }
 
 __typeof__(__builtin_arm_rsr("1:2:3:4:5")) rsr(void);
 

 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = call { i64, i1 } @llvm.aarch64.rndr()
 // CHECK-NEXT:    [[TMP1:%.*]] = extractvalue { i64, i1 } [[TMP0]], 0
 // CHECK-NEXT:    [[TMP2:%.*]] = extractvalue { i64, i1 } [[TMP0]], 1
 // CHECK-NEXT:    store i64 [[TMP1]], ptr [[__ADDR:%.*]], align 8
 // CHECK-NEXT:    [[TMP3:%.*]] = zext i1 [[TMP2]] to i32
 // CHECK-NEXT:    ret i32 [[TMP3]]
 //
+__attribute__((target("rand")))
 int rndr(uint64_t *__addr) {
   return __builtin_arm_rndr(__addr);
 }
 
 // CHECK-LABEL: @rndrrs(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = call { i64, i1 } @llvm.aarch64.rndrrs()
 // CHECK-NEXT:    [[TMP1:%.*]] = extractvalue { i64, i1 } [[TMP0]], 0
 // CHECK-NEXT:    [[TMP2:%.*]] = extractvalue { i64, i1 } [[TMP0]], 1
 // CHECK-NEXT:    store i64 [[TMP1]], ptr [[__ADDR:%.*]], align 8
 // CHECK-NEXT:    [[TMP3:%.*]] = zext i1 [[TMP2]] to i32
 // CHECK-NEXT:    ret i32 [[TMP3]]
 //
+__attribute__((target("rand")))
 int rndrrs(uint64_t *__addr) {
   return __builtin_arm_rndrrs(__addr);
 }
 
 // CHECK: ![[M0]] = !{!"1:2:3:4:5"}