[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