[sanitizer] add msvc atomic_compare_exchange_strong for sint32_t

[comicfans44]

Summary

It is short enough , so just a copy/paste from uint32_t

[dvyukov]

Where do you want to use this?
I see only 2 uses of atomic_sint32_t, at least one of which should actually use atomic_uint32_t.

[comicfans44]

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

[dvyukov]

In 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.

[dberris]

In D40545#938984, @dvyukov wrote:

In 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?

[dvyukov]

In D40545#938998, @dberris wrote:

In D40545#938984, @dvyukov wrote:

In 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.

The enum has values 0, 1 and 2. I don't fully understand your concern.

Well, strictly saying converting int to s32/int32_t is not safe either. int can be larger. And int does not have to be able to represent values 0, 1 and 2 as far as I see. And int32_t may not exist at all. I don't think you can beat C++ here.

We can do:

enum XRayLogFlushStatus : uint32_t {

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?

Every time I checked they were either broken or slower. I filed a bunch of bugs some time ago. I don't know what's the current status.

compiler-rt/lib/sanitizer_common/sanitizer_atomic_msvc.h

   return false;
 }
 
+INLINE bool atomic_compare_exchange_strong(volatile atomic_sint32_t *a,
+                                           s32 *cmp,
+                                           s32 xchg,
+                                           memory_order mo) {
+  s32 cmpv = *cmp;
+  s32 prev = (s32)_InterlockedCompareExchange(
+      (volatile long*)&a->val_dont_use, (long)xchg, (long)cmpv);
+  if (prev == cmpv)
+    return true;
+  *cmp = prev;
+  return false;
+}
+
 INLINE bool atomic_compare_exchange_strong(volatile atomic_uint64_t *a,
                                            u64 *cmp,
                                            u64 xchg,