[ARM] arm_acle.h add Coprocessor Instrinsics #75440

hstk30-hw · 2023-12-14T07:49:09Z

#75424

Add Coprocessor Instrinsics

llvmbot · 2023-12-14T07:49:42Z

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-clang

Author: None (hstk30-hw)

Changes

#75424

Add Corprocessor Instrinsics

Full diff: https://github.com/llvm/llvm-project/pull/75440.diff

1 Files Affected:

(modified) clang/lib/Headers/arm_acle.h (+48-1)

diff --git a/clang/lib/Headers/arm_acle.h b/clang/lib/Headers/arm_acle.h
index 61d80258d166a1..9c709550ccc45b 100644
--- a/clang/lib/Headers/arm_acle.h
+++ b/clang/lib/Headers/arm_acle.h
@@ -752,10 +752,57 @@ __arm_st64bv0(void *__addr, data512_t __value) {
 #define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)
 
 /* Memory Operations Intrinsics */
-#define __arm_mops_memset_tag(__tagged_address, __value, __size)    \
+#define __arm_mops_memset_tag(__tagged_address, __value, __size)               \
   __builtin_arm_mops_memset_tag(__tagged_address, __value, __size)
 #endif
 
+/* Coprocessor Intrinsics */
+#if (!__thumb__ || __thumb2__) && __ARM_ARCH >= 4
+#define __arm_cdp(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)               \
+  __builtin_arm_cdp(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)
+#define __arm_ldc(__coproc, __CRd, __p) __builtin_arm_ldc(__coproc, __CRd, __p)
+#define __arm_ldcl(__coproc, __CRd, __p)                                       \
+  __builtin_arm_ldcl(__coproc, __CRd, __p)
+#define __arm_stc(__coproc, __CRd, __p) __builtin_arm_stc(__coproc, __CRd, __p)
+#define __arm_stcl(__coproc, __CRd, __p)                                       \
+  __builtin_arm_stcl(__coproc, __CRd, __p)
+#define __arm_mcr(__coproc, __opc1, __value, __CRn, __CRm, __opc2)             \
+  __builtin_arm_mcr(__coproc, __opc1, __value, __CRn, __CRm, __opc2)
+#define __arm_mrc(__coproc, __opc1, __CRn, __CRm, __opc2)                      \
+  __builtin_arm_mrc(__coproc, __opc1, __CRn, __CRm, __opc2)
+
+#if __ARM_ARCH >= 5
+#define __arm_cdp2(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)              \
+  __builtin_arm_cdp2(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)
+#define __arm_ldc2(__coproc, __CRd, __p)                                       \
+  __builtin_arm_ldc2(__coproc, __CRd, __p)
+#define __arm_ldc2l(__coproc, __CRd, __p)                                      \
+  __builtin_arm_ldc2l(__coproc, __CRd, __p)
+#define __arm_stc2(__coproc, __CRd, __p)                                       \
+  __builtin_arm_stc2(__coproc, __CRd, __p)
+#define __arm_stc2l(__coproc, __CRd, __p)                                      \
+  __builtin_arm_stc2l(__coproc, __CRd, __p)
+#define __arm_mcr2(__coproc, __opc1, __value, __CRn, __CRm, __opc2)            \
+  __builtin_arm_mcr2(__coproc, __opc1, __value, __CRn, __CRm, __opc2)
+#define __arm_mrc2(__coproc, __opc1, __CRn, __CRm, __opc2)                     \
+  __builtin_arm_mrc2(__coproc, __opc1, __CRn, __CRm, __opc2)
+
+#if __ARM_ARCH >= 6 || defined(__ARM_ARCH_5TE__)
+#define __arm_mcrr(__coproc, __opc1, __value, __CRm)                           \
+  __builtin_arm_mcrr(__coproc, __opc1, __value, __CRm)
+#define __arm_mrrc(__coproc, __opc1, __CRm)                                    \
+  __builtin_arm_mrrc(__coproc, __opc1, __CRm)
+
+#if __ARM_ARCH >= 6
+#define __arm_mcrr2(__coproc, __opc1, __value, __CRm)                          \
+  __builtin_arm_mcrr2(__coproc, __opc1, __value, __CRm)
+#define __arm_mrrc2(__coproc, __opc1, __CRm)                                   \
+  __builtin_arm_mrrc2(__coproc, __opc1, __CRm)
+#endif /* __ARM_ARCH >= 6.  */
+#endif /* __ARM_ARCH >= 6 ||  defined (__ARM_ARCH_5TE__).  */
+#endif /*  __ARM_ARCH >= 5.  */
+#endif /* (!__thumb__ || __thumb2__) &&  __ARM_ARCH >= 4.  */
+
 /* Transactional Memory Extension (TME) Intrinsics */
 #if defined(__ARM_FEATURE_TME) && __ARM_FEATURE_TME

llvmbot · 2023-12-14T07:49:42Z

@llvm/pr-subscribers-backend-x86

Author: None (hstk30-hw)

Changes

#75424

Add Corprocessor Instrinsics

Full diff: https://github.com/llvm/llvm-project/pull/75440.diff

1 Files Affected:

(modified) clang/lib/Headers/arm_acle.h (+48-1)

diff --git a/clang/lib/Headers/arm_acle.h b/clang/lib/Headers/arm_acle.h
index 61d80258d166a1..9c709550ccc45b 100644
--- a/clang/lib/Headers/arm_acle.h
+++ b/clang/lib/Headers/arm_acle.h
@@ -752,10 +752,57 @@ __arm_st64bv0(void *__addr, data512_t __value) {
 #define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)
 
 /* Memory Operations Intrinsics */
-#define __arm_mops_memset_tag(__tagged_address, __value, __size)    \
+#define __arm_mops_memset_tag(__tagged_address, __value, __size)               \
   __builtin_arm_mops_memset_tag(__tagged_address, __value, __size)
 #endif
 
+/* Coprocessor Intrinsics */
+#if (!__thumb__ || __thumb2__) && __ARM_ARCH >= 4
+#define __arm_cdp(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)               \
+  __builtin_arm_cdp(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)
+#define __arm_ldc(__coproc, __CRd, __p) __builtin_arm_ldc(__coproc, __CRd, __p)
+#define __arm_ldcl(__coproc, __CRd, __p)                                       \
+  __builtin_arm_ldcl(__coproc, __CRd, __p)
+#define __arm_stc(__coproc, __CRd, __p) __builtin_arm_stc(__coproc, __CRd, __p)
+#define __arm_stcl(__coproc, __CRd, __p)                                       \
+  __builtin_arm_stcl(__coproc, __CRd, __p)
+#define __arm_mcr(__coproc, __opc1, __value, __CRn, __CRm, __opc2)             \
+  __builtin_arm_mcr(__coproc, __opc1, __value, __CRn, __CRm, __opc2)
+#define __arm_mrc(__coproc, __opc1, __CRn, __CRm, __opc2)                      \
+  __builtin_arm_mrc(__coproc, __opc1, __CRn, __CRm, __opc2)
+
+#if __ARM_ARCH >= 5
+#define __arm_cdp2(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)              \
+  __builtin_arm_cdp2(__coproc, __opc1, __CRd, __CRn, __CRm, __opc2)
+#define __arm_ldc2(__coproc, __CRd, __p)                                       \
+  __builtin_arm_ldc2(__coproc, __CRd, __p)
+#define __arm_ldc2l(__coproc, __CRd, __p)                                      \
+  __builtin_arm_ldc2l(__coproc, __CRd, __p)
+#define __arm_stc2(__coproc, __CRd, __p)                                       \
+  __builtin_arm_stc2(__coproc, __CRd, __p)
+#define __arm_stc2l(__coproc, __CRd, __p)                                      \
+  __builtin_arm_stc2l(__coproc, __CRd, __p)
+#define __arm_mcr2(__coproc, __opc1, __value, __CRn, __CRm, __opc2)            \
+  __builtin_arm_mcr2(__coproc, __opc1, __value, __CRn, __CRm, __opc2)
+#define __arm_mrc2(__coproc, __opc1, __CRn, __CRm, __opc2)                     \
+  __builtin_arm_mrc2(__coproc, __opc1, __CRn, __CRm, __opc2)
+
+#if __ARM_ARCH >= 6 || defined(__ARM_ARCH_5TE__)
+#define __arm_mcrr(__coproc, __opc1, __value, __CRm)                           \
+  __builtin_arm_mcrr(__coproc, __opc1, __value, __CRm)
+#define __arm_mrrc(__coproc, __opc1, __CRm)                                    \
+  __builtin_arm_mrrc(__coproc, __opc1, __CRm)
+
+#if __ARM_ARCH >= 6
+#define __arm_mcrr2(__coproc, __opc1, __value, __CRm)                          \
+  __builtin_arm_mcrr2(__coproc, __opc1, __value, __CRm)
+#define __arm_mrrc2(__coproc, __opc1, __CRm)                                   \
+  __builtin_arm_mrrc2(__coproc, __opc1, __CRm)
+#endif /* __ARM_ARCH >= 6.  */
+#endif /* __ARM_ARCH >= 6 ||  defined (__ARM_ARCH_5TE__).  */
+#endif /*  __ARM_ARCH >= 5.  */
+#endif /* (!__thumb__ || __thumb2__) &&  __ARM_ARCH >= 4.  */
+
 /* Transactional Memory Extension (TME) Intrinsics */
 #if defined(__ARM_FEATURE_TME) && __ARM_FEATURE_TME

clang/lib/Headers/arm_acle.h

vhscampos · 2023-12-18T14:32:14Z

There are some typos in the commit message and in the PR title.

davemgreen · 2023-12-19T07:31:56Z

It looks like there is a downstream implementation of this that was never upstreamed. Perhaps someone can fish it out for you to show how it looked? It might be using the wrong predefined macro, but does have some tests.

hstk30-hw · 2023-12-21T14:39:32Z

I follow the gcc arm_acle.h https://github.com/gcc-mirror/gcc/blob/144c531fe25483b65ad3189d7b5e9f78154477c2/gcc/config/arm/arm_acle.h#L99C1-L239C1
code. And check the doc https://developer.arm.com/documentation/101028/latest/ .
I think It's ok to merge it.
Check it plz. @vhscampos @davemgreen

davemgreen · 2023-12-21T14:41:47Z

Let me try and get the downstream version, you might be able to pick up some things from it. A test at least should probably be present.

davemgreen · 2023-12-21T15:01:43Z

This is the downstream code we have: https://gist.github.com/davemgreen/e7ade833274a60e975e67a66eda7cb44
Note that the __ARM_TARGET_COPROC_XYZ macros are probably wrong. They should be __ARM_FEATURE_COPROC bitfield macros according to the ACLE.

Can you make use of some of that? It would be good to add the macro definition at the same time as the intrinsics (they can be used to control when the intrinsics are available), and the test should be useful for checking they are available at the right times.

github-actions · 2023-12-22T13:31:51Z

✅ With the latest revision this PR passed the C/C++ code formatter.

davemgreen · 2023-12-22T13:40:46Z

Thanks for doing this.
I think that __ARM_FEATURE_COPROC should be a bitfield, as defined in https://arm-software.github.io/acle/main/acle.html#coprocessor-intrinsics. That would remove the need for the other macros.

hstk30-hw · 2023-12-23T09:25:41Z

clang/test/CodeGen/arm-acle-coproc.c

+void ldc(int i) {
+  __arm_ldc(1, 2, &i);
+  // CHECK-V4: __builtin_arm_ldc
+  // CHECK-V4-NOT: __builtin_arm_ldc


plz check it, maybe a typo for V4-THUMB @davemgreen

hstk30-hw · 2023-12-23T09:31:18Z

Use bitfield still a mess up because the different arm version have different Instrinsics available even in the same bit group.
I will try it, hope it readable.

davemgreen

Thanks. This is looking good to me. I just have a few comments about different architecture revisions.

davemgreen · 2024-01-02T09:05:31Z

clang/lib/Basic/Targets/ARM.cpp

+  case llvm::ARM::ArchKind::ARMV9_1A:
+  case llvm::ARM::ArchKind::ARMV9_2A:
+  case llvm::ARM::ArchKind::ARMV9_3A:
+  case llvm::ARM::ArchKind::ARMV9_4A:


There is a ARMV9_5A now too. I think I would expect these to be the same as ARMV8.
Is this switch statement exhaustive? Could the default case be made the same as ARMV8 so we don't need to extend it every time an architecture is added?

We need defined ARMV9_5A in ARMTargetParser.def first.

In this https://gist.github.com/davemgreen/e7ade833274a60e975e67a66eda7cb44, not have test cases on ARMV9. According to the code logic, ARMV9 seem support all Coprocessor Instrinsics. This is different from ARMV8, I'm not sure, so the default case is 0 for now.

Oh right, ARMV9_5A is AArch64 only. That's OK then.
I would expect the other ArmV9-A cases to be the same as ArmV8-A for AArch32, and wouldn't have expected a change in coprocessor instructions.
The reference manual is at https://developer.arm.com/documentation/ddi0487/ja/?lang=en and doesn't seem to mention cdp.

clang/lib/Basic/Targets/ARM.cpp

clang/lib/Headers/arm_acle.h

davemgreen · 2024-01-07T14:18:52Z

If you can make armv9-a work the same as armv8-a and add some tests for it then this LGTM

davemgreen

Thanks. LGTM

hstk30-hw · 2024-01-09T01:05:11Z

There are some typos in the commit message and in the PR title.

@vhscampos Check again. Fixed the typos.

vhscampos

LGTM. Thanks

llvm#75424 Add Coprocessor Instrinsics

hstk30-hw requested review from rengolin, kongy, topperc and tyb-arm December 14, 2023 07:49

llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Dec 14, 2023

hstk30-hw added backend:ARM and removed backend:X86 labels Dec 16, 2023

hstk30-hw requested review from compnerd and vhscampos December 18, 2023 01:18

compnerd approved these changes Dec 18, 2023

View reviewed changes

clang/lib/Headers/arm_acle.h Outdated Show resolved Hide resolved

vhscampos requested changes Dec 18, 2023

View reviewed changes

clang/lib/Headers/arm_acle.h Outdated Show resolved Hide resolved

hstk30-hw changed the title ~~[ARM] arm_acle.h add Corprocessor Instrinsics~~ [ARM] arm_acle.h add Coprocessor Instrinsics Dec 19, 2023

hstk30-hw force-pushed the acle-add-copro-instr branch from 12fc8ec to fdb2b45 Compare December 19, 2023 01:12

llvmbot added the backend:X86 label Dec 19, 2023

hstk30-hw removed the backend:X86 label Dec 19, 2023

hstk30-hw requested a review from vhscampos December 19, 2023 14:47

hstk30-hw force-pushed the acle-add-copro-instr branch from fdb2b45 to 5a746e9 Compare December 22, 2023 13:29

llvmbot added backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 22, 2023

KanRobert removed the backend:X86 label Dec 22, 2023

hstk30-hw commented Dec 23, 2023

View reviewed changes

hstk30-hw force-pushed the acle-add-copro-instr branch from 5a746e9 to ce9db66 Compare January 1, 2024 15:34

llvmbot added the backend:X86 label Jan 1, 2024

hstk30-hw removed the backend:X86 label Jan 2, 2024

davemgreen reviewed Jan 2, 2024

View reviewed changes

hstk30-hw force-pushed the acle-add-copro-instr branch from ce9db66 to 18af0ae Compare January 6, 2024 15:13

llvmbot added the backend:X86 label Jan 6, 2024

hstk30-hw removed the backend:X86 label Jan 6, 2024

feat: arm_acle.h add Coprocessor Instrinsics

ee562f7

hstk30-hw force-pushed the acle-add-copro-instr branch from 18af0ae to ee562f7 Compare January 8, 2024 14:07

llvmbot added the backend:X86 label Jan 8, 2024

davemgreen approved these changes Jan 8, 2024

View reviewed changes

hstk30-hw removed the backend:X86 label Jan 9, 2024

vhscampos approved these changes Jan 9, 2024

View reviewed changes

hstk30-hw merged commit 4f68ee3 into llvm:main Jan 9, 2024

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024

[ARM] arm_acle.h add Coprocessor Instrinsics (llvm#75440)

6fe87bf

llvm#75424 Add Coprocessor Instrinsics

[ARM] arm_acle.h add Coprocessor Instrinsics #75440

[ARM] arm_acle.h add Coprocessor Instrinsics #75440

Uh oh!

Conversation

hstk30-hw commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 14, 2023 • edited by KanRobert Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 14, 2023

Uh oh!

Uh oh!

Uh oh!

vhscampos commented Dec 18, 2023

Uh oh!

davemgreen commented Dec 19, 2023

Uh oh!

hstk30-hw commented Dec 21, 2023

Uh oh!

davemgreen commented Dec 21, 2023

Uh oh!

davemgreen commented Dec 21, 2023

Uh oh!

github-actions bot commented Dec 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davemgreen commented Dec 22, 2023

Uh oh!

hstk30-hw Dec 23, 2023

Choose a reason for hiding this comment

Uh oh!

hstk30-hw commented Dec 23, 2023

Uh oh!

davemgreen left a comment

Choose a reason for hiding this comment

Uh oh!

davemgreen Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

hstk30-hw Jan 6, 2024

Choose a reason for hiding this comment

Uh oh!

hstk30-hw Jan 6, 2024

Choose a reason for hiding this comment

Uh oh!

davemgreen Jan 7, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davemgreen commented Jan 7, 2024

Uh oh!

davemgreen left a comment

Choose a reason for hiding this comment

Uh oh!

hstk30-hw commented Jan 9, 2024

Uh oh!

vhscampos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hstk30-hw commented Dec 14, 2023 •

edited

Loading

llvmbot commented Dec 14, 2023 •

edited by KanRobert

Loading

github-actions bot commented Dec 22, 2023 •

edited

Loading