Introduce arm_acle.h supporting existing LLVM builtin intrinsics

[kongyi]

Summary

This patch introduces ACLE header file, implementing extensions that can be directly mapped to existing Clang intrinsics. It implements for both AArch32 and AArch64.

[compnerd]

Im sure that Im missing some context here. I am not sure what using this wrapper header to map the intrinsics to a __builtin_ form really gains. It adds an additional header that needs to be maintained, and requires that users include this header for the intrinsics.

At the very least, similar approaches to this on the Windows side have proven to be less than ideal.

Having some additional context around why this is preferable would be nice.

[echristo]

Sadly it looks like something ARM implemented in gcc so we should probably have it as compatibility.

[kongyi]

The intention is described in the ACLE documentation. Mainly it is for code portability across different compilers and different ARM architecture variants.

[rnk]

This sounds very similar to the immintrin.h etc headers that Intel
provides, which IMO are great. We don't have to implement everything as a
builtin in the compiler. We can insulate ourselves with a header layer.

GCC's __builtins aren't standard, so having a real document seems like a
step forward.

• from a peanut gallery somewhere

[kongyi]

A minor fix up. clzll is now guarded by the proper flag, SIZE_OF_LONG_LONG__.

[compnerd]

Ah, I had forgotten about the fact that ACLE itself specifies the arm_acle.h header.

lib/Headers/arm_acle.h

79

Why are these macros and the others static inline functions? It should be possible to do that here as well I believe?

static __inline__ int32_t __attribute__ (( always_inline, nodebug ))
__ssat(int32_t x, unsigned int y) {
  return __builtin_arm_ssat(x, y);
}
static __inline__ uint32_t __attribute__ (( always_inline, nodebug ))
__usat(int32_t x, unsigned int y) {
  return __builtin_arm_usat(x, y);
}

[rengolin]

test/CodeGen/arm_acle.c

2

Maybe add ARMv7 test with the same check-prefix?

[kongyi]

test/CodeGen/arm_acle.c

2

Cortex-A57 is chosen as the target processor(therefore ARMv8) because it implements all the features used in this patch. ARMv7 uses exactly the same code as ARMv8, but its targets doesn't cover all the features, therefore its not really useful.

[rengolin]

test/CodeGen/arm_acle.c

2

ok

[compnerd]

lib/Headers/arm_acle.h

79

Ugh, this took a while for me to reason out. Can you add a diagnostics test case to ensure that we correctly diagnose use of a non-constant value with these macros?

[kongyi]

Added unit test to make sure Clang reports error if a run-time variable is given to SSAT and USAT intrinsic.

Also updated header file for ANSI-C conformance.

[compnerd]

LGTM. Unless Tim or Renato have any objections, Id say this looks fine to commit.

[rengolin]

LGTM too.

cheers,
--renato

lib/Basic/Targets.cpp

void getTargetDefines(const LangOptions &Opts,
     Builder.defineMacro("__ARM_32BIT_STATE", "1");
 
     // ACLE 6.4.2 Architectural Profile (A, R, M or pre-Cortex)
 
     // __ARM_ARCH_PROFILE is defined as 'A', 'R', 'M' or 'S', or unset.
     if (!CPUProfile.empty())
       Builder.defineMacro("__ARM_ARCH_PROFILE", "'" + CPUProfile + "'");
 
+    // ACLE predefines.
+    Builder.defineMacro("__ARM_ACLE", "200");
+
     // Subtarget options.
 
     // FIXME: It's more complicated than this and we don't really support
     // interworking.
     // Windows on ARM does not "support" interworking
     if (5 <= CPUArchVer && CPUArchVer <= 8 && !getTriple().isOSWindows())
       Builder.defineMacro("__THUMB_INTERWORK__");
 

lib/Headers/CMakeLists.txt

 set(files
   altivec.h
   ammintrin.h
+  arm_acle.h
   avxintrin.h
   avx2intrin.h
   bmiintrin.h
   bmi2intrin.h
   emmintrin.h
   f16cintrin.h
   float.h
   fma4intrin.h

lib/Headers/arm_acle.h

+/*===---- arm_acle.h - ARM Non-Neon intrinsics -----------------------------===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===-----------------------------------------------------------------------===
+ */
+
+#ifndef __ARM_ACLE_H
+#define __ARM_ACLE_H
+
+#ifndef __ARM_ACLE
+#error "ACLE intrinsics support not enabled."
+#endif
+
+#include <stdint.h>
+
+/* Miscellaneous data-processing intrinsics */
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __clz(uint32_t t) {
+  return __builtin_clz(t);
+}
+
+static __inline__ unsigned long __attribute__((always_inline, nodebug))
+  __clzl(unsigned long t) {
+  return __builtin_clzl(t);
+}
+
+static __inline__ uint64_t __attribute__((always_inline, nodebug))
+  __clzll(uint64_t t) {
+#if __SIZEOF_LONG_LONG__ == 8
+  return __builtin_clzll(t);
+#else
+  return __builtin_clzl(t);
+#endif
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __rev(uint32_t t) {
+  return __builtin_bswap32(t);
+}
+
+static __inline__ unsigned long __attribute__((always_inline, nodebug))
+  __revl(unsigned long t) {
+#if __SIZEOF_LONG__ == 4
+  return __builtin_bswap32(t);
+#else
+  return __builtin_bswap64(t);
+#endif
+}
+
+static __inline__ uint64_t __attribute__((always_inline, nodebug))
+  __revll(uint64_t t) {
+  return __builtin_bswap64(t);
+}
+
+
+/*
+ * Saturating intrinsics
+ *
+ * FIXME: Change guard to their corrosponding __ARM_FEATURE flag when Q flag
+ * intrinsics are implemented and the flag is enabled.
+ */
+#if __ARM_32BIT_STATE
+#define __ssat(x, y) __builtin_arm_ssat(x, y)
+#define __usat(x, y) __builtin_arm_usat(x, y)
+
+static __inline__ int32_t __attribute__((always_inline, nodebug))
+  __qadd(int32_t t, int32_t v) {
+  return __builtin_arm_qadd(t, v);
+}
+
+static __inline__ int32_t __attribute__((always_inline, nodebug))
+  __qsub(int32_t t, int32_t v) {
+  return __builtin_arm_qsub(t, v);
+}
+#endif
+
+/* CRC32 intrinsics */
+#if __ARM_FEATURE_CRC32
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32b(uint32_t a, uint8_t b) {
+  return __builtin_arm_crc32b(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32h(uint32_t a, uint16_t b) {
+  return __builtin_arm_crc32h(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32w(uint32_t a, uint32_t b) {
+  return __builtin_arm_crc32w(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32d(uint32_t a, uint64_t b) {
+  return __builtin_arm_crc32d(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32cb(uint32_t a, uint8_t b) {
+  return __builtin_arm_crc32cb(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32ch(uint32_t a, uint16_t b) {
+  return __builtin_arm_crc32ch(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32cw(uint32_t a, uint32_t b) {
+  return __builtin_arm_crc32cw(a, b);
+}
+
+static __inline__ uint32_t __attribute__((always_inline, nodebug))
+  __crc32cd(uint32_t a, uint64_t b) {
+  return __builtin_arm_crc32cd(a, b);
+}
+#endif
+
+#endif /* __ARM_ACLE_H */

test/CodeGen/arm_acle.c

+// RUN: %clang_cc1 -triple armv8 -target-cpu cortex-a57 -O -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch32
+// RUN: %clang_cc1 -triple aarch64 -target-cpu cortex-a57 -O -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch64
+
+#include <arm_acle.h>
+#include <stdint.h>
+
+/* Miscellaneous data-processing intrinsics */
+// ARM-LABEL: test_rev
+// ARM: call i32 @llvm.bswap.i32(i32 %t)
+uint32_t test_rev(uint32_t t) {
+  return __rev(t);
+}
+
+// ARM-LABEL: test_revl
+// AArch32: call i32 @llvm.bswap.i32(i32 %t)
+// AArch64: call i64 @llvm.bswap.i64(i64 %t)
+long test_revl(long t) {
+  return __revl(t);
+}
+
+// ARM-LABEL: test_revll
+// ARM: call i64 @llvm.bswap.i64(i64 %t)
+uint64_t test_revll(uint64_t t) {
+  return __revll(t);
+}
+
+// ARM-LABEL: test_clz
+// ARM: call i32 @llvm.ctlz.i32(i32 %t, i1 false)
+uint32_t test_clz(uint32_t t) {
+  return __clz(t);
+}
+
+// ARM-LABEL: test_clzl
+// AArch32: call i32 @llvm.ctlz.i32(i32 %t, i1 false)
+// AArch64: call i64 @llvm.ctlz.i64(i64 %t, i1 false)
+long test_clzl(long t) {
+  return __clzl(t);
+}
+
+// ARM-LABEL: test_clzll
+// ARM: call i64 @llvm.ctlz.i64(i64 %t, i1 false)
+uint64_t test_clzll(uint64_t t) {
+  return __clzll(t);
+}
+
+/* Saturating intrinsics */
+#ifdef __ARM_32BIT_STATE
+// AArch32-LABEL: test_ssat
+// AArch32: call i32 @llvm.arm.ssat(i32 %t, i32 1)
+int32_t test_ssat(int32_t t) {
+  return __ssat(t, 1);
+}
+
+// AArch32-LABEL: test_usat
+// AArch32: call i32 @llvm.arm.usat(i32 %t, i32 2)
+int32_t test_usat(int32_t t) {
+  return __usat(t, 2);
+}
+// AArch32-LABEL: test_qadd
+// AArch32: call i32 @llvm.arm.qadd(i32 %a, i32 %b)
+int32_t test_qadd(int32_t a, int32_t b) {
+  return __qadd(a, b);
+}
+
+// AArch32-LABEL: test_qsub
+// AArch32: call i32 @llvm.arm.qsub(i32 %a, i32 %b)
+int32_t test_qsub(int32_t a, int32_t b) {
+  return __qsub(a, b);
+}
+#endif
+
+/* CRC32 intrinsics */
+// ARM-LABEL: test_crc32b
+// AArch32: call i32 @llvm.arm.crc32b
+// AArch64: call i32 @llvm.aarch64.crc32b
+uint32_t test_crc32b(uint32_t a, uint8_t b) {
+  return __crc32b(a, b);
+}
+
+// ARM-LABEL: test_crc32h
+// AArch32: call i32 @llvm.arm.crc32h
+// AArch64: call i32 @llvm.aarch64.crc32h
+uint32_t test_crc32h(uint32_t a, uint16_t b) {
+  return __crc32h(a, b);
+}
+
+// ARM-LABEL: test_crc32w
+// AArch32: call i32 @llvm.arm.crc32w
+// AArch64: call i32 @llvm.aarch64.crc32w
+uint32_t test_crc32w(uint32_t a, uint32_t b) {
+  return __crc32w(a, b);
+}
+
+// ARM-LABEL: test_crc32d
+// AArch32: call i32 @llvm.arm.crc32w
+// AArch32: call i32 @llvm.arm.crc32w
+// AArch64: call i32 @llvm.aarch64.crc32x
+uint32_t test_crc32d(uint32_t a, uint64_t b) {
+  return __crc32d(a, b);
+}
+
+// ARM-LABEL: test_crc32cb
+// AArch32: call i32 @llvm.arm.crc32cb
+// AArch64: call i32 @llvm.aarch64.crc32cb
+uint32_t test_crc32cb(uint32_t a, uint8_t b) {
+  return __crc32cb(a, b);
+}
+
+// ARM-LABEL: test_crc32ch
+// AArch32: call i32 @llvm.arm.crc32ch
+// AArch64: call i32 @llvm.aarch64.crc32ch
+uint32_t test_crc32ch(uint32_t a, uint16_t b) {
+  return __crc32ch(a, b);
+}
+
+// ARM-LABEL: test_crc32cw
+// AArch32: call i32 @llvm.arm.crc32cw
+// AArch64: call i32 @llvm.aarch64.crc32cw
+uint32_t test_crc32cw(uint32_t a, uint32_t b) {
+  return __crc32cw(a, b);
+}
+
+// ARM-LABEL: test_crc32cd
+// AArch32: call i32 @llvm.arm.crc32cw
+// AArch32: call i32 @llvm.arm.crc32cw
+// AArch64: call i32 @llvm.aarch64.crc32cx
+uint32_t test_crc32cd(uint32_t a, uint64_t b) {
+  return __crc32cd(a, b);
+}

test/Headers/arm-acle-header.c

+// RUN: %clang_cc1 -triple armv7 -target-cpu cortex-a15 -fsyntax-only -ffreestanding %s
+// RUN: %clang_cc1 -triple aarch64 -target-cpu cortex-a53 -fsyntax-only -ffreestanding %s
+// RUN: %clang_cc1 -x c++ -triple armv7 -target-cpu cortex-a15 -fsyntax-only -ffreestanding %s
+// RUN: %clang_cc1 -x c++ -triple aarch64 -target-cpu cortex-a57 -fsyntax-only -ffreestanding %s
+// expected-no-diagnostics
+
+#include <arm_acle.h>

test/Sema/arm_acle.c

+// RUN: %clang_cc1 -triple armv8 -target-cpu cortex-a57 -fsyntax-only -verify %s
+
+#include <arm_acle.h>
+
+/*
+ * Saturating intrinsics
+ * Second argument for SSAT and USAT intrinsics must be compile-time constant,
+ * otherwise an error should be raised.
+ */
+int32_t test_ssat_const_diag(int32_t t, const int32_t v) {
+  return __ssat(t, v);  // expected-error-re {{argument to {{.*}} must be a constant integer}}
+}
+
+int32_t test_usat_const_diag(int32_t t, const int32_t v) {
+  return __usat(t, v);  // expected-error-re {{argument to {{.*}} must be a constant integer}}
+}