Re: typo in fallback implementation for pg_atomic_test_set_flag()

On Tue, Nov 14, 2023 at 07:17:32PM -0800, Andres Freund wrote:
> Are you planning to apply the fix?

Yes, I'll take care of it.

>> I'd ordinarily suggest removing this section of code since it doesn't seem
>> to have gotten much coverage
>
> Which section precisely?

The lines below this:

/*
* provide fallback for test_and_set using atomic_exchange if available
*/
#if !defined(PG_HAVE_ATOMIC_TEST_SET_FLAG) && defined(PG_HAVE_ATOMIC_EXCHANGE_U32)

but above this:

/*
* provide fallback for test_and_set using atomic_compare_exchange if
* available.
*/
#elif !defined(PG_HAVE_ATOMIC_TEST_SET_FLAG) && defined(PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U32)

>> but I'm actually looking into adding some faster atomic-exchange
>> implementations that may activate this code for certain
>> compiler/architecture combinations.
>
> Hm. I don't really see how adding a faster atomic-exchange implementation
> could trigger this implementation being used?

That'd define PG_HAVE_ATOMIC_EXCHANGE_U32, so this fallback might be used
if PG_HAVE_ATOMIC_TEST_SET_FLAG is not defined. I haven't traced through
all the #ifdefs that lead to this point exhaustively, though, so perhaps
this is still unlikely.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com