[PATCH] D21564: [OpenMP] Initial implementation of parse and sema for composite pragma 'distribute parallel for'

Alexey Bataev via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 21 21:13:58 PDT 2016


ABataev added inline comments.

================
Comment at: include/clang/AST/StmtOpenMP.h:2884-2896
@@ +2883,15 @@
+
+  /// Increment expression for distribute loop (OMPLoopDirective contains
+  /// increment expression for #for loop)
+  Expr *DistIncExpr;
+
+  /// \brief EnsureUpperBound for #pragma omp for: expression LB = PrevUB;
+  Expr *PrevEUBExpr;
+
+  Expr *getDistInc() const { return DistIncExpr; }
+  Expr *getPrevEnsureUpperBound() const { return PrevEUBExpr; }
+
+protected:
+  void setDistInc(Expr *DistInc) { DistIncExpr = DistInc; }
+  void setPrevEnsureUpperBound(Expr *PrevEUB) { PrevEUBExpr = PrevEUB; }
+};
----------------
Don't like the idea of public fields with protected setters. If these fields are required, they must be the part of the base OMPLoopDirective and reuse the logic for other helper expressions.

Also, I believe, these fields are required for codegen. If so, it is better to add them in codegen patch

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:1868-1871
@@ -1867,1 +1867,6 @@
 
+void CodeGenFunction::EmitOMPDistributeParallelForDirective(
+    const OMPDistributeParallelForDirective &S) {
+  // TODO: codegen for distribute parallel for.
+}
+
----------------
Could you just emit the captured statement as is, without dropping it?


http://reviews.llvm.org/D21564





More information about the cfe-commits mailing list