[PATCH 0/2] Improved feature testing for ARMv6

[PATCH 0/2] Improved feature testing for ARMv6

[Andre McCurdy]

Fix two corner cases where ARM architecture specific optimizations
are not being applied consistently and/or optimally for ARMv6.

Andre McCurdy (2):
  arm: respect both __ARM_ARCH_6KZ__ and __ARM_ARCH_6ZK__ macros
  arm: enable a_ll and a_sc helper functions when building for ARMv6T2

 arch/arm/atomic_arch.h  | 4 ++--
 arch/arm/pthread_arch.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH 1/2] arm: respect both __ARM_ARCH_6KZ__ and __ARM_ARCH_6ZK__ macros

[Andre McCurdy]

__ARM_ARCH_6ZK__ is a gcc specific historical typo which may not be
defined by other compilers.

  https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02237.html

To avoid unexpected results when building for ARMv6KZ with clang, the
correct form of the macro (ie 6KZ) needs to be tested. The incorrect
form of the macro (ie 6ZK) still needs to be tested for compatibility
with pre-2015 versions of gcc.
---
 arch/arm/atomic_arch.h  | 2 +-
 arch/arm/pthread_arch.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
index 72fcddb..5ff1be1 100644
--- a/arch/arm/atomic_arch.h
+++ b/arch/arm/atomic_arch.h
@@ -7,7 +7,7 @@
 extern uintptr_t __attribute__((__visibility__("hidden")))
 	__a_cas_ptr, __a_barrier_ptr;
 
-#if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6ZK__) && !__thumb__) \
+#if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
  || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
 
 #define a_ll a_ll
diff --git a/arch/arm/pthread_arch.h b/arch/arm/pthread_arch.h
index 197752e..6657e19 100644
--- a/arch/arm/pthread_arch.h
+++ b/arch/arm/pthread_arch.h
@@ -1,4 +1,4 @@
-#if ((__ARM_ARCH_6K__ || __ARM_ARCH_6ZK__) && !__thumb__) \
+#if ((__ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
  || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
 
 static inline pthread_t __pthread_self()
-- 
1.9.1

[PATCH 2/2] arm: enable a_ll and a_sc helper functions when building for ARMv6T2

[Andre McCurdy]

ARMv6 cores with support for Thumb2 can take advantage of the "ldrex"
and "strex" based implementations of a_ll and a_sc.
---
 arch/arm/atomic_arch.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
index 5ff1be1..62458b4 100644
--- a/arch/arm/atomic_arch.h
+++ b/arch/arm/atomic_arch.h
@@ -8,7 +8,7 @@ extern uintptr_t __attribute__((__visibility__("hidden")))
 	__a_cas_ptr, __a_barrier_ptr;
 
 #if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
- || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
+ || __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
 
 #define a_ll a_ll
 static inline int a_ll(volatile int *p)
-- 
1.9.1

Re: [PATCH 2/2] arm: enable a_ll and a_sc helper functions when building for ARMv6T2

[Rich Felker]

On Wed, Apr 18, 2018 at 06:51:44PM -0700, Andre McCurdy wrote:
> ARMv6 cores with support for Thumb2 can take advantage of the "ldrex"
> and "strex" based implementations of a_ll and a_sc.
> ---
>  arch/arm/atomic_arch.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
> index 5ff1be1..62458b4 100644
> --- a/arch/arm/atomic_arch.h
> +++ b/arch/arm/atomic_arch.h
> @@ -8,7 +8,7 @@ extern uintptr_t __attribute__((__visibility__("hidden")))
>  	__a_cas_ptr, __a_barrier_ptr;
>  
>  #if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
> - || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
> + || __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
>  
>  #define a_ll a_ll
>  static inline int a_ll(volatile int *p)

I'm merging this along with the others, but there is some concern that
our use of a_ll/a_sc might not actually be valid on most or all of the
archs we currently use it on. Depending on how this turns out it might
all be removed at some later time.

Rich

Re: [PATCH 2/2] arm: enable a_ll and a_sc helper functions when building for ARMv6T2

[Andre McCurdy]

On Thu, Apr 19, 2018 at 9:38 AM, Rich Felker <dalias@libc.org> wrote:
> On Wed, Apr 18, 2018 at 06:51:44PM -0700, Andre McCurdy wrote:
>> ARMv6 cores with support for Thumb2 can take advantage of the "ldrex"
>> and "strex" based implementations of a_ll and a_sc.
>> ---
>>  arch/arm/atomic_arch.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
>> index 5ff1be1..62458b4 100644
>> --- a/arch/arm/atomic_arch.h
>> +++ b/arch/arm/atomic_arch.h
>> @@ -8,7 +8,7 @@ extern uintptr_t __attribute__((__visibility__("hidden")))
>>       __a_cas_ptr, __a_barrier_ptr;
>>
>>  #if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
>> - || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
>> + || __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
>>
>>  #define a_ll a_ll
>>  static inline int a_ll(volatile int *p)
>
> I'm merging this along with the others, but there is some concern that
> our use of a_ll/a_sc might not actually be valid on most or all of the
> archs we currently use it on. Depending on how this turns out it might
> all be removed at some later time.

That sound ominous. What's the concern?

Re: [PATCH 2/2] arm: enable a_ll and a_sc helper functions when building for ARMv6T2

[Rich Felker]

On Thu, Apr 19, 2018 at 12:14:51PM -0700, Andre McCurdy wrote:
> On Thu, Apr 19, 2018 at 9:38 AM, Rich Felker <dalias@libc.org> wrote:
> > On Wed, Apr 18, 2018 at 06:51:44PM -0700, Andre McCurdy wrote:
> >> ARMv6 cores with support for Thumb2 can take advantage of the "ldrex"
> >> and "strex" based implementations of a_ll and a_sc.
> >> ---
> >>  arch/arm/atomic_arch.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
> >> index 5ff1be1..62458b4 100644
> >> --- a/arch/arm/atomic_arch.h
> >> +++ b/arch/arm/atomic_arch.h
> >> @@ -8,7 +8,7 @@ extern uintptr_t __attribute__((__visibility__("hidden")))
> >>       __a_cas_ptr, __a_barrier_ptr;
> >>
> >>  #if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
> >> - || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
> >> + || __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
> >>
> >>  #define a_ll a_ll
> >>  static inline int a_ll(volatile int *p)
> >
> > I'm merging this along with the others, but there is some concern that
> > our use of a_ll/a_sc might not actually be valid on most or all of the
> > archs we currently use it on. Depending on how this turns out it might
> > all be removed at some later time.
> 
> That sound ominous. What's the concern?

Originally ARM didn't document it, but reportedly it's now documented
somewhere that the ll and sc operations have certain strong conditions
on how they're used. RISC-V's and maybe other archs' also have similar
conditions. They're something along the lines of (from my memory):

- no intervening loads or stores between ll and sc
- limit on number of instructions between ll and sc
- no jumps or branches between ll and sc

and there is no way to guarantee these kinds of conditions when the
compiler is free to move the ll and sc asm blocks independently.

From a practical standpoint, it looks like the conditions are overly
conservative and designed to allow cpu implementations with very bad
cache designs (direct-mapped, small, etc.) but they may turn out to be
relevant so we need to evaluate if we need to care about this...

Rich

Re: [PATCH 2/2] arm: enable a_ll and a_sc helper functions when building for ARMv6T2

[Andre McCurdy]

On Thu, Apr 19, 2018 at 12:25 PM, Rich Felker <dalias@libc.org> wrote:
> On Thu, Apr 19, 2018 at 12:14:51PM -0700, Andre McCurdy wrote:
>> On Thu, Apr 19, 2018 at 9:38 AM, Rich Felker <dalias@libc.org> wrote:
>> > On Wed, Apr 18, 2018 at 06:51:44PM -0700, Andre McCurdy wrote:
>> >> ARMv6 cores with support for Thumb2 can take advantage of the "ldrex"
>> >> and "strex" based implementations of a_ll and a_sc.
>> >> ---
>> >>  arch/arm/atomic_arch.h | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
>> >> index 5ff1be1..62458b4 100644
>> >> --- a/arch/arm/atomic_arch.h
>> >> +++ b/arch/arm/atomic_arch.h
>> >> @@ -8,7 +8,7 @@ extern uintptr_t __attribute__((__visibility__("hidden")))
>> >>       __a_cas_ptr, __a_barrier_ptr;
>> >>
>> >>  #if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
>> >> - || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
>> >> + || __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
>> >>
>> >>  #define a_ll a_ll
>> >>  static inline int a_ll(volatile int *p)
>> >
>> > I'm merging this along with the others, but there is some concern that
>> > our use of a_ll/a_sc might not actually be valid on most or all of the
>> > archs we currently use it on. Depending on how this turns out it might
>> > all be removed at some later time.
>>
>> That sound ominous. What's the concern?
>
> Originally ARM didn't document it, but reportedly it's now documented
> somewhere that the ll and sc operations have certain strong conditions
> on how they're used. RISC-V's and maybe other archs' also have similar
> conditions. They're something along the lines of (from my memory):
>
> - no intervening loads or stores between ll and sc
> - limit on number of instructions between ll and sc
> - no jumps or branches between ll and sc
>
> and there is no way to guarantee these kinds of conditions when the
> compiler is free to move the ll and sc asm blocks independently.
>
> From a practical standpoint, it looks like the conditions are overly
> conservative and designed to allow cpu implementations with very bad
> cache designs (direct-mapped, small, etc.) but they may turn out to be
> relevant so we need to evaluate if we need to care about this...

Thanks. Google found the following:

  https://stackoverflow.com/questions/10812442/arm-ll-sc-exclusive-access-by-register-width-or-cache-line-width
  http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dht0008a/CJAGCFAF.html

It takes some digesting, but I don't see an immediate red flag.

An instruction sequence which Interleaves two ll sc sequences would
seem to be an issue, but we don't do that.

A uniprocessor system running without data caching could be problem
(implementation dependent). But if I read correctly, the failure mode
would be that sc would always fail and therefore the system would
deadlock, so it won't be a subtle failure?

Re: [PATCH 2/2] arm: enable a_ll and a_sc helper functions when building for ARMv6T2

[Rich Felker]

On Thu, Apr 19, 2018 at 01:38:51PM -0700, Andre McCurdy wrote:
> On Thu, Apr 19, 2018 at 12:25 PM, Rich Felker <dalias@libc.org> wrote:
> > On Thu, Apr 19, 2018 at 12:14:51PM -0700, Andre McCurdy wrote:
> >> On Thu, Apr 19, 2018 at 9:38 AM, Rich Felker <dalias@libc.org> wrote:
> >> > On Wed, Apr 18, 2018 at 06:51:44PM -0700, Andre McCurdy wrote:
> >> >> ARMv6 cores with support for Thumb2 can take advantage of the "ldrex"
> >> >> and "strex" based implementations of a_ll and a_sc.
> >> >> ---
> >> >>  arch/arm/atomic_arch.h | 2 +-
> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/arch/arm/atomic_arch.h b/arch/arm/atomic_arch.h
> >> >> index 5ff1be1..62458b4 100644
> >> >> --- a/arch/arm/atomic_arch.h
> >> >> +++ b/arch/arm/atomic_arch.h
> >> >> @@ -8,7 +8,7 @@ extern uintptr_t __attribute__((__visibility__("hidden")))
> >> >>       __a_cas_ptr, __a_barrier_ptr;
> >> >>
> >> >>  #if ((__ARM_ARCH_6__ || __ARM_ARCH_6K__ || __ARM_ARCH_6KZ__ || __ARM_ARCH_6ZK__) && !__thumb__) \
> >> >> - || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ ||  __ARM_ARCH >= 7
> >> >> + || __ARM_ARCH_6T2__ || __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
> >> >>
> >> >>  #define a_ll a_ll
> >> >>  static inline int a_ll(volatile int *p)
> >> >
> >> > I'm merging this along with the others, but there is some concern that
> >> > our use of a_ll/a_sc might not actually be valid on most or all of the
> >> > archs we currently use it on. Depending on how this turns out it might
> >> > all be removed at some later time.
> >>
> >> That sound ominous. What's the concern?
> >
> > Originally ARM didn't document it, but reportedly it's now documented
> > somewhere that the ll and sc operations have certain strong conditions
> > on how they're used. RISC-V's and maybe other archs' also have similar
> > conditions. They're something along the lines of (from my memory):
> >
> > - no intervening loads or stores between ll and sc
> > - limit on number of instructions between ll and sc
> > - no jumps or branches between ll and sc
> >
> > and there is no way to guarantee these kinds of conditions when the
> > compiler is free to move the ll and sc asm blocks independently.
> >
> > From a practical standpoint, it looks like the conditions are overly
> > conservative and designed to allow cpu implementations with very bad
> > cache designs (direct-mapped, small, etc.) but they may turn out to be
> > relevant so we need to evaluate if we need to care about this...
> 
> Thanks. Google found the following:
> 
>   https://stackoverflow.com/questions/10812442/arm-ll-sc-exclusive-access-by-register-width-or-cache-line-width
>   http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dht0008a/CJAGCFAF.html
> 
> It takes some digesting, but I don't see an immediate red flag.
> 
> An instruction sequence which Interleaves two ll sc sequences would
> seem to be an issue, but we don't do that.
> 
> A uniprocessor system running without data caching could be problem
> (implementation dependent). But if I read correctly, the failure mode
> would be that sc would always fail and therefore the system would
> deadlock, so it won't be a subtle failure?

That sounds right.

Rich