Re: typo in fallback implementation for pg_atomic_test_set_flag()

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Date: 2023-11-15T15:52:34Z
Lists: pgsql-hackers
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



Commits

  1. Fix fallback implementation for pg_atomic_test_set_flag().

  2. Add a basic atomic ops API abstracting away platform/architecture details.