[AArch64] Implement __jcvt intrinsic from Armv8.3-A

[ktkachov]

Summary

The jcvt intrinsic defined in ACLE [1] is available when ARM_FEATURE_JCVT is defined.

This change introduces the AArch64 intrinsic, wires it up to the instruction and a new clang builtin function.
The __ARM_FEATURE_JCVT macro is now defined when an Armv8.3-A or higher target is used.
I've implemented the target detection logic in Clang so that this feature is enabled for architectures from armv8.3-a onwards (so -march=armv8.4-a also enables this, for example).

make check-all didn't show any new failures.

[1] https://developer.arm.com/docs/101028/latest/data-processing-intrinsics

N.B. This is my first patch to LLVM. Apologies if some code looks weird. If this is okay can somebody please apply it for me?

[SjoerdMeijer]

clang/lib/Basic/Targets/AArch64.cpp

237 ↗

(On Diff #208951)

It is a good change, but I think you should either add tests for these cases, or remove this (temporarily) because it is not adding much at the moment.

clang/test/CodeGen/arm_acle.c

829 ↗

(On Diff #208951)

Same comment, better is to do a CHECK-LABEL first

clang/test/CodeGen/builtins-arm64.c

61 ↗

(On Diff #208951)

Although this file doesn't seem to do this, better is check the function name first, e.g.:

// CHECK-LABEL: @jcvt(

followed by what you want to check:

//CHECK: call i32 @llvm.aarch64.fjcvtzs

[SjoerdMeijer]

clang/lib/Basic/Targets/AArch64.cpp

237 ↗

(On Diff #208951)

Ah, sorry: "-march=armv8.4-a also enables this"
probably good to add some tests for v8.4 and v8.5 too then.

[SjoerdMeijer]

llvm/lib/Target/AArch64/AArch64InstrInfo.td

720 ↗

(On Diff #208951)

and a last nit: this needs some reformatting (exceeding the max column width)

[ktkachov]

Add more CHECK-LABEL tests, test v8.4a and v8.5a features. Fix formatting in pattern.

[SjoerdMeijer]

Looks like a good change to me, some nits inlined.

It shouldn't be difficult to request an account and commit it yourself, which might be useful if you maybe intend to submit more patches.
But I can of course easily commit this on your behalf, just let me know. If you want me to commit this, it is easiest if you do upload a new diff with the nits fixed.

clang/test/CodeGen/arm_acle.c

5 ↗

(On Diff #208980)

Some nits that you can fix before committing; no need for another review I think.

You can remove:

-check-prefix=CHECK-LABEL

in all these 3 lines below.

830 ↗

(On Diff #208980)

and change

// CHECK-LABEL: @test_jcvt(

into

// AArch64-v8_3-LABEL: @test_jcvt(

[ktkachov]

Fix comments plus an offline comment I had (copy-pasto in an assert message)

[sammccall]

FYI, this change broke git-llvm for everyone with a different username :-)
Fixed in r366198

[ktkachov]

In D64495#1587266, @sammccall wrote:

FYI, this change broke git-llvm for everyone with a different username :-)
Fixed in r366198

Ah sorry, I accidentally included it in the commit!

cfe/trunk/include/clang/Basic/BuiltinsAArch64.def

 BUILTIN(__builtin_arm_stg, "vv*", "t")
 BUILTIN(__builtin_arm_subp, "Uiv*v*", "t")
 
 // 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")
+
 // 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")
 BUILTIN(__builtin_arm_wsr, "vcC*Ui", "nc")

cfe/trunk/lib/Basic/Targets/AArch64.h

public:
   bool useFP16ConversionIntrinsics() const override {
     return false;
   }
 
   void getTargetDefinesARMV81A(const LangOptions &Opts,
                                MacroBuilder &Builder) const;
   void getTargetDefinesARMV82A(const LangOptions &Opts,
                                MacroBuilder &Builder) const;
+  void getTargetDefinesARMV83A(const LangOptions &Opts,
+                               MacroBuilder &Builder) const;
+  void getTargetDefinesARMV84A(const LangOptions &Opts,
+                               MacroBuilder &Builder) const;
+  void getTargetDefinesARMV85A(const LangOptions &Opts,
+                               MacroBuilder &Builder) const;
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 
   ArrayRef<Builtin::Info> getTargetBuiltins() const override;
 
   bool hasFeature(StringRef Feature) const override;
   bool handleTargetFeatures(std::vector<std::string> &Features,
                             DiagnosticsEngine &Diags) override;

cfe/trunk/lib/Basic/Targets/AArch64.cpp

 }
 
 void AArch64TargetInfo::getTargetDefinesARMV82A(const LangOptions &Opts,
                                                 MacroBuilder &Builder) const {
   // Also include the ARMv8.1 defines
   getTargetDefinesARMV81A(Opts, Builder);
 }
 
+void AArch64TargetInfo::getTargetDefinesARMV83A(const LangOptions &Opts,
+                                                MacroBuilder &Builder) const {
+  Builder.defineMacro("__ARM_FEATURE_JCVT", "1");
+  // Also include the Armv8.2 defines
+  getTargetDefinesARMV82A(Opts, Builder);
+}
+
+void AArch64TargetInfo::getTargetDefinesARMV84A(const LangOptions &Opts,
+                                                MacroBuilder &Builder) const {
+  // Also include the Armv8.3 defines
+  // FIXME: Armv8.4 makes some extensions mandatory. Handle them here.
+  getTargetDefinesARMV83A(Opts, Builder);
+}
+
+void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions &Opts,
+                                                MacroBuilder &Builder) const {
+  // Also include the Armv8.4 defines
+  // FIXME: Armv8.5 makes some extensions mandatory. Handle them here.
+  getTargetDefinesARMV84A(Opts, Builder);
+}
+
+
 void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
                                          MacroBuilder &Builder) const {
   // Target identification.
   Builder.defineMacro("__aarch64__");
   // For bare-metal.
   if (getTriple().getOS() == llvm::Triple::UnknownOS &&
       getTriple().isOSBinFormatELF())
     Builder.defineMacro("__ELF__");
void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
   default:
     break;
   case llvm::AArch64::ArchKind::ARMV8_1A:
     getTargetDefinesARMV81A(Opts, Builder);
     break;
   case llvm::AArch64::ArchKind::ARMV8_2A:
     getTargetDefinesARMV82A(Opts, Builder);
     break;
+  case llvm::AArch64::ArchKind::ARMV8_3A:
+    getTargetDefinesARMV83A(Opts, Builder);
+    break;
+  case llvm::AArch64::ArchKind::ARMV8_4A:
+    getTargetDefinesARMV84A(Opts, Builder);
+    break;
+  case llvm::AArch64::ArchKind::ARMV8_5A:
+    getTargetDefinesARMV85A(Opts, Builder);
+    break;
   }
 
   // All of the __sync_(bool|val)_compare_and_swap_(1|2|4|8) builtins work.
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1");
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2");
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4");
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
 }
for (const auto &Feature : Features) {
     if (Feature == "+crypto")
       Crypto = 1;
     if (Feature == "+strict-align")
       Unaligned = 0;
     if (Feature == "+v8.1a")
       ArchKind = llvm::AArch64::ArchKind::ARMV8_1A;
     if (Feature == "+v8.2a")
       ArchKind = llvm::AArch64::ArchKind::ARMV8_2A;
+    if (Feature == "+v8.3a")
+      ArchKind = llvm::AArch64::ArchKind::ARMV8_3A;
+    if (Feature == "+v8.4a")
+      ArchKind = llvm::AArch64::ArchKind::ARMV8_4A;
+    if (Feature == "+v8.5a")
+      ArchKind = llvm::AArch64::ArchKind::ARMV8_5A;
     if (Feature == "+fullfp16")
       HasFullFP16 = 1;
     if (Feature == "+dotprod")
       HasDotProd = 1;
     if (Feature == "+fp16fml")
       HasFP16FML = 1;
     if (Feature == "+mte")
       HasMTE = 1;

cfe/trunk/lib/CodeGen/CGBuiltin.cpp

Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
   if (BuiltinID == AArch64::BI__builtin_arm_rbit64) {
     assert((getContext().getTypeSize(E->getType()) == 64) &&
            "rbit of unusual size!");
     llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
     return Builder.CreateCall(
         CGM.getIntrinsic(Intrinsic::bitreverse, Arg->getType()), Arg, "rbit");
   }
 
+  if (BuiltinID == AArch64::BI__builtin_arm_jcvt) {
+    assert((getContext().getTypeSize(E->getType()) == 32) &&
+           "__jcvt of unusual size!");
+    llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+    return Builder.CreateCall(
+        CGM.getIntrinsic(Intrinsic::aarch64_fjcvtzs), Arg);
+  }
+
   if (BuiltinID == AArch64::BI__clear_cache) {
     assert(E->getNumArgs() == 2 && "__clear_cache takes 2 arguments");
     const FunctionDecl *FD = E->getDirectCallee();
     Value *Ops[2];
     for (unsigned i = 0; i < 2; i++)
       Ops[i] = EmitScalarExpr(E->getArg(i));
     llvm::Type *Ty = CGM.getTypes().ConvertType(FD->getType());
     llvm::FunctionType *FTy = cast<llvm::FunctionType>(Ty);

cfe/trunk/lib/Headers/arm_acle.h

 }
 
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __crc32cd(uint32_t __a, uint64_t __b) {
   return __builtin_arm_crc32cd(__a, __b);
 }
 #endif
 
+/* Armv8.3-A Javascript conversion intrinsic */
+#if __ARM_64BIT_STATE && defined(__ARM_FEATURE_JCVT)
+static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
+__jcvt(double __a) {
+  return __builtin_arm_jcvt(__a);
+}
+#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_wsr(sysreg, v) __builtin_arm_wsr(sysreg, v)
 #define __arm_wsr64(sysreg, v) __builtin_arm_wsr64(sysreg, v)
 #define __arm_wsrp(sysreg, v) __builtin_arm_wsrp(sysreg, v)
 

cfe/trunk/test/CodeGen/arm_acle.c

 // RUN: %clang_cc1 -ffreestanding -triple armv8-eabi -target-cpu cortex-a57 -O2  -fno-experimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch32 -check-prefix=ARM-LEGACY -check-prefix=AArch32-LEGACY
 // RUN: %clang_cc1 -ffreestanding -triple armv8-eabi -target-cpu cortex-a57 -O2  -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch32 -check-prefix=ARM-NEWPM -check-prefix=AArch32-NEWPM
 // RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +neon -target-feature +crc -target-feature +crypto -O2 -fno-experimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch64 -check-prefix=ARM-LEGACY -check-prefix=AArch64-LEGACY
 // RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +neon -target-feature +crc -target-feature +crypto -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch64 -check-prefix=ARM-NEWPM -check-prefix=AArch64-NEWPM
+// RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +v8.3a -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=AArch64-v8_3
+// RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +v8.4a -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=AArch64-v8_3
+// RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +v8.5a -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=AArch64-v8_3
 
 #include <arm_acle.h>
 
 /* 8 SYNCHRONIZATION, BARRIER AND HINT INTRINSICS */
 /* 8.3 Memory Barriers */
 // ARM-LABEL: test_dmb
 // AArch32: call void @llvm.arm.dmb(i32 1)
 // AArch64: call void @llvm.aarch64.dmb(i32 1)

 }
 
 // AArch32: ![[M2]] = !{!"cp1:2:c3:c4:5"}
 // AArch32: ![[M3]] = !{!"cp1:2:c3"}
 // AArch32: ![[M4]] = !{!"sysreg"}
 
 // AArch64: ![[M0]] = !{!"1:2:3:4:5"}
 // AArch64: ![[M1]] = !{!"sysreg"}
+
+// AArch64-v8_3-LABEL: @test_jcvt(
+// AArch64-v8_3: call i32 @llvm.aarch64.fjcvtzs
+#ifdef __ARM_64BIT_STATE
+int32_t test_jcvt(double v) {
+  return __jcvt(v);
+}
+#endif

cfe/trunk/test/CodeGen/builtins-arm64.c

// CHECK: call {{.*}} @llvm.prefetch(i8* null, i32 0, i32 0, i32 1)
 
   __builtin_arm_prefetch(0, 0, 0, 1, 1); // pldl1strm
 // CHECK: call {{.*}} @llvm.prefetch(i8* null, i32 0, i32 0, i32 1)
 
   __builtin_arm_prefetch(0, 0, 0, 0, 0); // plil1keep
 // CHECK: call {{.*}} @llvm.prefetch(i8* null, i32 0, i32 3, i32 0)
 }
 
+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);
 
 uint32_t rsr() {
   // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
   // CHECK-NEXT: trunc i64 [[V0]] to i32
   return __builtin_arm_rsr("1:2:3:4:5");
 }
 

llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td

 
 def int_aarch64_clrex : Intrinsic<[]>;
 
 def int_aarch64_sdiv : Intrinsic<[llvm_anyint_ty], [LLVMMatchType<0>,
                                 LLVMMatchType<0>], [IntrNoMem]>;
 def int_aarch64_udiv : Intrinsic<[llvm_anyint_ty], [LLVMMatchType<0>,
                                 LLVMMatchType<0>], [IntrNoMem]>;
 
+def int_aarch64_fjcvtzs : Intrinsic<[llvm_i32_ty], [llvm_double_ty], [IntrNoMem]>;
+
 //===----------------------------------------------------------------------===//
 // HINT
 
 def int_aarch64_hint : Intrinsic<[], [llvm_i32_ty]>;
 
 //===----------------------------------------------------------------------===//
 // Data Barrier Instructions
 

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td

let Predicates = [HasPA] in {
   defm LDRAA  : AuthLoad<0, "ldraa", simm10Scaled>;
   defm LDRAB  : AuthLoad<1, "ldrab", simm10Scaled>;
 
 }
 
 // v8.3a floating point conversion for javascript
 let Predicates = [HasJS, HasFPARMv8] in
 def FJCVTZS  : BaseFPToIntegerUnscaled<0b01, 0b11, 0b110, FPR64, GPR32,
-                                      "fjcvtzs", []> {
+                                      "fjcvtzs",
+                                      [(set GPR32:$Rd,
+                                         (int_aarch64_fjcvtzs FPR64:$Rn))]> {
   let Inst{31} = 0;
 } // HasJS, HasFPARMv8
 
 // v8.4 Flag manipulation instructions
 let Predicates = [HasFMI] in {
 def CFINV : SimpleSystemI<0, (ins), "cfinv", "">, Sched<[WriteSys]> {
   let Inst{20-5} = 0b0000001000000000;
 }

llvm/trunk/test/CodeGen/AArch64/fjcvtzs.ll

+; RUN: llc -mtriple=arm64-eabi -mattr=+jsconv -o - %s | FileCheck %s
+
+define i32 @test_jcvt(double %v) {
+; CHECK-LABEL: test_jcvt:
+; CHECK: fjcvtzs w0, d0
+  %val = call i32 @llvm.aarch64.fjcvtzs(double %v)
+  ret i32 %val
+}
+
+declare i32 @llvm.aarch64.fjcvtzs(double)

llvm/trunk/utils/git-svn/git-llvm

for l in (l for l in status_lines if (l.startswith('?') or
                                           l.startswith('I'))):
         svn(svn_repo, 'add', '--no-ignore', l[1:].strip())
     for l in (l for l in status_lines if l.startswith('!')):
         svn(svn_repo, 'remove', l[1:].strip())
 
     # Now we're ready to commit.
     commit_msg = git('show', '--pretty=%B', '--quiet', rev)
     if not dry_run:
-        commit_args = ['commit', '-m', commit_msg]
+        commit_args = ['commit', '-m', commit_msg, '--username', 'ktkachov']
         if '--force-interactive' in svn(svn_repo, 'commit', '--help'):
             commit_args.append('--force-interactive')
         log(svn(svn_repo, *commit_args))
         log('Committed %s to svn.' % rev)
     else:
         log("Would have committed %s to svn, if this weren't a dry run." % rev)