[PATCH] D154180: [OPENMP52] Codegen support for doacross clause.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11382-11383
 
 void CGOpenMPRuntime::emitDoacrossOrdered(CodeGenFunction &CGF,
-                                          const OMPDependClause *C) {
+                                          const OMPClause *CL) {
+  const OMPDependClause *DC =
----------------
I suggest to create template static function, specialized for `OMPDependClause` and `OMPDoacrossClause`to avoid all those multiple selects.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11383
 void CGOpenMPRuntime::emitDoacrossOrdered(CodeGenFunction &CGF,
-                                          const OMPDependClause *C) {
+                                          const OMPClause *CL) {
+  const OMPDependClause *DC =
----------------
Just `C` is better, or `Cl`


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11384-11387
+  const OMPDependClause *DC =
+      isa<OMPDependClause>(CL) ? dyn_cast<OMPDependClause>(CL) : nullptr;
+  const OMPDoacrossClause *DoC =
+      isa<OMPDoacrossClause>(CL) ? dyn_cast<OMPDoacrossClause>(CL) : nullptr;
----------------
```
const auto *DepC = dyn_cast<OMPDependClause>(CL);
const auto *DoC = dyn_cast<OMPDoacrossClause>(CL);
```


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11410-11429
+  if (DoC) {
+    if (DoC->getDependenceType() == OMPC_DOACROSS_source) {
+      RTLFn = OMPBuilder.getOrCreateRuntimeFunction(
+          CGM.getModule(), OMPRTL___kmpc_doacross_post);
+    } else {
+      assert(DoC->getDependenceType() == OMPC_DOACROSS_sink);
+      RTLFn = OMPBuilder.getOrCreateRuntimeFunction(
----------------
Try to unify this code, avoid duplications


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5845-5846
+                          llvm::OpenMPIRBuilder &OMPBuilder) {
+  auto DOC = dyn_cast<OMPDoacrossClause>(C);
+  auto DC = dyn_cast<OMPDependClause>(C);
+
----------------
const auto *


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5874-5889
     if (S.hasClausesOfKind<OMPDependClause>()) {
       // The ordered directive with depend clause.
       assert(!S.hasAssociatedStmt() &&
              "No associated statement must be in ordered depend construct.");
       InsertPointTy AllocaIP(AllocaInsertPt->getParent(),
                              AllocaInsertPt->getIterator());
+      for (const auto *DC : S.getClausesOfKind<OMPDependClause>())
----------------
Same, try to unify the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154180