Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][Clang][Headers] Update refs to ACLE in comments #66662

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Blue-Dot
Copy link
Contributor

Non functional change to update section comments in arm_acle.h, in order to align with updated documentation: https://arm-software.github.io/acle/main/acle.html

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Sep 18, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 18, 2023

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Changes

Non functional change to update section comments in arm_acle.h, in order to align with updated documentation: https://arm-software.github.io/acle/main/acle.html


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

1 Files Affected:

  • (modified) clang/lib/Headers/arm_acle.h (+30-31)
diff --git a/clang/lib/Headers/arm_acle.h b/clang/lib/Headers/arm_acle.h
index 61d80258d166a1d..aed789863f29a2c 100644
--- a/clang/lib/Headers/arm_acle.h
+++ b/clang/lib/Headers/arm_acle.h
@@ -20,8 +20,8 @@
 extern "C" {
 #endif
 
-/* 8 SYNCHRONIZATION, BARRIER AND HINT INTRINSICS */
-/* 8.3 Memory barriers */
+/* 7 SYNCHRONIZATION, BARRIER AND HINT INTRINSICS */
+/* 7.3 Memory barriers */
 #if !__has_builtin(__dmb)
 #define __dmb(i) __builtin_arm_dmb(i)
 #endif
@@ -32,7 +32,7 @@ extern "C" {
 #define __isb(i) __builtin_arm_isb(i)
 #endif
 
-/* 8.4 Hints */
+/* 7.4 Hints */
 
 #if !__has_builtin(__wfi)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) __wfi(void) {
@@ -68,7 +68,7 @@ static __inline__ void __attribute__((__always_inline__, __nodebug__)) __yield(v
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
-/* 8.5 Swap */
+/* 7.5 Swap */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __swp(uint32_t __x, volatile uint32_t *__p) {
   uint32_t v;
@@ -78,8 +78,8 @@ __swp(uint32_t __x, volatile uint32_t *__p) {
   return v;
 }
 
-/* 8.6 Memory prefetch intrinsics */
-/* 8.6.1 Data prefetch */
+/* 7.6 Memory prefetch intrinsics */
+/* 7.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
 #if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
@@ -90,7 +90,7 @@ __swp(uint32_t __x, volatile uint32_t *__p) {
   __builtin_arm_prefetch(addr, access_kind, cache_level, retention_policy, 1)
 #endif
 
-/* 8.6.2 Instruction prefetch */
+/* 7.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
 #if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
@@ -101,15 +101,15 @@ __swp(uint32_t __x, volatile uint32_t *__p) {
   __builtin_arm_prefetch(addr, 0, cache_level, retention_policy, 0)
 #endif
 
-/* 8.7 NOP */
+/* 7.7 NOP */
 #if !defined(_MSC_VER) || !defined(__aarch64__)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) __nop(void) {
   __builtin_arm_nop();
 }
 #endif
 
-/* 9 DATA-PROCESSING INTRINSICS */
-/* 9.2 Miscellaneous data-processing intrinsics */
+/* 8 DATA-PROCESSING INTRINSICS */
+/* 8.2 Miscellaneous data-processing intrinsics */
 /* ROR */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __ror(uint32_t __x, uint32_t __y) {
@@ -248,9 +248,7 @@ __rbitl(unsigned long __t) {
 #endif
 }
 
-/*
- * 9.3 16-bit multiplications
- */
+/* 8.3 16-bit multiplications */
 #if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
@@ -279,18 +277,18 @@ __smulwt(int32_t __a, int32_t __b) {
 #endif
 
 /*
- * 9.4 Saturating intrinsics
+ * 8.4 Saturating intrinsics
  *
  * FIXME: Change guard to their corresponding __ARM_FEATURE flag when Q flag
  * intrinsics are implemented and the flag is enabled.
  */
-/* 9.4.1 Width-specified saturation intrinsics */
+/* 8.4.1 Width-specified saturation intrinsics */
 #if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
-/* 9.4.2 Saturating addition and subtraction intrinsics */
+/* 8.4.2 Saturating addition and subtraction intrinsics */
 #if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
@@ -308,7 +306,7 @@ __qdbl(int32_t __t) {
 }
 #endif
 
-/* 9.4.3 Accumultating multiplications */
+/* 8.4.3 Accumultating multiplications */
 #if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
@@ -337,13 +335,13 @@ __smlawt(int32_t __a, int32_t __b, int32_t __c) {
 #endif
 
 
-/* 9.5.4 Parallel 16-bit saturation */
+/* 8.5.4 Parallel 16-bit saturation */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
-/* 9.5.5 Packing and unpacking */
+/* 8.5.5 Packing and unpacking */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
@@ -368,7 +366,7 @@ __uxtb16(int8x4_t __a) {
 }
 #endif
 
-/* 9.5.6 Parallel selection */
+/* 8.5.6 Parallel selection */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
@@ -376,7 +374,7 @@ __sel(uint8x4_t __a, uint8x4_t __b) {
 }
 #endif
 
-/* 9.5.7 Parallel 8-bit addition and subtraction */
+/* 8.5.7 Parallel 8-bit addition and subtraction */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t __attribute__((__always_inline__, __nodebug__))
 __qadd8(int8x4_t __a, int8x4_t __b) {
@@ -428,7 +426,7 @@ __usub8(uint8x4_t __a, uint8x4_t __b) {
 }
 #endif
 
-/* 9.5.8 Sum of 8-bit absolute differences */
+/* 8.5.8 Sum of 8-bit absolute differences */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __usad8(uint8x4_t __a, uint8x4_t __b) {
@@ -440,7 +438,7 @@ __usada8(uint8x4_t __a, uint8x4_t __b, uint32_t __c) {
 }
 #endif
 
-/* 9.5.9 Parallel 16-bit addition and subtraction */
+/* 8.5.9 Parallel 16-bit addition and subtraction */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int16x2_t __attribute__((__always_inline__, __nodebug__))
 __qadd16(int16x2_t __a, int16x2_t __b) {
@@ -540,7 +538,7 @@ __usub16(uint16x2_t __a, uint16x2_t __b) {
 }
 #endif
 
-/* 9.5.10 Parallel 16-bit multiplications */
+/* 8.5.10 Parallel 16-bit multiplications */
 #if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlad(int16x2_t __a, int16x2_t __b, int32_t __c) {
@@ -607,7 +605,7 @@ __rintnf(float __a) {
 }
 #endif
 
-/* 9.7 CRC32 intrinsics */
+/* 8.8 CRC32 intrinsics */
 #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")))
@@ -651,6 +649,7 @@ __crc32cd(uint32_t __a, uint64_t __b) {
 }
 #endif
 
+/* 8.6 Floating-point data-processing intrinsics */
 /* Armv8.3-A Javascript conversion intrinsic */
 #if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__, target("v8.3a")))
@@ -702,7 +701,7 @@ __rint64x(double __a) {
 }
 #endif
 
-/* Armv8.7-A load/store 64-byte intrinsics */
+/* 8.9 Armv8.7-A load/store 64-byte intrinsics */
 #if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
 typedef struct {
     uint64_t val[8];
@@ -728,7 +727,7 @@ __arm_st64bv0(void *__addr, data512_t __value) {
 }
 #endif
 
-/* 10.1 Special register intrinsics */
+/* 11.1 Special register intrinsics */
 #define __arm_rsr(sysreg) __builtin_arm_rsr(sysreg)
 #define __arm_rsr64(sysreg) __builtin_arm_rsr64(sysreg)
 #define __arm_rsr128(sysreg) __builtin_arm_rsr128(sysreg)
@@ -742,7 +741,7 @@ __arm_st64bv0(void *__addr, data512_t __value) {
 #define __arm_wsrf(sysreg, v) __arm_wsr(sysreg, __builtin_bit_cast(uint32_t, v))
 #define __arm_wsrf64(sysreg, v) __arm_wsr64(sysreg, __builtin_bit_cast(uint64_t, v))
 
-/* Memory Tagging Extensions (MTE) Intrinsics */
+/* 10.3 Memory Tagging Extensions (MTE) Intrinsics */
 #if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
 #define __arm_mte_create_random_tag(__ptr, __mask)  __builtin_arm_irg(__ptr, __mask)
 #define __arm_mte_increment_tag(__ptr, __tag_offset)  __builtin_arm_addg(__ptr, __tag_offset)
@@ -751,12 +750,12 @@ __arm_st64bv0(void *__addr, data512_t __value) {
 #define __arm_mte_set_tag(__ptr) __builtin_arm_stg(__ptr)
 #define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)
 
-/* Memory Operations Intrinsics */
+/* 18 Memory Operations Intrinsics */
 #define __arm_mops_memset_tag(__tagged_address, __value, __size)    \
   __builtin_arm_mops_memset_tag(__tagged_address, __value, __size)
 #endif
 
-/* Transactional Memory Extension (TME) Intrinsics */
+/* 17 Transactional Memory Extension (TME) Intrinsics */
 #if defined(__ARM_FEATURE_TME) && __ARM_FEATURE_TME
 
 #define _TMFAILURE_REASON  0x00007fffu
@@ -778,7 +777,7 @@ __arm_st64bv0(void *__addr, data512_t __value) {
 
 #endif /* __ARM_FEATURE_TME */
 
-/* Armv8.5-A Random number generation intrinsics */
+/* 8.7 Armv8.5-A Random number generation intrinsics */
 #if defined(__ARM_64BIT_STATE) && __ARM_64BIT_STATE
 static __inline__ int __attribute__((__always_inline__, __nodebug__, target("rand")))
 __rndr(uint64_t *__p) {

@@ -20,8 +20,8 @@
extern "C" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all of these updates look correct to me.
Since these all refer to sections in a document, I think it may be useful too to add a hyperlink at the top of the document to where the ACLE lives.
I think the correct hyperlink would be https://github.com/ARM-software/acle/releases/.

@vhscampos Do you think that would be a good idea too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Blue-Dot for the patch. I agree with @kbeyls that it would be a good idea.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to note that section numbers were correct at time of writing, if they've changed then look for the section title.

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these section refs weren't already here I'd suggest not adding numbers at all given that they will change later, and Firefox and Chrome seem hopeless at searching for them in the HTML document.

But we have them already so I won't suggest that :) Thanks for taking the time to do this.

@@ -32,7 +32,7 @@ extern "C" {
#define __isb(i) __builtin_arm_isb(i)
#endif

/* 8.4 Hints */
/* 7.4 Hints */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're matching the section titles then they should have the extra . on the end of the number, so here 7.4. Hints. Just to make searching more obvious.

Though in Firefox and Chrome they struggled to find it but that's down to the HTML formatting I think. At least visually it'll match up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTML version has the extra . https://arm-software.github.io/acle/main/acle.html#hints but the PDF one does not https://github.com/ARM-software/acle/releases/download/r2023Q2/acle-2023Q2.pdf - the solution, probably, depends on what is linked as suggested in a comment above: HTML or PDF version.

@vhscampos
Copy link
Member

I've addressed your comments, but I don't have permissions to edit this PR directly. Thus I've created a new pull request: #78305

Can you please have a look? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants