This patch performs lazy initialization of the relevant CPUID feature register value. It will needlessly invoke the CPUID determination code on architectures which lack CPUID support or support for the feature register, but I think it's not worth to avoid the complexity for that.

I verified manually that the CMPXCHG16B implementation is still selected for a 128-bit load after the change. I don't know how to write an automated test for that.

Thanks,
Florian
Index: libatomic/ChangeLog
===================================================================
--- libatomic/ChangeLog	(revision 258952)
+++ libatomic/ChangeLog	(working copy)
@@ -1,3 +1,18 @@
+2018-03-29  Florian Weimer  <fwei...@tor.usersys.redhat.com>
+
+	PR libgcc/60790
+	x86: Do not assume ELF constructors run before IFUNC resolvers.
+	* config/x86/host-config.h (libat_feat1_ecx, libat_feat1_edx):
+	Remove declarations.
+	(__libat_feat1, __libat_feat1_init): Declare.
+	(FEAT1_REGISTER): Define.
+	(load_feat1): New function.
+	(IFUNC_COND_1): Adjust.
+	* config/x86/init.c (libat_feat1_ecx, libat_feat1_edx)
+	(init_cpuid): Remove definitions.
+	(__libat_feat1): New variable.
+	(__libat_feat1_init): New function.
+
 2018-03-09  Andreas Krebbel  <kreb...@linux.vnet.ibm.com>
 
 	* config/s390/exch_n.c: New file.
Index: libatomic/config/x86/host-config.h
===================================================================
--- libatomic/config/x86/host-config.h	(revision 258952)
+++ libatomic/config/x86/host-config.h	(working copy)
@@ -25,13 +25,39 @@
 #if HAVE_IFUNC
 #include <cpuid.h>
 
-extern unsigned int libat_feat1_ecx HIDDEN;
-extern unsigned int libat_feat1_edx HIDDEN;
+#ifdef __x86_64__
+# define FEAT1_REGISTER ecx
+#else
+# define FEAT1_REGISTER edx
+#endif
 
+/* Value of the CPUID feature FEAT1_REGISTER for the cmpxchg bit for
+   IFUNC_COND1 below.  */
+extern unsigned int __libat_feat1 HIDDEN;
+
+/* Initialize libat_feat1 and return its value.  */
+unsigned int __libat_feat1_init (void) HIDDEN;
+
+/* Return the value of the relevant feature register for the relevant
+   cmpxchg bit, or 0 if there is no CPUID support.  */
+static inline unsigned int
+__attribute__ ((const))
+load_feat1 (void)
+{
+  /* Synchronizes with the store in __libat_feat1_init.  */
+  unsigned int feat1 = __atomic_load_n (&__libat_feat1, __ATOMIC_RELAXED);
+  if (feat1 == 0)
+    /* Assume that initialization has not happened yet.  This may get
+       called repeatedly if the CPU does not have any feature bits at
+       all.  */
+    feat1 = __libat_feat1_init ();
+  return feat1;
+}
+
 #ifdef __x86_64__
-# define IFUNC_COND_1	(libat_feat1_ecx & bit_CMPXCHG16B)
+# define IFUNC_COND_1	(load_feat1 () & bit_CMPXCHG16B)
 #else
-# define IFUNC_COND_1	(libat_feat1_edx & bit_CMPXCHG8B)
+# define IFUNC_COND_1	(load_feat1 () & bit_CMPXCHG8B)
 #endif
 
 #ifdef __x86_64__
Index: libatomic/config/x86/init.c
===================================================================
--- libatomic/config/x86/init.c	(revision 258952)
+++ libatomic/config/x86/init.c	(working copy)
@@ -26,13 +26,17 @@
 
 #if HAVE_IFUNC
 
-unsigned int libat_feat1_ecx, libat_feat1_edx;
+unsigned int __libat_feat1;
 
-static void __attribute__((constructor))
-init_cpuid (void)
+unsigned int
+__libat_feat1_init (void)
 {
-  unsigned int eax, ebx;
-  __get_cpuid (1, &eax, &ebx, &libat_feat1_ecx, &libat_feat1_edx);
+  unsigned int eax, ebx, ecx, edx;
+  FEAT1_REGISTER = 0;
+  __get_cpuid (1, &eax, &ebx, &ecx, &edx);
+  /* Synchronizes with the load in load_feat1.  */
+  __atomic_store_n (&__libat_feat1, FEAT1_REGISTER, __ATOMIC_RELAXED);
+  return FEAT1_REGISTER;
 }
 
 #endif /* HAVE_IFUNC */

Reply via email to