rnk added inline comments.
================ Comment at: lib/CodeGen/CGBuiltin.cpp:3003 + case Builtin::BI_InterlockedCompareExchangePointer: + case Builtin::BI_InterlockedCompareExchangePointer_nf: { llvm::Type *RTy; ---------------- mgrang wrote: > rnk wrote: > > Is the no fence version really equivalent to this seq_cst version here? > I don't see InterlockedCompareExchangePointer creating a fence but it should. > (since it is the fence version). So maybe that needs to be fixed (and > possibly other fence functions). > > InterlockedCompareExchangePointer_nf should not create a fence so the current > implementation seems correct. Hm, if we're supposed to have fences, we're probably missing them everywhere. I thought the atomicrmw orderings were enough, but that shows what I know about the C++ memory model. =p We don't generate a fence for this intrinsic when MSVC does: ``` unsigned char test_arm(long *base, long idx) { return _interlockedbittestandreset_acq(base, idx); } ``` @efriedma, is MSVC over-emitting extra fences, or are we under-emitting? If this is their "bug", should we be bug-for-bug compatible with it so we get the same observable memory states? Repository: rC Clang https://reviews.llvm.org/D52807 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits