[PATCH] D118024: [sanitizer_common] Use atomic builtin in sanitizer_atomic_clang.h
Rainer Orth via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 28 02:37:13 PST 2022
ro added inline comments.
================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:78
+ return __atomic_compare_exchange(&a->val_dont_use, cmp, &xchg, false, mo,
+ __ATOMIC_RELAXED);
}
----------------
efriedma wrote:
> ro wrote:
> > efriedma wrote:
> > > Please don't change the memory ordering in this patch. (__sync_* is sequentially consistent.) If you really want to change it, please post as a separate patch with an appropriate title.
> > It wasn't clear to me from GCC's `__sync` builtin documentation that they are `__ATOMIC_SEQ_CST`. I just assumed that `scudo/standalone/atomic_helpers.h` really implemented `atomic_compare_exchange_strong`.
> >
> > As I mentioned, both `__ATOMIC_RELAXED` and `__ATOMIC_SEQ_CST` cause 3 regressions on Solaris/sparcv9, so this patch certainly cannot go in as is, despite fixing hundreds of link failures.
> Still not changed completely to use SEQ_CST. (__atomic_compare_exchange has two memory order operands.)
I'd done this based on [[ https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html#g_t_005f_005fatomic-Builtins | GCC's __atomic builtin description ]]:
> Note that the C++11 standard allows for the memory order parameter to be determined at run time rather than at compile time. These built-in functions map any run-time value to __ATOMIC_SEQ_CST rather than invoke a runtime library call or inline a switch statement. This is standard compliant, safe, and the simplest approach for now.
`mo` would thus be mapped to `__ATOMIC_SEQ_CST`. I don't know if this is just a GCC implementation detail, though, especially given that I've no idea what's the formal specification of the `__atomic` builtins, if any.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118024/new/
https://reviews.llvm.org/D118024
More information about the llvm-commits
mailing list