[m-rev.] for review: Use clang compiler intrinsics for more atomic ops.

Julien Fischer jfischer at opturion.com
Fri Aug 30 17:59:01 AEST 2024


Hi Peter,

On Thu, 29 Aug 2024 at 12:53, Peter Wang <novalazy at gmail.com> wrote:
>
> Use clang compiler intrinsics for more atomic ops.
>
> runtime/mercury_atomic_ops.h:
>     Use compiler intrinsic for MR_ATOMIC_ADD_AND_FETCH_WORD_BODY
>     when compiling with clang, instead of falling back to the
>     compare and swap implementation.
>
>     Use compiler intrinsic for MR_ATOMIC_SUB_INT_BODY
>     when compiling with clang but not targeting x86/x86-64.
>     In those cases, MR_atomic_sub_int() was not being defined,
>     and causing a link error.

Presumably on aarch64, or were you trying something else?

>     Use compiler intrinsic for MR_ATOMIC_DEC_AND_IS_ZERO_WORD_BODY
>     when compiling with clang but not targeting x86/x86-64.
>
> [Note that I don't really have a way of testing that the atomic ops work,
> as I would only be testing under emulation with QEMU.
> And, asm_fast.par.gc doesn't work with clang anyway.]

none.par.gc should work?

> diff --git a/runtime/mercury_atomic_ops.h b/runtime/mercury_atomic_ops.h
> index 1ce95f3d8..a7128d674 100644
> --- a/runtime/mercury_atomic_ops.h
> +++ b/runtime/mercury_atomic_ops.h
> @@ -1,7 +1,7 @@
>  // vim: ts=4 sw=4 expandtab ft=c
>
>  // Copyright (C) 2007, 2009-2011 The University of Melbourne.
> -// Copyright (C) 2016, 2018 The Mercury team.
> +// Copyright (C) 2016, 2018, 2021, 2024 The Mercury team.
>  // This file is distributed under the terms specified in COPYING.LIB.
>
>  // mercury_atomic.h - defines atomic operations and other primitives used by
> @@ -189,7 +189,7 @@ MR_EXTERN_INLINE MR_bool    MR_atomic_dec_and_is_zero_uint(
>
>  ////////////////////////////////////////////////////////////////////////////
>
> -#if (MR_GNUC > 4 || (MR_GNUC == 4 && __GNUC_MINOR__ >= 1)) &&           \
> +#if (defined(MR_CLANG) || MR_GNUC > 4 || (MR_GNUC == 4 && __GNUC_MINOR__ >= 1)) && \
>      !defined(MR_AVOID_COMPILER_INTRINSICS)

The first condition in that conjunction is repeated several times and
is getting quite complicated.
Can we hide it all behind another macro and use that instead?

The diff looks fine otherwise.

Julien.


More information about the reviews mailing list