[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

Johannes Doerfert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 11 02:34:49 PDT 2019


jdoerfert added a comment.

I left some minor notes but overall I think we should put this in and improve on it in-tree. @ABataev, would that be OK with you?



================
Comment at: lib/Parse/ParseOpenMP.cpp:748
 }
 
+/// Parses clauses for 'declare variant' directive.
----------------
Remove the "vector" parts in the descriptions and variable names below please, it is not "vector" specific.

Nit: I'd remove the note justifying the choice for a member function.


================
Comment at: lib/Parse/ParseOpenMP.cpp:788
+}
+
 /// Parse clauses for '#pragma omp declare simd'.
----------------
I actually doubt the "vec-var-id" has to be an `tok::identifier` but let's put that discussion on hold for later patches and get it in for the most common case, identifiers.


================
Comment at: lib/Sema/SemaOpenMP.cpp:4930
+}
+
 StmtResult Sema::ActOnOpenMPParallelDirective(ArrayRef<OMPClause *> Clauses,
----------------
I'm a little unsure about the template instantiation check (have to actually open the standard on that) but I guess being to conservative now and making progress is an acceptable way to go anyway.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67294/new/

https://reviews.llvm.org/D67294





More information about the cfe-commits mailing list