glibc 2.21 - Machine maintainers, please test your machines.

[Chris Metcalf]

On 1/25/2015 10:18 AM, Chris Metcalf wrote:
> I'm now running with the following change, to see if tilegx32 will
> also pass with it; this implements my suggestion of rounding new_sem to
> an 8-byte boundary explicitly on ILP32 platforms. 

With my proposed change, tilegx32 (and tilegx64) pass all the tests.  Repeated
here with a suitable ChangeLog.  Let me know if this is acceptable to commit
for 2.21.

2015-01-25  Chris Metcalf  <cmetcalf@ezchip.com>

         * sysdeps/nptl/internaltypes.h (to_new_sem): Define.  Provides new
         behavior for [__HAVE_64B_ATOMICS && !defined (_LP64)].
         * nptl/sem_getvalue.c (__new_sem_getvalue): Use to_new_sem.
         * nptl/sem_init.c (__new_sem_init): Likewise.
         * nptl/sem_post.c (__new_sem_post): Likewise.
         * nptl/sem_wait.c (__new_sem_wait): Likewise.
         (__new_sem_trywait): Likewise.
         * nptl/sem_timedwait.c (sem_timedwait): Likewise.
         * nptl/sem_open.c (sem_open): Add comment about to_new_sem.

diff --git a/nptl/sem_getvalue.c b/nptl/sem_getvalue.c
index 1432cc7..08a0789 100644
--- a/nptl/sem_getvalue.c
+++ b/nptl/sem_getvalue.c
@@ -25,7 +25,7 @@
  int
  __new_sem_getvalue (sem_t *sem, int *sval)
  {
-  struct new_sem *isem = (struct new_sem *) sem;
+  struct new_sem *isem = to_new_sem (sem);

    /* XXX Check for valid SEM parameter.  */
    /* FIXME This uses relaxed MO, even though POSIX specifies that this function
diff --git a/nptl/sem_init.c b/nptl/sem_init.c
index 575b661..aaa6af8 100644
--- a/nptl/sem_init.c
+++ b/nptl/sem_init.c
@@ -50,7 +50,7 @@ __new_sem_init (sem_t *sem, int pshared, unsigned int value)
      }

    /* Map to the internal type.  */
-  struct new_sem *isem = (struct new_sem *) sem;
+  struct new_sem *isem = to_new_sem (sem);

    /* Use the values the caller provided.  */
  #if __HAVE_64B_ATOMICS
diff --git a/nptl/sem_open.c b/nptl/sem_open.c
index bfd2dea..9c1f279 100644
--- a/nptl/sem_open.c
+++ b/nptl/sem_open.c
@@ -186,7 +186,9 @@ sem_open (const char *name, int oflag, ...)
           return SEM_FAILED;
         }

-      /* Create the initial file content.  */
+      /* Create the initial file content.  Note that the new_sem
+         will have the necessary alignment in this case, so we are
+         not obliged to use the to_new_sem macro in this case.  */
        union
        {
         sem_t initsem;
diff --git a/nptl/sem_post.c b/nptl/sem_post.c
index 6e495ed..71818ea 100644
--- a/nptl/sem_post.c
+++ b/nptl/sem_post.c
@@ -56,7 +56,7 @@ futex_wake (unsigned int* futex, int processes_to_wake, int private)
  int
  __new_sem_post (sem_t *sem)
  {
-  struct new_sem *isem = (struct new_sem *) sem;
+  struct new_sem *isem = to_new_sem (sem);
    int private = isem->private;

  #if __HAVE_64B_ATOMICS
diff --git a/nptl/sem_timedwait.c b/nptl/sem_timedwait.c
index 042b0ac..5e650e0 100644
--- a/nptl/sem_timedwait.c
+++ b/nptl/sem_timedwait.c
@@ -30,8 +30,8 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
        return -1;
      }

-  if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
+  if (__new_sem_wait_fast (to_new_sem (sem), 0) == 0)
      return 0;
    else
-    return __new_sem_wait_slow((struct new_sem *) sem, abstime);
+    return __new_sem_wait_slow(to_new_sem (sem), abstime);
  }
diff --git a/nptl/sem_wait.c b/nptl/sem_wait.c
index c1fd10c..3dade0c 100644
--- a/nptl/sem_wait.c
+++ b/nptl/sem_wait.c
@@ -22,10 +22,10 @@
  int
  __new_sem_wait (sem_t *sem)
  {
-  if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
+  if (__new_sem_wait_fast (to_new_sem (sem), 0) == 0)
      return 0;
    else
-    return __new_sem_wait_slow((struct new_sem *) sem, NULL);
+    return __new_sem_wait_slow(to_new_sem (sem), NULL);
  }
  versioned_symbol (libpthread, __new_sem_wait, sem_wait, GLIBC_2_1);

@@ -65,7 +65,7 @@ __new_sem_trywait (sem_t *sem)
  {
    /* We must not fail spuriously, so require a definitive result even if this
       may lead to a long execution time.  */
-  if (__new_sem_wait_fast ((struct new_sem *) sem, 1) == 0)
+  if (__new_sem_wait_fast (to_new_sem (sem), 1) == 0)
      return 0;
    __set_errno (EAGAIN);
    return -1;
diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h
index 8f5cfa4..45ba99a 100644
--- a/sysdeps/nptl/internaltypes.h
+++ b/sysdeps/nptl/internaltypes.h
@@ -168,6 +168,22 @@ struct new_sem
  #endif
  };

+/* The sem_t alignment is typically just 4 bytes for ILP32, whereas
+   new_sem is 8 bytes.  For better atomic performance on platforms
+   that tolerate unaligned atomics (and to make it work at all on
+   platforms that don't), round up the new_sem pointer within the
+   sem_t object to an 8-byte boundary.
+
+   Note that in this case, the "pad" variable at the end of the
+   new_sem object extends beyond the end of the memory allocated
+   for the sem_t, so it's important that it not be initialized
+   or otherwise used.  */
+#if __HAVE_64B_ATOMICS && !defined (_LP64)
+# define to_new_sem(s) ((struct new_sem *)(((uintptr_t)(s) + 4) & -8))
+#else
+# define to_new_sem(s) ((struct new_sem *)(s))
+#endif
+
  struct old_sem
  {
    unsigned int value;

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com