Skip to content

Fix ill-formed 'inline inline' in the ARM __spin_loop_pause - #2167

Open
bjornpagen wants to merge 1 commit into
NVIDIA:mainfrom
bjornpagen:fix/arm64-duplicate-inline
Open

Fix ill-formed 'inline inline' in the ARM __spin_loop_pause#2167
bjornpagen wants to merge 1 commit into
NVIDIA:mainfrom
bjornpagen:fix/arm64-duplicate-inline

Conversation

@bjornpagen

Copy link
Copy Markdown

Commit f0e8ae6 ("Create a stdexec module", #2138) changed the ARM branch of __spin_loop_pause from static to inline. This change made the definition safe to include in the module purview. After this change, the declaration reads:

STDEXEC_ATTRIBUTE(always_inline) inlinevoid__spin_loop_pause() noexcept

On GCC and Clang, STDEXEC_ATTRIBUTE(always_inline) already ends in inline. The macro STDEXEC_ATTRIBUTE_CASE_ALWAYS_INLINE expands to __attribute__((__always_inline__, __artificial__)) inline. Clang adds __nodebug__. So the declaration contains inline inline. This is ill-formed. See [dcl.spec.general]/2: "At most one of each of the decl-specifiers friend, typedef, or inline shall appear in a decl-specifier-seq."

Both compilers give a diagnostic, as the standard requires. GCC rejects the code with a hard error:

include/stdexec/__detail/__spin_loop_pause.hpp:42:36: error: duplicate 'inline'

This error breaks every GCC build on arm/aarch64 since that commit. Clang accepts the code with a warning (-Wduplicate-decl-specifier, on by default). CI stayed green because no CI leg compiles the ARM branch. The Linux and Windows runners are x86_64. The macOS jobs use macos-26-large runners, which are Intel.

This PR makes two changes. Together they make ALWAYS_INLINE guarantee inline linkage on every compiler:

  1. Remove the explicit inline at the ARM use site. The macro supplies inline on GCC and Clang. On MSVC, __forceinline implies it. The HEAD spelling __forceinline inline likely causes warning C4141 there as well.
  2. Add a plain inline to the fallback branch of STDEXEC_ATTRIBUTE_CASE_ALWAYS_INLINE. nvcc, NVHPC, and other EDG front ends take the fallback branch, because they define neither STDEXEC_CLANG() nor STDEXEC_GCC(). Before this PR, the macro expanded to nothing for them. The explicit inline at the ARM site was the only thing that kept this header-defined function ODR-safe for them. The fallback inline keeps that property. (I verified this by reading the detection chain in __config.hpp. I did not run nvcc or NVHPC.)

Site survey: the attribute has 81 uses. The ARM site was the only STDEXEC_ATTRIBUTE(always_inline) inline pairing. Two sites pair the attribute with static (the x86 and fallback-architecture branches of this same function). These become inline static on GCC and Clang. That is well-formed and keeps internal linkage.

Verification, on aarch64-apple-darwin:

  • GCC 16.1 (-std=c++26, with and without -fno-exceptions) and Clang 22 compile the header cleanly after this change.
  • At HEAD, GCC errors and Clang warns.
  • With this fix, the full default test suite builds and passes with GCC 16.1 on this host: 963/968. The 5 failures are the relacy binaries. They fail identically with and without this change, so they are environmental on this host. At HEAD, they do not build at all.
  • The -fno-exceptions test configuration from the CI matrix also passes: 879/879.

Note: the x86 and fallback-architecture branches still spell static. If the module-purview concern from #2138 applies to them, they may need the same treatment. I did not touch them, to keep this a pure build fix. I can also send a separate PR that adds an arm64 macOS CI leg (macos-26-class runners are Apple Silicon), so CI catches this class of break.

…64 build
f0e8ae6 ("Create a stdexec module (NVIDIA#2138)") changed the ARM branch of
__spin_loop_pause from `static` to `inline` so the definition is safe to
include in the module purview. But STDEXEC_ATTRIBUTE(always_inline)
already ends in `inline` on GCC and Clang
(STDEXEC_ATTRIBUTE_CASE_ALWAYS_INLINE in __config.hpp expands to
`__attribute__((__always_inline__, __artificial__)) inline`; Clang adds
`__nodebug__`), so the declaration became `inline inline` -- ill-formed
per [dcl.spec.general]/2 ("At most one of each of the decl-specifiers
friend, typedef, or inline shall appear in a decl-specifier-seq").
Both compilers diagnose it, as conformance requires: GCC rejects with a
hard error --
include/stdexec/__detail/__spin_loop_pause.hpp:42:36: error: duplicate 'inline'
-- which breaks every GCC build on arm/aarch64 since that commit, while
Clang accepts with an on-by-default -Wduplicate-decl-specifier warning.
CI stayed green because no CI leg compiles the ARM branch at all: the
Linux and Windows runners are x86_64, and the macOS jobs run on
macos-26-large runners, which are Intel.
Two changes, which together make ALWAYS_INLINE guarantee inline linkage
on every compiler:
* drop the explicit `inline` at the ARM use site (the macro supplies
it on GCC/Clang, and MSVC's `__forceinline` implies it -- the HEAD
spelling `__forceinline inline` likely draws C4141 there as well);
* give the fallback branch of STDEXEC_ATTRIBUTE_CASE_ALWAYS_INLINE a
plain `inline`. Compilers that take the fallback branch (nvcc,
NVHPC, and other EDG front ends define neither STDEXEC_CLANG() nor
STDEXEC_GCC()) previously expanded the macro to nothing, so the
explicit `inline` at the ARM site was the only thing keeping this
header-defined function ODR-safe for them; the fallback `inline`
preserves exactly that. (Read-verified against the detection chain
in __config.hpp; not run against nvcc/NVHPC.)
Site survey: the ARM site was the only
`STDEXEC_ATTRIBUTE(always_inline) inline` pairing among all 81 uses of
the attribute. The two `STDEXEC_ATTRIBUTE(always_inline) static` sites
(the x86 and fallback-architecture branches of this same function)
become `inline static` on GCC/Clang, which is well-formed and still
internal linkage.
Verified on aarch64-apple-darwin: GCC 16.1 (-std=c++26, with and
without -fno-exceptions) and Clang 22 compile the header cleanly after
this change; at HEAD, GCC errors and Clang warns. With this fix the
full default test suite builds and passes with GCC 16.1 on this host
(963/968; the 5 relacy binaries fail identically with and without this
change -- environmental on aarch64-apple-darwin -- and cannot be built
at all at HEAD, where no test TU that reaches this header compiles).
The -fno-exceptions test configuration from the CI matrix also builds
and passes (879/879).
Note: the x86 and fallback-architecture branches still spell `static`;
if NVIDIA#2138's module-purview concern applies to them as well they may want
the same treatment. Left untouched here to keep this a pure build fix.
@copy-pr-bot

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bjornpagen