================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to