https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127963

            Bug ID: 127963
           Summary: std::atomic<bool> loses the requested memory_order at
                    -Os: members taking memory_order are not
                    _GLIBCXX_ALWAYS_INLINE
           Product: gcc
           Version: 17.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nebkat at gmail dot com
  Target Milestone: ---

At -Os the compiler can decline to inline std::atomic<bool>'s member functions.
The out-of-line copy then receives the memory_order as a runtime argument, so
the __atomic built-in it forwards to falls back to seq_cst: a relaxed load
becomes a call to a function that does a seq_cst load.

https://godbolt.org/z/Peffn8eez (ARM64 gcc 16.1.0 and trunk, x86-64 gcc trunk,
-std=c++20 -Os)

#include <atomic>

std::atomic<bool> b;
std::atomic<unsigned char> u;
int sink;

void bool_1() { if (b.load(std::memory_order_relaxed)) sink = 1; }
void bool_2() { if (b.load(std::memory_order_relaxed)) sink = 2; }
void bool_3() { if (b.load(std::memory_order_relaxed)) sink = 3; }
void bool_4() { if (b.load(std::memory_order_relaxed)) sink = 4; }
void bool_5() { if (b.load(std::memory_order_relaxed)) sink = 5; }
void bool_6() { if (b.load(std::memory_order_relaxed)) sink = 6; }
void bool_7() { if (b.load(std::memory_order_relaxed)) sink = 7; }
void bool_8() { if (b.load(std::memory_order_relaxed)) sink = 8; }
void bool_9() { if (b.load(std::memory_order_relaxed)) sink = 9; }
void bool_10() { if (b.load(std::memory_order_relaxed)) sink = 10; }

void uchar_1() { if (u.load(std::memory_order_relaxed)) sink = 1; }
void uchar_2() { if (u.load(std::memory_order_relaxed)) sink = 2; }
void uchar_3() { if (u.load(std::memory_order_relaxed)) sink = 3; }
void uchar_4() { if (u.load(std::memory_order_relaxed)) sink = 4; }
void uchar_5() { if (u.load(std::memory_order_relaxed)) sink = 5; }
void uchar_6() { if (u.load(std::memory_order_relaxed)) sink = 6; }
void uchar_7() { if (u.load(std::memory_order_relaxed)) sink = 7; }
void uchar_8() { if (u.load(std::memory_order_relaxed)) sink = 8; }
void uchar_9() { if (u.load(std::memory_order_relaxed)) sink = 9; }
void uchar_10() { if (u.load(std::memory_order_relaxed)) sink = 10; }

All ten bool_*() functions call an out-of-line
std::atomic<bool>::load(std::memory_order), passing relaxed (0), which it
ignores: on ARM64 it does ldarb rather than ldrb. The uchar_*() functions,
using std::atomic<unsigned char>, each get an inline ldrb. With six functions
of each the bool loads are still inlined; from seven they are outlined. On
x86-64 the calls are there too, though the load instruction is the same for
both orders. GCC 16.1.0 for xtensa-esp-elf does the same; its out-of-line load
is memw; l8ui; memw instead of l8ui.

Cause: PR 55947 raised the same fallback for HLE hints and was closed as
intended behaviour, with the advice to use always_inline where the order must
survive inlining decisions (comment 4). r198733 then marked the __atomic_base
members that take a memory_order _GLIBCXX_ALWAYS_INLINE. std::atomic<bool> and
std::atomic<_Tp*> in <atomic> hold an __atomic_base member and forward to it,
but the forwarding members were never marked, so the forwarding layer
reintroduces the problem.

Suggested fix: add _GLIBCXX_ALWAYS_INLINE to the members of atomic<bool> and
atomic<_Tp*> that take a memory_order (store, load, exchange,
compare_exchange_weak/strong, fetch_add/fetch_sub), plus wait, notify_one and
notify_all, matching what __atomic_base marks; the conversion operators and
operator= stay as they are, as in __atomic_base. With that change the bool_*()
functions get an inline ldrb too, and the testcase's __TEXT goes from 1752 to
1304 bytes (aarch64-apple-darwin). I haven't seen atomic<_Tp*> outlined in
practice, but it has the same structure. The patch is 38 one-line changes to
include/std/atomic; I can attach it or send it to gcc-patches.

Why it matters to us: on ESP32 targets, interrupt handlers that must run while
the flash cache is disabled are placed in IRAM, and an outlined atomic<bool>
member lands in flash, so the handler calls code it cannot execute.

Reply via email to