rnk marked an inline comment as done.
rnk added inline comments.

================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:336
+
+  // For Release ordering, the failure ordering should be Monotonic.
+  auto FailureOrdering = SuccessOrdering == AtomicOrdering::Release
----------------
thakis wrote:
> Why?
Honestly, I don't know. It's copied from above.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:360
+                                              SuccessOrdering, 
FailureOrdering);
+  CXI->setVolatile(true);
+
----------------
thakis wrote:
> why?
Truly, I don't think we should mark these volatile, but MSVC marks the 
destination pointer as volatile. I think it would be fine if we treated 
_Interlocked* atomic instructions as regular atomic instructions. I guess we 
just mark them volatile for safety. There are a few atomic instr optimizations, 
and volatile blocks them.


================
Comment at: clang/test/CodeGen/ms-intrinsics.c:441
                                         ++ExchangeLow, ++ComparandResult);
 }
+// CHECK-64: define{{.*}}i8 @test_InterlockedCompareExchange128(i64*{{[a-z_ 
]*}}%Destination, i64{{[a-z_ ]*}}%ExchangeHigh, i64{{[a-z_ ]*}}%ExchangeLow, 
i64*{{[a-z_ ]*}}%ComparandResult){{.*}}{
----------------
thakis wrote:
> should this test some of the new aarc64-only intrins too if `__aarch64__`?
Yeah, let's add them, but just check for the key cmpxchg instruction.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92062/new/

https://reviews.llvm.org/D92062

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to