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

Reply via email to