[compiler-rt][builtins] Do not use ldrexd or strexd on ARMv6

[benwolsieffer]

Summary

The ldrexd and strexd instructions are not available on base ARMv6, and were
only added in ARMv6K (see [1]). This patch solves this problem once and for all
using the __ARM_FEATURE_LDREX macro (see [2]) defined in the ARM C Language
Extensions (ACLE). Although this macro is technically deprecated in the ACLE,
it allows compiler-rt to reliably detect whether ldrexd and strexd are supported
without complicated conditionals to detect different ARM architecture variants.

[1] https://developer.arm.com/documentation/dht0008/a/ch01s02s01
[2] https://arm-software.github.io/acle/main/acle.html#ldrexstrex

[compnerd]

Might be useful to add a reference to https://developer.arm.com/documentation/dht0008/a/ch01s02s01 in the commit message.

I'm worried about the current condition though as what we really want is to ensure that the ARM architecture is 7 or newer and that not in the M profile. What do you think of using:

__ARM_ARCH > 7 || (__ARM_ARCH == 7 && __ARM_ARCH_PROFILE != 'M')

instead?

[benwolsieffer]

That's not quite right though, since that excludes ARMv6K and ARMv6Z (which is a superset of ARMv6K). The only ARMv6 variant that doesn't support these instructions is base ARMv6 (and technically ARMv6J, but LLVM doesn't appear to consider that as a separate target). __ARM_ARCH_6__ is defined only when targeting base ARMv6, not any of the other variants.

According to https://reviews.llvm.org/D95891, these instruction are not supported in ARMv8-M either, so we don't want to use __ARM_ARCH > 7.

[benwolsieffer]

I updated the commit message to include the reference to the ARM documentation.

[compnerd]

In D139585#3982211, @benwolsieffer wrote:

That's not quite right though, since that excludes ARMv6K and ARMv6Z (which is a superset of ARMv6K). The only ARMv6 variant that doesn't support these instructions is base ARMv6 (and technically ARMv6J, but LLVM doesn't appear to consider that as a separate target). __ARM_ARCH_6__ is defined only when targeting base ARMv6, not any of the other variants.

According to https://reviews.llvm.org/D95891, these instruction are not supported in ARMv8-M either, so we don't want to use __ARM_ARCH > 7.

Doesn't the current check not exclude those variants as well? __ARM_ARCH_6__ should be defined when targeting the ARMv6{K,Z} targets right? I think that this is complicated enough that we should define this centrally and use that to guard the operations.

[benwolsieffer]

No, __ARM_ARCH_6__ is only defined on base ARMv6. __ARM_ARCH_6K__ is defined instead on ARMv6K. I can't find this clearly documented anywhere, but I have confirmed this behavior through experimentation with GCC and by examination with clang (see implementation here)

On the other hand, there is the __ARM_FEATURE_LDREX macro, which provides the exact functionality we want here. The only problem is that it is deprecated in the ARM C Language Extensions specification, although it is still defined even in the latest versions of the ACLE. This macro was not used in https://reviews.llvm.org/D5490 in part because it was deprecated, but mostly because clang did not implement it at the time.

What do you think about using that macro instead, despite the deprecation?

[compnerd]

In D139585#3985179, @benwolsieffer wrote:

No, __ARM_ARCH_6__ is only defined on base ARMv6. __ARM_ARCH_6K__ is defined instead on ARMv6K. I can't find this clearly documented anywhere, but I have confirmed this behavior through experimentation with GCC and by examination with clang (see implementation here)

On the other hand, there is the __ARM_FEATURE_LDREX macro, which provides the exact functionality we want here. The only problem is that it is deprecated in the ARM C Language Extensions specification, although it is still defined even in the latest versions of the ACLE. This macro was not used in https://reviews.llvm.org/D5490 in part because it was deprecated, but mostly because clang did not implement it at the time.

What do you think about using that macro instead, despite the deprecation?

I think that is a much clearer macro for this. While it is deprecated, this is an internal reference, so should we ever drop support for it, we should catch it pretty quickly I think. I think that going with that is reasonable. Thanks for the reference, and walking me through this.

[benwolsieffer]

I updated the patch to use __ARM_FEATURE_LDREX.

[compnerd]

Thanks! This looks good. Personally, I'd use 0x8 instead of 8, which might make it more obvious that this is intentional (and that it is a bitmask).

[benwolsieffer]

Rebase on main

[benwolsieffer]

Sorry I went silent for so long. I updated the patch to use 0x8. Also, I don't have commit access, so I need someone else to commit this.

compiler-rt/lib/builtins/arm/sync_fetch_and_add_8.S

 //===-- sync_fetch_and_add_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_add_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define add_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
     adds rD_LO, rN_LO, rM_LO ; \
     adc rD_HI, rN_HI, rM_HI
 
 SYNC_OP_8(add_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_and_8.S

 //===-- sync_fetch_and_and_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_and_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define and_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
     and rD_LO, rN_LO, rM_LO ; \
     and rD_HI, rN_HI, rM_HI
 
 SYNC_OP_8(and_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_max_8.S

 //===-- sync_fetch_and_max_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_max_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define max_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI)         MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, gt)
 
 SYNC_OP_8(max_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_min_8.S

 //===-- sync_fetch_and_min_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_min_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define min_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI)         MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, lt)
 
 SYNC_OP_8(min_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_nand_8.S

 //===-- sync_fetch_and_nand_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_nand_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define nand_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
     bic rD_LO, rN_LO, rM_LO ; \
     bic rD_HI, rN_HI, rM_HI
 
 SYNC_OP_8(nand_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_or_8.S

 //===-- sync_fetch_and_or_8.S - -------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_or_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define or_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
     orr rD_LO, rN_LO, rM_LO ; \
     orr rD_HI, rN_HI, rM_HI
 
 SYNC_OP_8(or_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_sub_8.S

 //===-- sync_fetch_and_sub_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_sub_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define sub_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
     subs rD_LO, rN_LO, rM_LO ; \
     sbc rD_HI, rN_HI, rM_HI
 
 SYNC_OP_8(sub_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_umax_8.S

 //===-- sync_fetch_and_umax_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_umax_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define umax_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI)         MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, hi)
 
 SYNC_OP_8(umax_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_umin_8.S

 //===-- sync_fetch_and_umin_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_umin_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define umin_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI)         MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, lo)
 
 SYNC_OP_8(umin_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 

compiler-rt/lib/builtins/arm/sync_fetch_and_xor_8.S

 //===-- sync_fetch_and_xor_8.S - ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the __sync_fetch_and_xor_8 function for the ARM
 // architecture.
 //
 //===----------------------------------------------------------------------===//
 
 #include "sync-ops.h"
 
-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 0x8
 #define xor_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
     eor rD_LO, rN_LO, rM_LO ; \
     eor rD_HI, rN_HI, rM_HI
 
 SYNC_OP_8(xor_8)
 #endif
 
 NO_EXEC_STACK_DIRECTIVE