[Bug go/60790] New: libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[Bug go/60790] New: libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[gary at intrepid dot com]

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

            Bug ID: 60790
           Summary: libatomic convenience library selects IFUNC
                    implementation before obtaining cpu info.
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: go
          Assignee: ian at airs dot com
          Reporter: gary at intrepid dot com
                CC: nenad at intrepid dot com, PHHargrove at lbl dot gov,
                    rth at gcc dot gnu.org

Created attachment 32569
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32569&action=edit
libatomic cpuinfo patch

We ran into this issue recently while implementing atomic memory support for
GNU UPC.  We followed an approach similar to libgo -- we included
libatomic_convenience.a into libgupc.a.  This lets libgupc access GCC's
__atomic support without requiring the user to link explicitly with libatomic.

While debugging the UPC AMO library on a modern X86-64 platform, we noticed
that the locked implementation of compare-exchange-16 was being used in spite
of the fact of that the CPU supports the CMPX16 operation directly.

We determined that the IFUNC selector function was being called before the
constructor function in init.c had run.  Thus the libat_feat1_ecx variable was
zero.  Because the feature bit wasn't set, the selector function chose the CAS
function which requires explicit locks.  Interestingly,
__atomic_is_lock_free(16, NULL) returns true, as expected on this platform
because it is called after the init_cpuid() constructor is called.

One way to fix this issue is to re-work the logic that queries the CPU feature
register value, so that it explicitly initializes the value if it is zero.  The
assumption here is that __get_cpuid() will always return a non-zero value in
the ECX feature register.  A patch that implements this approach is attached.

[Bug go/60790] libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[gary at intrepid dot com]

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

Gary Funck <gary at intrepid dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32569|0                           |1
        is obsolete|                            |

--- Comment #1 from Gary Funck <gary at intrepid dot com> ---
Created attachment 32570
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32570&action=edit
libatomic cpuinfo patch

Test for edx when checking bit_CMPXCHG8B.

[Bug libgcc/60790] libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
          Component|go                          |libgcc
         Resolution|---                         |INVALID
           Assignee|ian at airs dot com                |unassigned at gcc dot gnu.org

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It is better if you just link directly against libatomic.  We already require
that for C11 _Atomic support in some/most cases.

>We followed an approach similar to libgo 

libgo needs to be fixed the same way really.

[Bug libgcc/60790] libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[ian at airs dot com]

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

Ian Lance Taylor <ian at airs dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
   Last reconfirmed|                            |2014-04-09
                 CC|                            |ian at airs dot com
         Resolution|INVALID                     |---
     Ever confirmed|0                           |1

--- Comment #3 from Ian Lance Taylor <ian at airs dot com> ---
Andrew, you are closing this way too fast.  This same problem is going to arise 
whenever libatomic is statically linked.  The library assumes that global
constructors are run before IFUNC selectors, but that is not the case in a
static link.  IFUNC selectors are run first.

[Bug libgcc/60790] libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That is not guaranteed for shared libs either actually in some more complex
cases of shared library dependencies.

[Bug libgcc/60790] libatomic convenience library selects IFUNC implementation before obtaining cpu info.

[gary at intrepid dot com]

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

--- Comment #5 from Gary Funck <gary at intrepid dot com> ---
(In reply to Andrew Pinski from comment #2)
> It is better if you just link directly against libatomic.  We already
> require that for C11 _Atomic support in some/most cases.

As Ian notes, this issue would apply if libatomic were statically linked as
well.

In GNU UPC, we have a technical need to link in the (static) convenience
library, apart from it being "convenient" for the user: we need to substitute
our own locking routines.  I plan to submit a separate RFE on this.

Basically, we need process-level system wide locks because in the default
configuration UPC "threads" map to an operating system "process".  The current
libatomic implementation uses pthread mutexes.   We of course could build our
own libatomic types of facility, but that is a lot of work - libatomic is
well-engineered and apart from our need to have a different locking policy it
is a good fit for our application.  (detail: we had to build a
libatomic_convenience_nolock.a archive).

PS: It would be helpful to have a separate "libatomic" bug reporting category.