Add ARM cdp intrinsics

[rengolin]

Summary

Please review my patch to add the \_\_arm_cdp and \_\_arm_cdp2 intrinsics. These intrinsics don't exist in the latest ACLE (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ihi0053c/index.html) but will exist in a future version.

[rengolin]

Please, re-submit this change once the documents have been made public.

Feel free to submit the assembly test to LLVM, using llc, on their own.

cheers,
--renato

include/clang/Basic/BuiltinsARM.def

55

I wonder why the old signature was wrong... probably because the docs weren't public and no one could really check whether they were right or not. I don't want to repeat the same mistake.

test/CodeGen/arm-coproc-intrinsics.c

1

Please, do not add assembly tests in Clang. This test should be in LLVM.

[t.p.northover]

I don't think we need to wait until the document is public, necessarily. The entire AArch64 backend went in before the encodings were public outside ARM (except maybe in binutils source?) and releasing specifications with no implementation yet is a bit annoying too.

We may discover bugs (though this seems fairly straightforward), but I think it's highly unlikely we'll have vocal and powerful enough users of such a niche feature to prevent us fixing them.

I agree about the asm tests though, they belong in LLVM.

include/clang/Basic/BuiltinsARM.def

55

The difference between "UIi" and "Ui" is that the fix requires a constant. A pretty obvious restriction even without a specification.

[rengolin]

In D20325#432086, @t.p.northover wrote:

I don't think we need to wait until the document is public, necessarily. The entire AArch64 backend went in before the encodings were public outside ARM (except maybe in binutils source?) and releasing specifications with no implementation yet is a bit annoying too.

I don't think a small future ACLE builtin is on the same league as a whole new back-end. :)

Nor I think that having this when the docs are public, rather than now, will affect users in any way.

It's been our stance for a long time to require docs to approve changes, however small. I don't want to relax that which I think is a good constraint, not for such a seemly irrelevant issue.

I also doubt this will be the only addition in the new ACLE, so why not release the document, and then submit all changes then?

Or, maybe I am mistaken, and this is really that important... is it?

cheers,
--renato

include/clang/Basic/BuiltinsARM.def

55

Ah, this makes sense. Looks like a good change in its own.

[rs]

It's been our stance for a long time to require docs to approve changes, however small. I don't want to relax that which I think is a good constraint, not for such a seemly irrelevant issue.
I also doubt this will be the only addition in the new ACLE, so why not release the document, and then submit all changes then?
Or, maybe I am mistaken, and this is really that important... is it?

Thanks for reviewing.

It's not that important to have the intrinsic added before the document is released. I'll upload a new patch without the intrinsic in arm_acle.h and I'll move the assembly test to LLVM.

[rengolin]

Thanks Ranjeet,

The tests don't really need the new builtin to exist at all and can be added now.

When you submit the __arm_cdp patch, you just need to make sure Clang generates a call to @llvm.arm.cdp and everything else will be covered.

cheers,
--renato

[rs]

Hi Renato,

I've created 2 new reviews for this work http://reviews.llvm.org/D20394 which adds a test for the intrinsic in llvm and http://reviews.llvm.org/D20394 which fixes the builtin signature for the cdp intrinsic.

Thanks,
Ranjeet

[rs]

http://reviews.llvm.org/D20394 which adds a test for the intrinsic in llvm

Wrong link, should be http://reviews.llvm.org/D20393

include/clang/Basic/BuiltinsARM.def

 BUILTIN(__builtin_arm_vcvtr_f, "ffi", "nc")
 BUILTIN(__builtin_arm_vcvtr_d, "fdi", "nc")
 
 // Coprocessor
 BUILTIN(__builtin_arm_mcr, "vUIiUIiUiUIiUIiUIi", "")
 BUILTIN(__builtin_arm_mcr2, "vUIiUIiUiUIiUIiUIi", "")
 BUILTIN(__builtin_arm_mrc, "UiUIiUIiUIiUIiUIi", "")
 BUILTIN(__builtin_arm_mrc2, "UiUIiUIiUIiUIiUIi", "")
-BUILTIN(__builtin_arm_cdp, "vUiUiUiUiUiUi", "")
-BUILTIN(__builtin_arm_cdp2, "vUiUiUiUiUiUi", "")
+BUILTIN(__builtin_arm_cdp, "vUIiUIiUIiUIiUIiUIi", "")
+BUILTIN(__builtin_arm_cdp2, "vUIiUIiUIiUIiUIiUIi", "")
 BUILTIN(__builtin_arm_mcrr, "vUIiUIiUiUiUIi", "")
 BUILTIN(__builtin_arm_mcrr2, "vUIiUIiUiUiUIi", "")
 
 // CRC32
 BUILTIN(__builtin_arm_crc32b, "UiUiUc", "nc")
 BUILTIN(__builtin_arm_crc32cb, "UiUiUc", "nc")
 BUILTIN(__builtin_arm_crc32h, "UiUiUs", "nc")
 BUILTIN(__builtin_arm_crc32ch, "UiUiUs", "nc")

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
 
+/* Co-Processor Intrinsics */
+#define __arm_cdp(coproc, opc1, crd, crn, crm, opc2) __builtin_arm_cdp(coproc, opc1, crd, crn, crm, opc2)
+#define __arm_cdp2(coproc, opc1, crd, crn, crm, opc2) __builtin_arm_cdp2(coproc, opc1, crd, crn, crm, opc2)
+
 /* 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)
 
 #if defined(__cplusplus)
 }
 #endif
 
 #endif /* __ARM_ACLE_H */

test/CodeGen/arm-coproc-intrinsics.c

+// RUN: %clang_cc1 -Wall -triple thumbv7-eabi -target-cpu cortex-a8 -S -o - %s | FileCheck %s
+
+#include <arm_acle.h>
+
+void test1() {
+    // CHECK: cdp p1, #2, c3, c4, c5, #6
+    __arm_cdp(1, 2, 3, 4, 5, 6);
+
+    // CHECK: cdp2 p1, #2, c3, c4, c5, #6
+    __arm_cdp2(1, 2, 3, 4, 5, 6);
+}

test/CodeGen/builtins-arm.c

// CHECK: call {{.*}} @llvm.prefetch(i8* %{{.*}}, i32 0, i32 3, i32 1)
   __builtin_arm_prefetch(&i, 1, 1);
 // CHECK: call {{.*}} @llvm.prefetch(i8* %{{.*}}, i32 1, i32 3, i32 1)
 
 
   __builtin_arm_prefetch(&i, 1, 0);
 // CHECK: call {{.*}} @llvm.prefetch(i8* %{{.*}}, i32 1, i32 3, i32 0)
 }
 
+void cdp() {
+  // CHECK: define void @cdp()
+  // CHECK: call void @llvm.arm.cdp(i32 1, i32 2, i32 3, i32 4, i32 5, i32 6)
+  // CHECK-NEXT: ret void
+  __builtin_arm_cdp(1, 2, 3, 4, 5, 6);
+}
+
+void cdp2() {
+  // CHECK: define void @cdp2()
+  // CHECK: call void @llvm.arm.cdp2(i32 1, i32 2, i32 3, i32 4, i32 5, i32 6)
+  // CHECK-NEXT: ret void
+  __builtin_arm_cdp2(1, 2, 3, 4, 5, 6);
+}
+
 unsigned mrc() {
   // CHECK: define i32 @mrc()
   // CHECK: [[R:%.*]] = call i32 @llvm.arm.mrc(i32 15, i32 0, i32 13, i32 0, i32 3)
   // CHECK-NEXT: ret i32 [[R]]
   return __builtin_arm_mrc(15, 0, 13, 0, 3);
 }
 
 unsigned mrc2() {

test/Sema/builtins-arm.c

void test4() {
   __builtin_arm_prefetch(0, 0, 2); // expected-error {{argument should be a value from 0 to 1}}
 }
 
 void test5() {
   __builtin_arm_dbg(16); // expected-error {{argument should be a value from 0 to 15}}
 }
 
 void test6(int a, int b, int c) {
+  __builtin_arm_cdp(a, 2, 3, 4, 5, 6); // expected-error {{argument to '__builtin_arm_cdp' must be a constant integer}}
+  __builtin_arm_cdp(1, a, 3, 4, 5, 6); // expected-error {{argument to '__builtin_arm_cdp' must be a constant integer}}
+  __builtin_arm_cdp(1, 2, a, 4, 5, 6); // expected-error {{argument to '__builtin_arm_cdp' must be a constant integer}}
+  __builtin_arm_cdp(1, 2, 3, a, 5, 6); // expected-error {{argument to '__builtin_arm_cdp' must be a constant integer}}
+  __builtin_arm_cdp(1, 2, 3, 4, 5, a); // expected-error {{argument to '__builtin_arm_cdp' must be a constant integer}}
+
+  __builtin_arm_cdp2(a, 2, 3, 4, 5, 6); // expected-error {{argument to '__builtin_arm_cdp2' must be a constant integer}}
+  __builtin_arm_cdp2(1, a, 3, 4, 5, 6); // expected-error {{argument to '__builtin_arm_cdp2' must be a constant integer}}
+  __builtin_arm_cdp2(1, 2, a, 4, 5, 6); // expected-error {{argument to '__builtin_arm_cdp2' must be a constant integer}}
+  __builtin_arm_cdp2(1, 2, 3, a, 5, 6); // expected-error {{argument to '__builtin_arm_cdp2' must be a constant integer}}
+  __builtin_arm_cdp2(1, 2, 3, 4, 5, a); // expected-error {{argument to '__builtin_arm_cdp2' must be a constant integer}}
+
   __builtin_arm_mrc( a, 0, 13, 0, 3); // expected-error {{argument to '__builtin_arm_mrc' must be a constant integer}}
   __builtin_arm_mrc(15, a, 13, 0, 3); // expected-error {{argument to '__builtin_arm_mrc' must be a constant integer}}
   __builtin_arm_mrc(15, 0,  a, 0, 3); // expected-error {{argument to '__builtin_arm_mrc' must be a constant integer}}
   __builtin_arm_mrc(15, 0, 13, a, 3); // expected-error {{argument to '__builtin_arm_mrc' must be a constant integer}}
   __builtin_arm_mrc(15, 0, 13, 0, a); // expected-error {{argument to '__builtin_arm_mrc' must be a constant integer}}
 
   __builtin_arm_mrc2( a, 0, 13, 0, 3); // expected-error {{argument to '__builtin_arm_mrc2' must be a constant integer}}
   __builtin_arm_mrc2(15, a, 13, 0, 3); // expected-error {{argument to '__builtin_arm_mrc2' must be a constant integer}}