Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Thu, 2017-12-07 at 09:56 +, James Greenhalgh wrote:
> 
> One obvious thing I missed in the review is that this change will break
> bootstrap on systems with older assemblers. Practically, that's those of
> us who are holding out on Ubuntu 14.04. -march=armv8-a+lse would go back
> a little further, so would be preferable, but even this won't get bootstrap
> back on older systems.
> 
> Is there anything you can do to check for assembler support before turning
> on IFUNCS for libatomic, or am I best to just start configuring with
> --disable-gnu-indirect-function ?
> 
> Thanks,
> James

It should be possible to check for assembler support.  I will work on a
patch to do that.

Steve Ellcey
sell...@cavium.com

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[James Greenhalgh]

On Fri, Sep 29, 2017 at 09:29:37PM +0100, Steve Ellcey wrote:
> On Thu, 2017-09-28 at 12:31 +0100, Szabolcs Nagy wrote:
> > 
> > i think this should be improved, see below.
> 
> diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
> index d731406..92d19c6 100644
> --- a/libatomic/Makefile.am
> +++ b/libatomic/Makefile.am
> @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix 
> _$(s)_.lo,$(SIZEOBJS)))
>  
>  ## On a target-specific basis, include alternates to be selected by IFUNC.
>  if HAVE_IFUNC
> +if ARCH_AARCH64_LINUX
> +IFUNC_OPTIONS = -march=armv8.1-a
> +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix 
> _$(s)_1_.lo,$(SIZEOBJS)))
> +endif
>  if ARCH_ARM_LINUX
>  IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64
>  libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix 
> _$(s)_1_.lo,$(SIZEOBJS)))

One obvious thing I missed in the review is that this change will break
bootstrap on systems with older assemblers. Practically, that's those of
us who are holding out on Ubuntu 14.04. -march=armv8-a+lse would go back
a little further, so would be preferable, but even this won't get bootstrap
back on older systems.

Is there anything you can do to check for assembler support before turning
on IFUNCS for libatomic, or am I best to just start configuring with
--disable-gnu-indirect-function ?

Thanks,
James

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

FYI: Since James approved the Aarch64 part and since I think the
generic part can be considered trivial, I have gone ahead and checked
this patch in.

Steve Ellcey
sell...@cavium.com

On Tue, 2017-11-28 at 16:12 -0800, Steve Ellcey wrote:
> On Tue, 2017-11-21 at 17:35 +, James Greenhalgh wrote:
> 
> > 
> > 
> > Thanks for the detailed explanation. I understood this, and my
> > opinion is
> > that the AArch64 parts of this patch are OK (and I don't know who
> > needs to
> > Ack the small generic changes you require).
> > 
> > Let's give Richard/Marcus 48 hours to object while we wait for an
> > OK on the
> > generic bits, and then OK for AArch64.
> > 
> > Thanks,
> > James
> > 
> > Reviewed-By: James Greenhalgh 
> James, I haven't seen anything from Richard or Marcus.  Do you think
> a
> review from a global maintainer is really needed?  There is no
> libatomic specific maintainer.  The only non-aarch64 specific change
> is using the macro IFUNC_RESOLVER_ARGS in place of the hardcoded
> 'void'
> argument for ifunc selector functions and for all non-aarch64
> platforms
> the macro will be defined as 'void' so there is no real change for
> any
> other platform.
> 
> Steve Ellcey
> sell...@cavium.com

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Tue, 2017-11-21 at 17:35 +, James Greenhalgh wrote:

> 
> Thanks for the detailed explanation. I understood this, and my opinion is
> that the AArch64 parts of this patch are OK (and I don't know who needs to
> Ack the small generic changes you require).
> 
> Let's give Richard/Marcus 48 hours to object while we wait for an OK on the
> generic bits, and then OK for AArch64.
> 
> Thanks,
> James
> 
> Reviewed-By: James Greenhalgh 

James, I haven't seen anything from Richard or Marcus.  Do you think a
review from a global maintainer is really needed?  There is no
libatomic specific maintainer.  The only non-aarch64 specific change
is using the macro IFUNC_RESOLVER_ARGS in place of the hardcoded 'void'
argument for ifunc selector functions and for all non-aarch64 platforms
the macro will be defined as 'void' so there is no real change for any
other platform.

Steve Ellcey
sell...@cavium.com

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[James Greenhalgh]

On Mon, Nov 20, 2017 at 07:22:15PM +, Steve Ellcey wrote:
> On Mon, 2017-11-20 at 18:27 +, James Greenhalgh wrote:
> > 
> > If you have the time, would you mind giving me a quick run-down of what
> > design decisions went in to this patch, and why they are the right thing
> > to do? Sorry to offload that, but it will be the most efficient route
> > to a review.
> 
> The main design decision was to use the existing IFUNC infrastructure
> that is used on ARM32 to enable atomic instructions that were added
> with armv7-a, on i386 to enable instructions added with i586, and on
> x86_64 to enable instructions added with cx16.
> 
> The basic idea for all these is to allow users who create programs that
> use the atomic_* functions to use new instructions on machines that
> support them while also working on older machines that do not support
> them and to not have to create two separate executables.
> 
> Some atomic_* functions get inlined into programs, and those will
> either use or not use LSE instructions based on the compiler arguments
> used during compilations.  If you want your program to work on all
> machines you have to not compile for LSE intructions.  But other
> functions (or all functions if -fno-inline-atomics is used) will call
> the libatomic library.  Currently those functions do not use LSE
> instructions but with this patch we can use the IFUNC infrastructure to
> check for LSE support and use LSE in libatomic on machines where it is
> supported or not use it on machines where it is not supported.
> 
> As an example of what this change does, __atomic_compare_exchange_8 will
> turn into a call to libat_compare_exchange_8_i1 on a machine that supports
> LSE:
> 
>  :
>    0: f9400023    ldr x3, [x1]
>    4: aa0303e4    mov x4, x3
>    8: c8e4fc02    casal   x4, x2, [x0]
>    c: eb03009f    cmp x4, x3
>   10: 1a9f17e0    csetw0, eq
>   14: 3540    cbnzw0, 1c 
>   18: f924    str x4, [x1]
>   1c: d65f03c0    ret
> 
> But call libat_compare_exchange_8 on a machine without LSE:
> 
>  :
>    0: f9400023    ldr x3, [x1]
>    4: c85ffc04    ldaxr   x4, [x0]
>    8: eb03009f    cmp x4, x3
>    c: 5461    b.ne18 
>   10: c805fc02    stlxr   w5, x2, [x0]
>   14: 3585    cbnzw5, 4 
>   18: 1a9f17e0    csetw0, eq
>   1c: 3440    cbz w0, 24 
>   20: d65f03c0    ret
>   24: f924    str x4, [x1]
>   28: d65f03c0    ret
>   2c: d503201f    nop

Thanks for the detailed explanation. I understood this, and my opinion is
that the AArch64 parts of this patch are OK (and I don't know who needs to
Ack the small generic changes you require).

Let's give Richard/Marcus 48 hours to object while we wait for an OK on the
generic bits, and then OK for AArch64.

Thanks,
James

Reviewed-By: James Greenhalgh 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Mon, 2017-11-20 at 18:27 +, James Greenhalgh wrote:
> 
> If you have the time, would you mind giving me a quick run-down of what
> design decisions went in to this patch, and why they are the right thing
> to do? Sorry to offload that, but it will be the most efficient route
> to a review.

The main design decision was to use the existing IFUNC infrastructure
that is used on ARM32 to enable atomic instructions that were added
with armv7-a, on i386 to enable instructions added with i586, and on
x86_64 to enable instructions added with cx16.

The basic idea for all these is to allow users who create programs that
use the atomic_* functions to use new instructions on machines that
support them while also working on older machines that do not support
them and to not have to create two separate executables.

Some atomic_* functions get inlined into programs, and those will
either use or not use LSE instructions based on the compiler arguments
used during compilations.  If you want your program to work on all
machines you have to not compile for LSE intructions.  But other
functions (or all functions if -fno-inline-atomics is used) will call
the libatomic library.  Currently those functions do not use LSE
instructions but with this patch we can use the IFUNC infrastructure to
check for LSE support and use LSE in libatomic on machines where it is
supported or not use it on machines where it is not supported.

As an example of what this change does, __atomic_compare_exchange_8 will
turn into a call to libat_compare_exchange_8_i1 on a machine that supports
LSE:

 :
   0:   f9400023    ldr x3, [x1]
   4:   aa0303e4    mov x4, x3
   8:   c8e4fc02    casal   x4, x2, [x0]
   c:   eb03009f    cmp x4, x3
  10:   1a9f17e0    csetw0, eq
  14:   3540    cbnzw0, 1c 
  18:   f924    str x4, [x1]
  1c:   d65f03c0    ret

But call libat_compare_exchange_8 on a machine without LSE:

 :
   0:   f9400023    ldr x3, [x1]
   4:   c85ffc04    ldaxr   x4, [x0]
   8:   eb03009f    cmp x4, x3
   c:   5461    b.ne18 
  10:   c805fc02    stlxr   w5, x2, [x0]
  14:   3585    cbnzw5, 4 
  18:   1a9f17e0    csetw0, eq
  1c:   3440    cbz w0, 24 
  20:   d65f03c0    ret
  24:   f924    str x4, [x1]
  28:   d65f03c0    ret
  2c:   d503201f    nop

Steve Ellcey
sell...@cavium.com

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[James Greenhalgh]

On Mon, Nov 20, 2017 at 05:39:25PM +, Steve Ellcey wrote:
> Re-ping with a CC to the Aarch64 maintainers.

If I'm completely honest with myself, I don't know enough about this area
to review the patch.

Szabolcs' OK holds a lot of weight with me, but I'd like to understand more
of the top-level overview of what this patch means.

If you have the time, would you mind giving me a quick run-down of what
design decisions went in to this patch, and why they are the right thing
to do? Sorry to offload that, but it will be the most efficient route
to a review.

Otherwise, I'll try and make some time for this once I've made it
through the Stack-smashing and SVE reviews. (Or another maintainer
might beat me to it :-) )

Thanks,
James

> > > > looks good to me, but i cannot approve.
> > > > 
> > > > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.)
> > > If you build GCC with default options, ifunc is enabled on aarch64
> > > and
> > > used by libatomic but if you configure with '--disable-gnu-
> > > indirect-
> > > function' then GCC will not recognize the 'resolver' attribute and
> > > libatomic will build the 'old' way and not use IFUNC's.
> > > 
> > > It seems like using '--disable-version-specific-runtime-libs' should
> > > allow GCC to recognize the 'resolver' attribute for IFUNCs but to not
> > > use them when building libatomic but when I tried that I got ifuncs
> > > in
> > > libatomic anyway.  I think that is a bug and I will look into it some
> > > more.
> > > 
> > > The recent alias checking improvements made to GCC and which caused
> > > some glibc build problems also caused the ifunc check in libatomic to
> > > fail.  That problem has been fixed but it involved a change to
> > > libatomic/ibatomic_i.h that conflicted with this patch so I am
> > > including an updated version that will apply cleanly to the top of
> > > tree.  Only libatomic_i.h changed.
> > > 
> > > Steve Ellcey
> > > sell...@cavium.com
> > > 
> > > 2017-10-03  Steve Ellcey  
> > > 
> > >   * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and
> > >   libatomic_la_LIBADD.
> > >   * config/linux/aarch64/host-config.h: New file.
> > >   * configure.ac (IFUNC_RESOLVER_ARGS): Define.
> > >   (ARCH_AARCH64_LINUX): New conditional for IFUNC builds.
> > >   * configure.tgt (aarch64): Set ARCH and try_ifunc.
> > >   (aarch64*-*-linux*) Update config_path.
> > >   (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS.
> > >   * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS
> > > argument.
> > >   * Makefile.in: Regenerate.
> > >   * auto-config.h.in: Regenerate.
> > >   * configure: Regenerate.

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

Re-ping with a CC to the Aarch64 maintainers.

Steve Ellcey
sell...@cavium.com

On Tue, 2017-10-24 at 11:11 -0700, Steve Ellcey wrote:
> Ping.
> 
> Steve Ellcey
> sell...@cavium.com
> 
> On Tue, 2017-10-03 at 11:57 -0700, Steve Ellcey wrote:
> > 
> > On Mon, 2017-10-02 at 15:38 +0100, Szabolcs Nagy wrote:
> > > 
> > > 
> > >  
> > > looks good to me, but i cannot approve.
> > > 
> > > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.)
> > If you build GCC with default options, ifunc is enabled on aarch64
> > and
> > used by libatomic but if you configure with '--disable-gnu-
> > indirect-
> > function' then GCC will not recognize the 'resolver' attribute and
> > libatomic will build the 'old' way and not use IFUNC's.
> > 
> > It seems like using '--disable-version-specific-runtime-libs' should
> > allow GCC to recognize the 'resolver' attribute for IFUNCs but to not
> > use them when building libatomic but when I tried that I got ifuncs
> > in
> > libatomic anyway.  I think that is a bug and I will look into it some
> > more.
> > 
> > The recent alias checking improvements made to GCC and which caused
> > some glibc build problems also caused the ifunc check in libatomic to
> > fail.  That problem has been fixed but it involved a change to
> > libatomic/ibatomic_i.h that conflicted with this patch so I am
> > including an updated version that will apply cleanly to the top of
> > tree.  Only libatomic_i.h changed.
> > 
> > Steve Ellcey
> > sell...@cavium.com
> > 
> > 2017-10-03  Steve Ellcey  
> > 
> > * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and
> > libatomic_la_LIBADD.
> > * config/linux/aarch64/host-config.h: New file.
> > * configure.ac (IFUNC_RESOLVER_ARGS): Define.
> > (ARCH_AARCH64_LINUX): New conditional for IFUNC builds.
> > * configure.tgt (aarch64): Set ARCH and try_ifunc.
> > (aarch64*-*-linux*) Update config_path.
> > (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS.
> > * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS
> > argument.
> > * Makefile.in: Regenerate.
> > * auto-config.h.in: Regenerate.
> > * configure: Regenerate.

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

Ping.

Steve Ellcey
sell...@cavium.com

On Tue, 2017-10-03 at 11:57 -0700, Steve Ellcey wrote:
> On Mon, 2017-10-02 at 15:38 +0100, Szabolcs Nagy wrote:
> > 
> >  
> > looks good to me, but i cannot approve.
> > 
> > (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.)
> If you build GCC with default options, ifunc is enabled on aarch64
> and
> used by libatomic but if you configure with '--disable-gnu-indirect-
> function' then GCC will not recognize the 'resolver' attribute and
> libatomic will build the 'old' way and not use IFUNC's.
> 
> It seems like using '--disable-version-specific-runtime-libs' should
> allow GCC to recognize the 'resolver' attribute for IFUNCs but to not
> use them when building libatomic but when I tried that I got ifuncs
> in
> libatomic anyway.  I think that is a bug and I will look into it some
> more.
> 
> The recent alias checking improvements made to GCC and which caused
> some glibc build problems also caused the ifunc check in libatomic to
> fail.  That problem has been fixed but it involved a change to
> libatomic/ibatomic_i.h that conflicted with this patch so I am
> including an updated version that will apply cleanly to the top of
> tree.  Only libatomic_i.h changed.
> 
> Steve Ellcey
> sell...@cavium.com
> 
> 2017-10-03  Steve Ellcey  
> 
>   * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and
>   libatomic_la_LIBADD.
>   * config/linux/aarch64/host-config.h: New file.
>   * configure.ac (IFUNC_RESOLVER_ARGS): Define.
>   (ARCH_AARCH64_LINUX): New conditional for IFUNC builds.
>   * configure.tgt (aarch64): Set ARCH and try_ifunc.
>   (aarch64*-*-linux*) Update config_path.
>   (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS.
>   * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS
> argument.
>   * Makefile.in: Regenerate.
>   * auto-config.h.in: Regenerate.
>   * configure: Regenerate.
> 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Mon, 2017-10-02 at 15:38 +0100, Szabolcs Nagy wrote:
> 
> looks good to me, but i cannot approve.
> 
> (this will make libatomic depend on ifuncs on aarch64*-linux-gnu.)

If you build GCC with default options, ifunc is enabled on aarch64 and
used by libatomic but if you configure with '--disable-gnu-indirect-
function' then GCC will not recognize the 'resolver' attribute and
libatomic will build the 'old' way and not use IFUNC's.

It seems like using '--disable-version-specific-runtime-libs' should
allow GCC to recognize the 'resolver' attribute for IFUNCs but to not
use them when building libatomic but when I tried that I got ifuncs in
libatomic anyway.  I think that is a bug and I will look into it some
more.

The recent alias checking improvements made to GCC and which caused
some glibc build problems also caused the ifunc check in libatomic to
fail.  That problem has been fixed but it involved a change to
libatomic/ibatomic_i.h that conflicted with this patch so I am
including an updated version that will apply cleanly to the top of
tree.  Only libatomic_i.h changed.

Steve Ellcey
sell...@cavium.com

2017-10-03  Steve Ellcey  

* Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and
libatomic_la_LIBADD.
* config/linux/aarch64/host-config.h: New file.
* configure.ac (IFUNC_RESOLVER_ARGS): Define.
(ARCH_AARCH64_LINUX): New conditional for IFUNC builds.
* configure.tgt (aarch64): Set ARCH and try_ifunc.
(aarch64*-*-linux*) Update config_path.
(aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS.
* libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS argument.
* Makefile.in: Regenerate.
* auto-config.h.in: Regenerate.
* configure: Regenerate.

diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
index d731406..92d19c6 100644
--- a/libatomic/Makefile.am
+++ b/libatomic/Makefile.am
@@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS)))
 
 ## On a target-specific basis, include alternates to be selected by IFUNC.
 if HAVE_IFUNC
+if ARCH_AARCH64_LINUX
+IFUNC_OPTIONS	 = -march=armv8.1-a
+libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
+endif
 if ARCH_ARM_LINUX
 IFUNC_OPTIONS	 = -march=armv7-a -DHAVE_KERNEL64
 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h
index e69de29..08810a9 100644
--- a/libatomic/config/linux/aarch64/host-config.h
+++ b/libatomic/config/linux/aarch64/host-config.h
@@ -0,0 +1,36 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+#if HAVE_IFUNC
+#include 
+
+# ifdef HWCAP_ATOMICS
+#  define IFUNC_COND_1	(hwcap & HWCAP_ATOMICS)
+# else
+#  define IFUNC_COND_1	(false)
+# endif
+# define IFUNC_NCOND(N)	(1)
+
+#endif /* HAVE_IFUNC */
+
+#include_next 
diff --git a/libatomic/configure.ac b/libatomic/configure.ac
index 023f172..b717d3d 100644
--- a/libatomic/configure.ac
+++ b/libatomic/configure.ac
@@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then
   AC_MSG_ERROR([Configuration ${target} is unsupported.])
 fi
 
+# Write out the ifunc resolver arg type.
+AC_DEFINE_UNQUOTED(IFUNC_RESOLVER_ARGS, $IFUNC_RESOLVER_ARGS,
+	[Define ifunc resolver function argument.])
+
 # Disable fallbacks to __sync routines from libgcc.  Otherwise we'll
 # make silly decisions about what the cpu can do.
 CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
@@ -247,6 +251,8 @@ AC_SUBST(LIBS)
 AC_SUBST(SIZES)
 
 AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes)
+AM_CONDITIONAL(ARCH_AARCH64_LINUX,
+	   [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null])
 AM_CONDITIONAL(ARCH_ARM_LINUX,
 	   [expr "$config_path" : ".* linux/arm .*" > /dev/null])
 AM_CONDITIONAL(ARCH_I386,
diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Szabolcs Nagy]

On 29/09/17 21:29, Steve Ellcey wrote:
> On Thu, 2017-09-28 at 12:31 +0100, Szabolcs Nagy wrote:
>>  
>> i think this should be improved, see below.
> 
> Those were all good suggestions, here is a new patch that incorporates
> the changes.  I fixed the IFUNC_OPTIONS argument,
> renamed ARCH_AARCH64_LINUX_LSE, got rid of the auxv references, and
> changed HWCAP_TYPE to IFUNC_RESOLVER_ARGS.
> 
> Here is the new patch, tested with no regressions.
> 

looks good to me, but i cannot approve.

(this will make libatomic depend on ifuncs on aarch64*-linux-gnu.)

> Steve Ellcey
> sell...@cavium.com
> 
> 
> 2017-09-29  Steve Ellcey  
> 
>   * Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and
>   libatomic_la_LIBADD.
>   * config/linux/aarch64/host-config.h: New file.
>   * configure.ac (IFUNC_RESOLVER_ARGS): Define.
>   (ARCH_AARCH64_LINUX): New conditional for IFUNC builds.
>   * configure.tgt (aarch64): Set ARCH and try_ifunc.
>   (aarch64*-*-linux*) Update config_path.
>   (aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS.
>   * libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS argument.
>   * Makefile.in: Regenerate.
>   * auto-config.h.in: Regenerate.
>   * configure: Regenerate.
> 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Thu, 2017-09-28 at 12:31 +0100, Szabolcs Nagy wrote:
> 
> i think this should be improved, see below.

Those were all good suggestions, here is a new patch that incorporates
the changes.  I fixed the IFUNC_OPTIONS argument,
renamed ARCH_AARCH64_LINUX_LSE, got rid of the auxv references, and
changed HWCAP_TYPE to IFUNC_RESOLVER_ARGS.

Here is the new patch, tested with no regressions.

Steve Ellcey
sell...@cavium.com


2017-09-29  Steve Ellcey  

* Makefile.am (ARCH_AARCH64_LINUX): Add IFUNC_OPTIONS and
libatomic_la_LIBADD.
* config/linux/aarch64/host-config.h: New file.
* configure.ac (IFUNC_RESOLVER_ARGS): Define.
(ARCH_AARCH64_LINUX): New conditional for IFUNC builds.
* configure.tgt (aarch64): Set ARCH and try_ifunc.
(aarch64*-*-linux*) Update config_path.
(aarch64*-*-linux*) Set IFUNC_RESOLVER_ARGS.
* libatomic_i.h (GEN_SELECTOR): Add IFUNC_RESOLVER_ARGS argument.
* Makefile.in: Regenerate.
* auto-config.h.in: Regenerate.
* configure: Regenerate.

diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
index d731406..92d19c6 100644
--- a/libatomic/Makefile.am
+++ b/libatomic/Makefile.am
@@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS)))
 
 ## On a target-specific basis, include alternates to be selected by IFUNC.
 if HAVE_IFUNC
+if ARCH_AARCH64_LINUX
+IFUNC_OPTIONS	 = -march=armv8.1-a
+libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
+endif
 if ARCH_ARM_LINUX
 IFUNC_OPTIONS	 = -march=armv7-a -DHAVE_KERNEL64
 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h
index e69de29..08810a9 100644
--- a/libatomic/config/linux/aarch64/host-config.h
+++ b/libatomic/config/linux/aarch64/host-config.h
@@ -0,0 +1,36 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+#if HAVE_IFUNC
+#include 
+
+# ifdef HWCAP_ATOMICS
+#  define IFUNC_COND_1	(hwcap & HWCAP_ATOMICS)
+# else
+#  define IFUNC_COND_1	(false)
+# endif
+# define IFUNC_NCOND(N)	(1)
+
+#endif /* HAVE_IFUNC */
+
+#include_next 
diff --git a/libatomic/configure.ac b/libatomic/configure.ac
index 023f172..b717d3d 100644
--- a/libatomic/configure.ac
+++ b/libatomic/configure.ac
@@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then
   AC_MSG_ERROR([Configuration ${target} is unsupported.])
 fi
 
+# Write out the ifunc resolver arg type.
+AC_DEFINE_UNQUOTED(IFUNC_RESOLVER_ARGS, $IFUNC_RESOLVER_ARGS,
+	[Define ifunc resolver function argument.])
+
 # Disable fallbacks to __sync routines from libgcc.  Otherwise we'll
 # make silly decisions about what the cpu can do.
 CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
@@ -247,6 +251,8 @@ AC_SUBST(LIBS)
 AC_SUBST(SIZES)
 
 AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes)
+AM_CONDITIONAL(ARCH_AARCH64_LINUX,
+	   [expr "$config_path" : ".* linux/aarch64 .*" > /dev/null])
 AM_CONDITIONAL(ARCH_ARM_LINUX,
 	   [expr "$config_path" : ".* linux/arm .*" > /dev/null])
 AM_CONDITIONAL(ARCH_I386,
diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index b8af3ab..388ae95 100644
--- a/libatomic/configure.tgt
+++ b/libatomic/configure.tgt
@@ -40,6 +40,14 @@ case "${target_cpu}" in
   riscv*)		ARCH=riscv ;;
   sh*)			ARCH=sh ;;
 
+  aarch64*)
+	ARCH=aarch64
+	case "${target}" in
+	aarch64*-*-linux*)
+		try_ifunc=yes
+		;;
+	esac
+	;;
   arm*)
 	ARCH=arm
 	case "${target}" in
@@ -109,6 +117,11 @@ fi
 
 # Other system configury
 case "${target}" in
+  aarch64*-*-linux*)
+	# OS support for atomic primitives.
+	config_path="${config_path} linux/aarch64 posix"
+	;;
+
   arm*-*-linux*)
 	# OS support for atomic primitives.
 	config_path="${config_path} linux/arm posix"
@@ -153,3 +166,14 @@ case "${target}" in
 	UNSUPPORTED=1
 	;;
 esac
+
+# glibc will 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Szabolcs Nagy]

On 31/08/17 18:24, Steve Ellcey wrote:
> On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote:
>> > 
>> > in glibc the hwcap is not used, because it has accesses to
>> > cached dispatch info, but in libatomic using the hwcap
>> > argument is the right way.
> Here is an updated version of the patch to allow aarch64 to use ifuncs
> in libatomic.
> 
> The main difference from the last patch is that the library does not
> access the hwcap value directly but accesses it through the ifunc
> resolver argument.  That means that we no longer need the
> init_cpu_revision static constructor to set a flag that the resolver
> checks, instead the resolver just does a comparision of its incoming
> argument with HWCAP_ATOMICS.
> 

i think this approach is fine.

> This did mean I had to change the prototype for the resolver functions
> in libatomic_i.h to have an argument, which is the way glibc calls
> them.  One complication of this is that the type of the argument can
> differ between platforms and ABIs so I added code to configure.tgt to
> set the type.  I used uint64_t for aarch64 and 'long unsigned int'
> for everything else.  That is not correct for all platforms but at 
> this point no other platforms access the argument so it should not
> matter.  If and when platforms do need to access it they can change
> the type if necessary.
> 

i think this should be improved, see below.

> Steve Ellcey
> sell...@cavium.com
> 
> 
> 2017-08-31  Steve Ellcey  
> 
>   * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
>   libatomic_la_LIBADD.
>   * config/linux/aarch64/host-config.h: New file.
>   * configure.ac (HWCAP_TYPE): Define.
>   (AC_CHECK_HEADERS): Check for sys/auxv.h.
>   (AC_CHECK_FUNCS): Check for getauxval.
>   (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
>   * configure.tgt (aarch64): Set AARCH and try_ifunc.
>   (aarch64*-*-linux*) Update config_path.
>   (aarch64*-*-linux*) Set HWCAP_TYPE.
>   * libatomic_i.h (GEN_SELECTOR): Add "HWCAP_TYPE hwcap" argument.
>   * Makefile.in: Regenerate.
>   * auto-config.h.in: Regenerate.
>   * configure: Regenerate.
> 
> 
> libatomic.patch
> 
> 
> diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
> index d731406..a35df1e 100644
> --- a/libatomic/Makefile.am
> +++ b/libatomic/Makefile.am
> @@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix 
> _$(s)_.lo,$(SIZEOBJS)))
>  
>  ## On a target-specific basis, include alternates to be selected by IFUNC.
>  if HAVE_IFUNC
> +if ARCH_AARCH64_LINUX_LSE
> +IFUNC_OPTIONS = -mcpu=thunderx2t99

i'd expect -march=armv8.1-a instead of a particular cpu here.
(and it think this assumes a not very old binutils gas)

> +libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix 
> _$(s)_1_.lo,$(SIZEOBJS)))
> +endif
>  if ARCH_ARM_LINUX
>  IFUNC_OPTIONS = -march=armv7-a -DHAVE_KERNEL64
>  libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix 
> _$(s)_1_.lo,$(SIZEOBJS)))
> diff --git a/libatomic/configure.ac b/libatomic/configure.ac
> index 023f172..4e06ffe 100644
> --- a/libatomic/configure.ac
> +++ b/libatomic/configure.ac
> @@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then
>AC_MSG_ERROR([Configuration ${target} is unsupported.])
>  fi
>  
> +# Write out the ifunc resolver arg type.
> +AC_DEFINE_UNQUOTED(HWCAP_TYPE, $HWCAP_TYPE,
> + [Define type of ifunc resolver function argument.])
> +
>  # Disable fallbacks to __sync routines from libgcc.  Otherwise we'll
>  # make silly decisions about what the cpu can do.
>  CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
> @@ -171,7 +175,8 @@ CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
>  AC_STDC_HEADERS
>  ACX_HEADER_STRING
>  GCC_HEADER_STDINT(gstdint.h)
> -AC_CHECK_HEADERS([fenv.h])
> +AC_CHECK_HEADERS([fenv.h sys/auxv.h])
> +AC_CHECK_FUNCS(getauxval)
>  

getauxval is no longer needed.

>  # Check for common type sizes
>  LIBAT_FORALL_MODES([LIBAT_HAVE_INT_MODE])
> @@ -247,6 +252,8 @@ AC_SUBST(LIBS)
>  AC_SUBST(SIZES)
>  
>  AM_CONDITIONAL(HAVE_IFUNC, test x$libat_cv_have_ifunc = xyes)
> +AM_CONDITIONAL(ARCH_AARCH64_LINUX_LSE,
> +[expr "$config_path" : ".* linux/aarch64 .*" > /dev/null])

linux/aarch64 seems to be set for all aarch64*-*-linux* targets below.
so why call it _LSE?

>  AM_CONDITIONAL(ARCH_ARM_LINUX,
>  [expr "$config_path" : ".* linux/arm .*" > /dev/null])
>  AM_CONDITIONAL(ARCH_I386,
> diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
> index b8af3ab..0bb5c66 100644
> --- a/libatomic/configure.tgt
> +++ b/libatomic/configure.tgt
> @@ -40,6 +40,14 @@ case "${target_cpu}" in
>riscv*)ARCH=riscv ;;
>sh*)   ARCH=sh ;;
>  
> +  aarch64*)
> + ARCH=aarch64
> + case "${target}" in
> + aarch64*-*-linux*)
> + try_ifunc=yes
> + ;;
> + esac
> + ;;
>arm*)
>   ARCH=arm
>   case "${target}" 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

Ping.

Steve Ellcey
sell...@cavium.com

On Thu, 2017-08-31 at 10:24 -0700, Steve Ellcey wrote:
> On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote:
> > 
> >  
> > in glibc the hwcap is not used, because it has accesses to
> > cached dispatch info, but in libatomic using the hwcap
> > argument is the right way.
> Here is an updated version of the patch to allow aarch64 to use
> ifuncs
> in libatomic.
> 
> The main difference from the last patch is that the library does not
> access the hwcap value directly but accesses it through the ifunc
> resolver argument.  That means that we no longer need the
> init_cpu_revision static constructor to set a flag that the resolver
> checks, instead the resolver just does a comparision of its incoming
> argument with HWCAP_ATOMICS.
> 
> This did mean I had to change the prototype for the resolver
> functions
> in libatomic_i.h to have an argument, which is the way glibc calls
> them.  One complication of this is that the type of the argument can
> differ between platforms and ABIs so I added code to configure.tgt to
> set the type.  I used uint64_t for aarch64 and 'long unsigned int'
> for everything else.  That is not correct for all platforms but at 
> this point no other platforms access the argument so it should not
> matter.  If and when platforms do need to access it they can change
> the type if necessary.
> 
> Steve Ellcey
> sell...@cavium.com
> 
> 
> 2017-08-31  Steve Ellcey  
> 
>   * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
>   libatomic_la_LIBADD.
>   * config/linux/aarch64/host-config.h: New file.
>   * configure.ac (HWCAP_TYPE): Define.
>   (AC_CHECK_HEADERS): Check for sys/auxv.h.
>   (AC_CHECK_FUNCS): Check for getauxval.
>   (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
>   * configure.tgt (aarch64): Set AARCH and try_ifunc.
>   (aarch64*-*-linux*) Update config_path.
>   (aarch64*-*-linux*) Set HWCAP_TYPE.
>   * libatomic_i.h (GEN_SELECTOR): Add "HWCAP_TYPE hwcap"
> argument.
>   * Makefile.in: Regenerate.
>   * auto-config.h.in: Regenerate.
>   * configure: Regenerate.
> 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote:
> 
> in glibc the hwcap is not used, because it has accesses to
> cached dispatch info, but in libatomic using the hwcap
> argument is the right way.

Here is an updated version of the patch to allow aarch64 to use ifuncs
in libatomic.

The main difference from the last patch is that the library does not
access the hwcap value directly but accesses it through the ifunc
resolver argument.  That means that we no longer need the
init_cpu_revision static constructor to set a flag that the resolver
checks, instead the resolver just does a comparision of its incoming
argument with HWCAP_ATOMICS.

This did mean I had to change the prototype for the resolver functions
in libatomic_i.h to have an argument, which is the way glibc calls
them.  One complication of this is that the type of the argument can
differ between platforms and ABIs so I added code to configure.tgt to
set the type.  I used uint64_t for aarch64 and 'long unsigned int'
for everything else.  That is not correct for all platforms but at 
this point no other platforms access the argument so it should not
matter.  If and when platforms do need to access it they can change
the type if necessary.

Steve Ellcey
sell...@cavium.com


2017-08-31  Steve Ellcey  

* Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
libatomic_la_LIBADD.
* config/linux/aarch64/host-config.h: New file.
* configure.ac (HWCAP_TYPE): Define.
(AC_CHECK_HEADERS): Check for sys/auxv.h.
(AC_CHECK_FUNCS): Check for getauxval.
(ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
* configure.tgt (aarch64): Set AARCH and try_ifunc.
(aarch64*-*-linux*) Update config_path.
(aarch64*-*-linux*) Set HWCAP_TYPE.
* libatomic_i.h (GEN_SELECTOR): Add "HWCAP_TYPE hwcap" argument.
* Makefile.in: Regenerate.
* auto-config.h.in: Regenerate.
* configure: Regenerate.

diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
index d731406..a35df1e 100644
--- a/libatomic/Makefile.am
+++ b/libatomic/Makefile.am
@@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS)))
 
 ## On a target-specific basis, include alternates to be selected by IFUNC.
 if HAVE_IFUNC
+if ARCH_AARCH64_LINUX_LSE
+IFUNC_OPTIONS	 = -mcpu=thunderx2t99
+libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
+endif
 if ARCH_ARM_LINUX
 IFUNC_OPTIONS	 = -march=armv7-a -DHAVE_KERNEL64
 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h
index e69de29..2e5e97a 100644
--- a/libatomic/config/linux/aarch64/host-config.h
+++ b/libatomic/config/linux/aarch64/host-config.h
@@ -0,0 +1,39 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+#if HAVE_IFUNC
+#include 
+#ifdef HAVE_SYS_AUXV_H
+# include 
+#endif
+
+# ifdef HWCAP_ATOMICS
+#  define IFUNC_COND_1	(hwcap & HWCAP_ATOMICS)
+# else
+#  define IFUNC_COND_1	(false)
+# endif
+# define IFUNC_NCOND(N)	(1)
+
+#endif /* HAVE_IFUNC */
+
+#include_next 
diff --git a/libatomic/configure.ac b/libatomic/configure.ac
index 023f172..4e06ffe 100644
--- a/libatomic/configure.ac
+++ b/libatomic/configure.ac
@@ -163,6 +163,10 @@ if test -n "$UNSUPPORTED"; then
   AC_MSG_ERROR([Configuration ${target} is unsupported.])
 fi
 
+# Write out the ifunc resolver arg type.
+AC_DEFINE_UNQUOTED(HWCAP_TYPE, $HWCAP_TYPE,
+	[Define type of ifunc resolver function argument.])
+
 # Disable fallbacks to __sync routines from libgcc.  Otherwise we'll
 # make silly decisions about what the cpu can do.
 CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
@@ -171,7 +175,8 @@ CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
 AC_STDC_HEADERS
 ACX_HEADER_STRING
 GCC_HEADER_STDINT(gstdint.h)

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Tue, 2017-08-29 at 12:25 +0100, Szabolcs Nagy wrote:
> 
> it's a general bug that most ifunc users (e.g. in binutils
> tests) declare the ifunc resolvers incorrectly, the correct
> prototype is the way the dynamic linker invokes the resolver
> in sysdeps/aarch64/dl-irel.h:
> 
> static inline ElfW(Addr)
> __attribute ((always_inline))
> elf_ifunc_invoke (ElfW(Addr) addr)
> {
>   return ((ElfW(Addr) (*) (unsigned long int)) (addr))
> (GLRO(dl_hwcap));
> }
> 
> (that argument should be uint64_t for ilp32, but that's a
> separate issue)
> 
> in glibc the hwcap is not used, because it has accesses to
> cached dispatch info, but in libatomic using the hwcap
> argument is the right way.

OK, I changed my patch to use the argument and it did work but
since the type of the argument should be uint64_t instead of 'unsigned
long int' for aarch64 I think I will send a patch to libc-alpha
first to fix this before sending an updated libatomic patch.  That way
I won't have to update GCC when glibc changes the type.  I see that
different platforms use different types for the resolver argument so I
think I will have to update the libatomic configure.tgt script as well
to set the resolver arg type when I resbumit the gcc libatomic patch so
that we can have the correct prototype in libatomic_i.h.

Steve Ellcey
sell...@cavium.com

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Szabolcs Nagy]

On 28/08/17 19:25, Steve Ellcey wrote:
> On Fri, 2017-08-25 at 15:37 +0100, Szabolcs Nagy wrote:
> 
>> the use of ifunc in gcc target libraries was a mistake
>> in my opinion, there are several known bugs in the ifunc
>> design and uclibc/musl/bionic don't plan to support it.
>> but at this point i dont have a better proposal for doing
>> runtime selection of optimal atomics code.
>>
>> however in this patch i don't see why would the ctor run
>> before ifunc resolvers. how does this work on x86_64?
>> (there the different 16byte atomics are not even compatible,
>> so if ifunc resolvers in different dsos return different
>> result because one ran before the ctor, another after then
>> the runtime behaviour is broken. this can happen when one
>> dso is bindnow so ifunc relocation is processed before
>> ctors and another runs resolvers lazily or dlopened later..
>> but i didnt test it just looks broken)
> 
> I am not sure I followed everything here.  My basic testing all
> worked, init_cpu_revision got run when libatomic was loaded and
> then the IFUNC resolver gets called after that when one of the IFUNC
> atomic functions is called.  init_cpu_revision sets libat_have_lse
> which, I assume, will not be used by any other libraries.  If other
> libraries have IFUNCs they would have their own static constructors and
> set their own variables to be checked by their own IFUNCs.  So I am
> not sure how one DSO is going to break another DSO.
> 

this can only possibly work with lazy binding of the
libatomic calls (there are many reasons why that may
not be the case starting from static linking to compiling
with -fno-plt or setting LD_BIND_NOW at runtime) as i
expected this is broken on x86 see the execution test
i added to pr 60790:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60790

>> note that aarch64 ifunc resolvers get hwcap as an argument
>> so all this brokenness can be avoided (and if we want to
>> disable hwcap flags then probably glibc should take care
>> of that before passing hwcap to the ifunc resolver).
> 
> I looked at the IFUNC memcpy resolver in glibc and it does not look
> like it gets hwcap as an argument.  When I preprocess everything I see:
> 
> void *__libc_memcpy_ifunc (void) __asm__ ("__libc_memcpy");
> void *__libc_memcpy_ifunc (void)
> {
>   uint64_t __attribute__((unused)) midr = _dl_aarch64_cpu_features.midr_el1;
>   *res = ** expression that looks at midr value and returns function pointer 
> **;
>   return res;
> }
> __asm__ (".type " "__libc_memcpy" ", %gnu_indirect_function");
> extern __typeof (__libc_memcpy) memcpy __attribute__ ((alias 
> ("__libc_memcpy")));
> 

it's a general bug that most ifunc users (e.g. in binutils
tests) declare the ifunc resolvers incorrectly, the correct
prototype is the way the dynamic linker invokes the resolver
in sysdeps/aarch64/dl-irel.h:

static inline ElfW(Addr)
__attribute ((always_inline))
elf_ifunc_invoke (ElfW(Addr) addr)
{
  return ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
}

(that argument should be uint64_t for ilp32, but that's a
separate issue)

in glibc the hwcap is not used, because it has accesses to
cached dispatch info, but in libatomic using the hwcap
argument is the right way.

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

On Fri, 2017-08-25 at 15:37 +0100, Szabolcs Nagy wrote:

> the use of ifunc in gcc target libraries was a mistake
> in my opinion, there are several known bugs in the ifunc
> design and uclibc/musl/bionic don't plan to support it.
> but at this point i dont have a better proposal for doing
> runtime selection of optimal atomics code.
> 
> however in this patch i don't see why would the ctor run
> before ifunc resolvers. how does this work on x86_64?
> (there the different 16byte atomics are not even compatible,
> so if ifunc resolvers in different dsos return different
> result because one ran before the ctor, another after then
> the runtime behaviour is broken. this can happen when one
> dso is bindnow so ifunc relocation is processed before
> ctors and another runs resolvers lazily or dlopened later..
> but i didnt test it just looks broken)

I am not sure I followed everything here.  My basic testing all
worked, init_cpu_revision got run when libatomic was loaded and
then the IFUNC resolver gets called after that when one of the IFUNC
atomic functions is called.  init_cpu_revision sets libat_have_lse
which, I assume, will not be used by any other libraries.  If other
libraries have IFUNCs they would have their own static constructors and
set their own variables to be checked by their own IFUNCs.  So I am
not sure how one DSO is going to break another DSO.

> note that aarch64 ifunc resolvers get hwcap as an argument
> so all this brokenness can be avoided (and if we want to
> disable hwcap flags then probably glibc should take care
> of that before passing hwcap to the ifunc resolver).

I looked at the IFUNC memcpy resolver in glibc and it does not look
like it gets hwcap as an argument.  When I preprocess everything I see:

void *__libc_memcpy_ifunc (void) __asm__ ("__libc_memcpy");
void *__libc_memcpy_ifunc (void)
{
  uint64_t __attribute__((unused)) midr = _dl_aarch64_cpu_features.midr_el1;
  *res = ** expression that looks at midr value and returns function pointer **;
  return res;
}
__asm__ (".type " "__libc_memcpy" ", %gnu_indirect_function");
extern __typeof (__libc_memcpy) memcpy __attribute__ ((alias 
("__libc_memcpy")));


Steve Ellcey
sell...@cavium.com

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Szabolcs Nagy]

On 07/08/17 21:44, Steve Ellcey wrote:
> This patch uses the libatomic IFUNC infrastructure so that aarch64
> machines that support the LSE instructions can use them.  Note that
> aarch64 still isn't enabling IFUNC support by default though I have
> submitted a patch to do that.  You can enable IFUNC support by
> configuring with --enable-gnu-indirect-function.
> 
> Glibc has an environment variable, LD_HWCAP_MASK, that can be used to
> mask out some or all of the bits returned by getauxval(AT_HWCAP) and
> ignore certain hardware capabilities.  I enabled this functionality
> for libatomic by looking at the LD_HWCAP_MASK variable in the IFUNC
> resolver function.  That way, if I had a system that supported LSE but
> did not want to use it for some reason, I could set LD_HWCAP_MASK to
> zero and then the IFUNC selector function would not enable the LSE
> routines.  I could remove this functionality if we thought it was not
> appropriate but I think it is useful, both for testing and for end
> users.
> 

the use of ifunc in gcc target libraries was a mistake
in my opinion, there are several known bugs in the ifunc
design and uclibc/musl/bionic don't plan to support it.
but at this point i dont have a better proposal for doing
runtime selection of optimal atomics code.

however in this patch i don't see why would the ctor run
before ifunc resolvers. how does this work on x86_64?
(there the different 16byte atomics are not even compatible,
so if ifunc resolvers in different dsos return different
result because one ran before the ctor, another after then
the runtime behaviour is broken. this can happen when one
dso is bindnow so ifunc relocation is processed before
ctors and another runs resolvers lazily or dlopened later..
but i didnt test it just looks broken)

note that aarch64 ifunc resolvers get hwcap as an argument
so all this brokenness can be avoided (and if we want to
disable hwcap flags then probably glibc should take care
of that before passing hwcap to the ifunc resolver).

> Tested on aarch64, OK for checkin?
> 
> Steve Ellcey
> sell...@cavium.com
> 
> 
> 
> 
> 2017-08-07  Steve Ellcey  
> 
>   * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
>   libatomic_la_LIBADD.
>   * config/linux/aarch64/host-config.h: New file.
>   * config/linux/aarch64/init.c: New file.
>   * configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h.
>   (AC_CHECK_FUNCS): Check for getauxval.
>   (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
>   * configure.tgt (aarch64): Set AARCH and try_ifunc.
>   (aarch64*-*-linux*) Update config_path.
>   * Makefile.in: Regenerate.
>   * auto-config.h.in: Regenerate.
>   * configure: Regenerate.
> 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

Ping.

> 2017-08-07  Steve Ellcey  
> 
>   * Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
>   libatomic_la_LIBADD.
>   * config/linux/aarch64/host-config.h: New file.
>   * config/linux/aarch64/init.c: New file.
>   * configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h.
>   (AC_CHECK_FUNCS): Check for getauxval.
>   (ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
>   * configure.tgt (aarch64): Set AARCH and try_ifunc.
>   (aarch64*-*-linux*) Update config_path.
>   * Makefile.in: Regenerate.
>   * auto-config.h.in: Regenerate.
>   * configure: Regenerate.
> 

Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

It would probably help if I included the patch.

Steve Ellcey
sell...@cavium.com

2017-08-07  Steve Ellcey  

* Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
libatomic_la_LIBADD.
* config/linux/aarch64/host-config.h: New file.
* config/linux/aarch64/init.c: New file.
* configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h.
(AC_CHECK_FUNCS): Check for getauxval.
(ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
* configure.tgt (aarch64): Set AARCH and try_ifunc.
(aarch64*-*-linux*) Update config_path.
* Makefile.in: Regenerate.
* auto-config.h.in: Regenerate.
* configure: Regenerate.

diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am
index d731406..a35df1e 100644
--- a/libatomic/Makefile.am
+++ b/libatomic/Makefile.am
@@ -122,6 +122,10 @@ libatomic_la_LIBADD = $(foreach s,$(SIZES),$(addsuffix _$(s)_.lo,$(SIZEOBJS)))
 
 ## On a target-specific basis, include alternates to be selected by IFUNC.
 if HAVE_IFUNC
+if ARCH_AARCH64_LINUX_LSE
+IFUNC_OPTIONS	 = -mcpu=thunderx2t99
+libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
+endif
 if ARCH_ARM_LINUX
 IFUNC_OPTIONS	 = -march=armv7-a -DHAVE_KERNEL64
 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS)))
diff --git a/libatomic/config/linux/aarch64/host-config.h b/libatomic/config/linux/aarch64/host-config.h
index e69de29..4d0ab96 100644
--- a/libatomic/config/linux/aarch64/host-config.h
+++ b/libatomic/config/linux/aarch64/host-config.h
@@ -0,0 +1,33 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+#if HAVE_IFUNC
+
+extern bool libat_have_lse HIDDEN;
+
+# define IFUNC_COND_1	libat_have_lse
+# define IFUNC_NCOND(N)	(1)
+
+#endif /* HAVE_IFUNC */
+
+#include_next 
diff --git a/libatomic/config/linux/aarch64/init.c b/libatomic/config/linux/aarch64/init.c
index e69de29..1b135c2 100644
--- a/libatomic/config/linux/aarch64/init.c
+++ b/libatomic/config/linux/aarch64/init.c
@@ -0,0 +1,51 @@
+/* Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of the GNU Atomic Library (libatomic).
+
+   Libatomic is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libatomic is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+#include "libatomic_i.h"
+
+#if HAVE_IFUNC
+
+#include 
+#ifdef HAVE_SYS_AUXV_H
+# include 
+#endif
+
+bool libat_have_lse = false;
+
+static void __attribute__((constructor))
+init_cpu_revision (void)
+{
+#if defined (HAVE_GETAUXVAL) && defined (HWCAP_ATOMICS)
+  unsigned long i;
+  char *s;
+
+  i = getauxval (AT_HWCAP);
+  s = getenv ("LD_HWCAP_MASK");
+  if (s)
+i = i & strtoul (s, NULL, 10);
+  if (i & HWCAP_ATOMICS)
+libat_have_lse = true;
+#endif
+}
+
+#endif /* HAVE_IFUNC */
diff --git a/libatomic/configure.ac b/libatomic/configure.ac
index 023f172..2bdc234 100644
--- a/libatomic/configure.ac
+++ b/libatomic/configure.ac
@@ -171,7 +171,8 @@ CFLAGS="$save_CFLAGS -fno-sync-libcalls $XCFLAGS"
 AC_STDC_HEADERS
 ACX_HEADER_STRING
 GCC_HEADER_STDINT(gstdint.h)

[Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[Steve Ellcey]

This patch uses the libatomic IFUNC infrastructure so that aarch64
machines that support the LSE instructions can use them.  Note that
aarch64 still isn't enabling IFUNC support by default though I have
submitted a patch to do that.  You can enable IFUNC support by
configuring with --enable-gnu-indirect-function.

Glibc has an environment variable, LD_HWCAP_MASK, that can be used to
mask out some or all of the bits returned by getauxval(AT_HWCAP) and
ignore certain hardware capabilities.  I enabled this functionality
for libatomic by looking at the LD_HWCAP_MASK variable in the IFUNC
resolver function.  That way, if I had a system that supported LSE but
did not want to use it for some reason, I could set LD_HWCAP_MASK to
zero and then the IFUNC selector function would not enable the LSE
routines.  I could remove this functionality if we thought it was not
appropriate but I think it is useful, both for testing and for end
users.

Tested on aarch64, OK for checkin?

Steve Ellcey
sell...@cavium.com




2017-08-07  Steve Ellcey  

* Makefile.am (ARCH_AARCH64_LINUX_LSE): Add IFUNC_OPTIONS and
libatomic_la_LIBADD.
* config/linux/aarch64/host-config.h: New file.
* config/linux/aarch64/init.c: New file.
* configure.ac (AC_CHECK_HEADERS): Check for sys/auxv.h.
(AC_CHECK_FUNCS): Check for getauxval.
(ARCH_AARCH64_LINUX_LSE): New conditional for IFUNC builds.
* configure.tgt (aarch64): Set AARCH and try_ifunc.
(aarch64*-*-linux*) Update config_path.
* Makefile.in: Regenerate.
* auto-config.h.in: Regenerate.
* configure: Regenerate.