[PATCH] [OPENMP] First changes for Parsing and Sema for 'omp simd' directive support

Dmitri Gribenko gribozavr at gmail.com
Mon Feb 17 06:53:31 PST 2014



================
Comment at: include/clang/AST/StmtOpenMP.h:609
@@ +608,3 @@
+  /// \param CollapsedNum Number of collapsed nested loops.
+  /// \param N Number of clauses.
+  ///
----------------
It would be better to rename 'N' to 'NumClauses' (here and in other constructors).

================
Comment at: lib/Sema/SemaOpenMP.cpp:746
@@ +745,3 @@
+
+  getCurFunction()->setHasBranchProtectedScope();
+  return Owned(OMPSimdDirective::Create(Context, StartLoc, EndLoc,
----------------
Please add a test for this.

================
Comment at: test/OpenMP/simd_misc_messages.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only -ferror-limit 100000 -fopenmp -verify %s
+
----------------
Is -ferror-limit really needed here?


================
Comment at: lib/Serialization/ASTReaderStmt.cpp:2247-2252
@@ +2246,8 @@
+    case STMT_OMP_SIMD_DIRECTIVE: {
+      unsigned Val = Record[ASTStmtReader::NumStmtFields];
+      S = OMPSimdDirective::CreateEmpty(
+            Context,
+            Val,
+            Record[ASTStmtReader::NumStmtFields + 1],
+            Empty);
+      }
----------------
The name 'Val' is obscure.  Please use the same name as in the constructor.  Same for the number of collapsed loops.


http://llvm-reviews.chandlerc.com/D2815



More information about the cfe-commits mailing list