[PATCH] [OPENMP] Codegen for 'atomic capture'.

John McCall rjmccall at gmail.com
Tue Apr 21 10:29:23 PDT 2015


The code seems fine, but I'm pretty concerned about the semantics.


================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:1685
@@ +1684,3 @@
+        // 'atomicrmw' does not provide new value, so read it from 'x'.
+        NewVVal = CGF.EmitAtomicLoad(XLValue, Loc, AO, XLValue.isVolatile());
+      }
----------------
Unfortunately, this makes the operation no longer atomic.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:1700
@@ -1610,1 +1699,3 @@
+      CGF, IsSeqCst, VLValue,
+      convertToType(CGF, NewVVal, NewVValType, VLValue.getType()));
   // OpenMP, 2.12.6, atomic Construct
----------------
So, this separate store makes the entire operation decidedly non-atomic.  If the store to 'y' needs to be atomic with the rest, then this is an atomic operation across multiple locations and cannot be done locklessly.  If it doesn't need to be — if only 'x' needs to be accessed atomically — then you should not do an atomic store here.

http://reviews.llvm.org/D9049

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list