Re: [PATCH] Native spinlock support on RISC-V

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Marek Szuba <marecki(at)gentoo(dot)org>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Native spinlock support on RISC-V
Date: 2021-08-13 17:55:53
Message-ID: 20210813175553.vkqmjqdvty5aqa6h@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2021-08-13 13:37:02 -0400, Tom Lane wrote:
> "Andres Freund" <andres(at)anarazel(dot)de> writes:
> > On Fri, Aug 13, 2021, at 19:25, Tom Lane wrote:
> >> I now have looked at the patch, and it seems good as far as it goes,
> >> but I wonder whether some effort ought to be expended in
> >> src/include/port/atomics/.
>
> > That should automatically pick up the intrinsic. I think we should do the same on modern compilers for spinlocks, but that's a separate discussion I guess.
>
> I was looking at the comment in atomics.h about
>
> * Provide a full fallback of the pg_*_barrier(), pg_atomic**_flag and
> * pg_atomic_* APIs for platforms without sufficient spinlock and/or atomics
> * support. In the case of spinlock backed atomics the emulation is expected
> * to be efficient, although less so than native atomics support.
>
> so it seems like someday we might want to expend some effort on native
> atomics. I agree that that day need not be today, though. This patch
> seems sufficient until we get to the point of (at least) having some
> RISC-V in the buildfarm.

For gcc compatible compilers the relevant comments would be

* There exist generic, hardware independent, implementations for several
* compilers which might be sufficient, although possibly not optimal, for a
* new platform. If no such generic implementation is available spinlocks (or
* even OS provided semaphores) will be used to implement the API.

and

/*
* Compiler specific, but architecture independent implementations.
*
* Provide architecture independent implementations of the atomic
* facilities. At the very least compiler barriers should be provided, but a
* full implementation of
* * pg_compiler_barrier(), pg_write_barrier(), pg_read_barrier()
* * pg_atomic_compare_exchange_u32(), pg_atomic_fetch_add_u32()
* using compiler intrinsics are a good idea.
*/

Gcc's intriniscs are pretty good these days (and if not, a *lot* of
projects just outright break).

What's more, It turns out that using intrinsics with compilers of the
last ~5 years often generates *better* code than inline assembly
(e.g. because the compiler can utilize condition codes more effectively
and has more detailed information about clobbers). So for new platforms
that'll only have support by new compilers it doesn't really make sense
to add inline assembler imo.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-08-13 18:09:04 Re: [PATCH] Native spinlock support on RISC-V
Previous Message Tom Lane 2021-08-13 17:37:02 Re: [PATCH] Native spinlock support on RISC-V