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/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
