Re: [LTP] [PATCH v3 1/3] tst_atomic: drop legacy inline assembly and use __atomic or __sync builtins

[Petr Vorel <pvorel@suse.cz>]

> On Fri, Jun 6, 2025 at 10:39 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Li, Cyril,

> > > Refactor tst_atomic.h to remove all legacy architecture-specific inline
> > > assembly and fallback code paths. The new implementation supports only
> > > two well-defined interfaces: __atomic_* built-ins (GCC ≥ 4.7) and
> > __sync_*
> > > built-ins (GCC ≥ 4.1).

> > > This simplification improves maintainability, clarity, and portability
> > > across platforms.

> > > The memory order is explicitly set to __ATOMIC_SEQ_CST to preserve strict
> > > sequential consistency, which aligns with the C++11 memory model.

> > > Reference:
> > https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
> > +1

> > Thanks for the work!
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > > Signed-off-by: Li Wang <liwang@redhat.com>
> > > Suggested-by: Cyril Hrubis <chrubis@suse.cz>

> > > +
> > > +/* Use __atomic built-ins (GCC >= 4.7), with sequential consistency. */

> > +1 for documenting this. Hopefully these are old enough to not having to
> > mention
> > it in the docs. But if you think it's not that old, it might be worth
> > later to
> > document it in doc/users/supported_systems.rst.

> > https://linux-test-project.readthedocs.io/en/latest/users/supported_systems.html

> > In that case minimal clang version should be noted as well.

> > BTW I tried to search, should it be clang 3.5?
> > I tried to search but not found.
> > https://clang.llvm.org/docs/LanguageExtensions.html#c11-atomic-builtins



> https://releases.llvm.org/3.1/docs/ReleaseNotes.html
> From the release notes, Clang was implemented __atomic builtins in v3.1.

> And you made a good point, we also need to note the minimal Clang version,
> Something like:

>   /* Use __atomic built-ins (GCC >= 4.7, Clang >= 3.1), with sequential
> consistency. */

+1 (no need to repost, please just add it before merge).


> P.s.

> The worth to mention github CI also passed with remove __sync built-ins,
> but to be on safe side, I still keep them in, maybe for someone's use on
> older
> platform.

+1

Kind regards,
Petr

> https://github.com/wangli5665/ltp/commit/3b1b81e662c49b04b79c2bdf807d818984a2dbd9

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp