[PATCH] D40545: [sanitizer] add msvc atomic_compare_exchange_strong for sint32_t

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 29 05:57:21 PST 2017


dberris added a comment.

In https://reviews.llvm.org/D40545#938984, @dvyukov wrote:

> In https://reviews.llvm.org/D40545#938979, @comicfans44 wrote:
>
> > this is used in x-ray runtime , in xray_fdr_logging.cc  ,such as  line 56:
> >
> >   if (!__sanitizer::atomic_compare_exchange_strong(
> >            &LogFlushStatus, &Result, XRayLogFlushStatus::XRAY_LOG_FLUSHING,
> >            __sanitizer::memory_order_release))
> >   
> >
> > currently x-ray didn't compile on windows,but this is a little step to make it build. I'm trying to build xray runtime on windows, some talks here:
> >  http://lists.llvm.org/pipermail/llvm-dev/2017-November/119218.html
>
>
> That's the one I referred to as "which should actually use atomic_uint32_t".
>  Please switch xray to uint32. I don't think we want to support all of 10 atomic operations X 6 memory orderings X 20 different types, that's more than 1000 combinations.
>  One day we should finally switch to C++ atomics, but until then it does not make sense to grow set of atomic operations infinitely.


I'm curious though, since that type is coercing an enum into an `int`, which is the only defined conversion as far as I can tell. While it "should" be safe to use `atomic_uint32_t` instead, I'm wondering whether it's actually safe to do that. If it is, then I'm happy with using `atomic_uint32_t` instead of an `atomic_int32_t` in XRay.

As an aside, what are the technical limitations for not using C++ atomics now? I understand the need to not depend on things in the C++ standard library that might have runtime dependencies, but the std::atomic<...> libs shouldn't really have those, right?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40545





More information about the llvm-commits mailing list