[PATCH] D102449: [WIP][Clang][OpenMP] Add the support for compare clause in atomic directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/AST/StmtOpenMP.h:2797-2824
+  enum DataPositionTy : size_t {
+    POS_X = 0,
+    POS_V,
+    POS_D,
+    POS_E,
+    POS_UpdateExpr,
+    POS_CondExpr,
----------------
It is worth it to pre-commit these and related changes in a separate NFC patch


================
Comment at: clang/include/clang/AST/StmtOpenMP.h:2848-2850
          ArrayRef<OMPClause *> Clauses, Stmt *AssociatedStmt, Expr *X, Expr *V,
-         Expr *E, Expr *UE, bool IsXLHSInRHSPart, bool IsPostfixUpdate);
+         Expr *E, Expr *D, Expr *Ex, Expr *UE, bool IsXLHSInRHSPart,
+         bool IsPostfixUpdate);
----------------
Can we pack it into a struct? At least `X`, `V` etc. params?


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5775-5802
+static void emitOMPAtomicCompareExpr(CodeGenFunction &CGF,
+                                     llvm::AtomicOrdering AO, const Expr *X,
+                                     const Expr *E, const Expr *D,
+                                     const Expr *CE, bool IsXLHSInRHSPart,
+                                     SourceLocation Loc) {
+  assert(X->isLValue() && "X of 'omp atomic compare' is not lvalue");
+  assert(isa<BinaryOperator>(CE->IgnoreImpCasts()) &&
----------------
I would think about moving most of the atomic codegen functionality to common runtime/OMPBuilder part to be able to override implementation for different targets. It may help to avoid codegen problems with Nvidia/AMD GPUs. Not directly related to this patch though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102449/new/

https://reviews.llvm.org/D102449