Re: [PATCH v2 07/14] LoongArch: Atomic and Locking Routines

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 31/12/2021 03:44, caiyinyu wrote:
> This patch implements various atomic and locking routines on LoongArch.
> ---
>  .../loongarch/nptl/bits/pthreadtypes-arch.h   |  41 ++++
>  sysdeps/loongarch/nptl/bits/semaphore.h       |  32 +++
>  sysdeps/loongarch/nptl/libc-lowlevellock.c    |   8 +
>  .../sysv/linux/loongarch/atomic-machine.h     | 196 ++++++++++++++++++
>  4 files changed, 277 insertions(+)
>  create mode 100644 sysdeps/loongarch/nptl/bits/pthreadtypes-arch.h
>  create mode 100644 sysdeps/loongarch/nptl/bits/semaphore.h
>  create mode 100644 sysdeps/loongarch/nptl/libc-lowlevellock.c
>  create mode 100644 sysdeps/unix/sysv/linux/loongarch/atomic-machine.h
> 
> diff --git a/sysdeps/loongarch/nptl/bits/pthreadtypes-arch.h b/sysdeps/loongarch/nptl/bits/pthreadtypes-arch.h
> new file mode 100644
> index 0000000000..e57f122a98
> --- /dev/null
> +++ b/sysdeps/loongarch/nptl/bits/pthreadtypes-arch.h
> @@ -0,0 +1,41 @@
> +/* Machine-specific pthread type layouts.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _BITS_PTHREADTYPES_ARCH_H
> +#define _BITS_PTHREADTYPES_ARCH_H 1
> +
> +#include <bits/endian.h>
> +
> +#ifdef __loongarch_lp64
> +#define __SIZEOF_PTHREAD_ATTR_T 56
> +#define __SIZEOF_PTHREAD_MUTEX_T 40
> +#define __SIZEOF_PTHREAD_MUTEXATTR_T 4
> +#define __SIZEOF_PTHREAD_COND_T 48
> +#define __SIZEOF_PTHREAD_CONDATTR_T 4
> +#define __SIZEOF_PTHREAD_RWLOCK_T 56
> +#define __SIZEOF_PTHREAD_RWLOCKATTR_T 8
> +#define __SIZEOF_PTHREAD_BARRIER_T 32
> +#define __SIZEOF_PTHREAD_BARRIERATTR_T 4
> +#else
> +#error "32bit LoongArch systems are not supported"
> +#endif
> +
> +#define __LOCK_ALIGNMENT
> +#define __ONCE_ALIGNMENT
> +
> +#endif /* bits/pthreadtypes.h */

Why can't you use the generic definition?   Also, I think all the __loongarch_lp64
tests to avoid 32-bit build should be moved to a configure test instead.

> diff --git a/sysdeps/loongarch/nptl/bits/semaphore.h b/sysdeps/loongarch/nptl/bits/semaphore.h
> new file mode 100644
> index 0000000000..510f3b0e9a
> --- /dev/null
> +++ b/sysdeps/loongarch/nptl/bits/semaphore.h
> @@ -0,0 +1,32 @@
> +/* POSIX semaphore type layout
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SEMAPHORE_H
> +#error "Never use <bits/semaphore.h> directly; include <semaphore.h> instead."
> +#endif
> +
> +#define __SIZEOF_SEM_T (4 * __SIZEOF_POINTER__)
> +
> +/* Value returned if `sem_open' failed.  */
> +#define SEM_FAILED ((sem_t *) 0)
> +
> +typedef union
> +{
> +  char __size[__SIZEOF_SEM_T];
> +  long int __align;
> +} sem_t;

Why can't you use the generic linux header interface?

> diff --git a/sysdeps/loongarch/nptl/libc-lowlevellock.c b/sysdeps/loongarch/nptl/libc-lowlevellock.c
> new file mode 100644
> index 0000000000..f4b8b5c193
> --- /dev/null
> +++ b/sysdeps/loongarch/nptl/libc-lowlevellock.c
> @@ -0,0 +1,8 @@
> +/* This kludge works around a libpthread static linking problem:
> +   https://sourceware.org/bugzilla/show_bug.cgi?id=15648.  */
> +
> +#ifndef SHARED
> +#define __lll_lock_wait_private weak_function __lll_lock_wait_private
> +#endif
> +
> +#include <lowlevellock.c>

Why this file is required?

> diff --git a/sysdeps/unix/sysv/linux/loongarch/atomic-machine.h b/sysdeps/unix/sysv/linux/loongarch/atomic-machine.h
> new file mode 100644
> index 0000000000..dbae608ee5
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/loongarch/atomic-machine.h
> @@ -0,0 +1,196 @@
> +/* Atomic operations.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _LINUX_LOONGARCH_BITS_ATOMIC_H
> +#define _LINUX_LOONGARCH_BITS_ATOMIC_H 1
> +
> +#include <stdint.h>
> +
> +typedef int32_t atomic32_t;
> +typedef uint32_t uatomic32_t;
> +
> +typedef int64_t atomic64_t;
> +typedef uint64_t uatomic64_t;
> +
> +typedef intptr_t atomicptr_t;
> +typedef uintptr_t uatomicptr_t;
> +typedef intmax_t atomic_max_t;
> +typedef uintmax_t uatomic_max_t;

These definition are not required any longer.

> +
> +#define atomic_full_barrier() __sync_synchronize ()
> +
> +#ifdef __LP64__
> +#define __HAVE_64B_ATOMICS 1
> +#endif

I think there is no need to guard it for __LP64__.