[PATCH] D13874: Atomics: support __c11_* calls on _Atomic struct types

[Tim Northover via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Saleem, committed in r252507.


================
Comment at: lib/CodeGen/CGAtomic.cpp:782
@@ +781,3 @@
+  LValue AtomicVal = MakeAddrLValue(Ptr, AtomicTy);
+  AtomicInfo Atomics(*this, AtomicVal);
+
----------------
compnerd wrote:
> s/Atomics/AI/?  Or perhaps Atomic?
Atomics seems to be the dominant name in the file, though both that and AI are used.

================
Comment at: lib/CodeGen/CGAtomic.cpp:1021
@@ -1010,1 +1020,3 @@
+          ResVal,
+          Builder.CreateBitCast(Dest, ResVal->getType()->getPointerTo()));
     }
----------------
compnerd wrote:
> It feels like it might be nice to hoist the `CreateBitCast`.
That doesn't quite work. Some atomics don't have a Dest (e.g. c11_atomic_store produces no value) so you'd have to move the RValTy->isVoidType() check up, but others can have a void RValTy yet still need to store (e.g. if an "atomic_load(addr, &dest)" is converted into a "dest =  atomic_load_n(addr)" call).

The matrix of possibilities is a horrible mess.


Repository:
  rL LLVM

http://reviews.llvm.org/D13874