[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