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



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

Reply via email to