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

Alexander Musman alexander.musman at gmail.com
Tue Feb 18 00:10:00 PST 2014



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

================
Comment at: lib/Sema/SemaOpenMP.cpp:746
@@ +745,3 @@
+
+  getCurFunction()->setHasBranchProtectedScope();
+  return Owned(OMPSimdDirective::Create(Context, StartLoc, EndLoc,
----------------
Dmitri Gribenko wrote:
> Please add a test for this.
I've added a test (in simd_misc_messages.c)

================
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);
+      }
----------------
Dmitri Gribenko wrote:
> The name 'Val' is obscure.  Please use the same name as in the constructor.  Same for the number of collapsed loops.
Fixed.

================
Comment at: test/OpenMP/simd_misc_messages.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only -ferror-limit 100000 -fopenmp -verify %s
+
----------------
Dmitri Gribenko wrote:
> Is -ferror-limit really needed here?
> 
It seems unnesessary, I removed it for now (we can add it later with new tests if needed).


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



More information about the cfe-commits mailing list