[PATCH] D91944: OpenMP 5.0 metadirective

[Alok Mishra via Phabricator via cfe-commits]

alokmishra.besu added inline comments.


================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1952
-      STMT_MS_DEPENDENT_EXISTS,   // MSDependentExistsStmt
-      EXPR_LAMBDA,                // LambdaExpr
       STMT_COROUTINE_BODY,
----------------
jdoerfert wrote:
> alokmishra.besu wrote:
> > jdoerfert wrote:
> > > Unrelated.
> > Only STMT_OMP_META_DIRECTIVE was added. Rest was formatted by git clang-format
> > Rest was formatted by git clang-format
> 
> Please don't format unrelated code. 
Removed.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2264
+          Actions.StartOpenMPDSABlock(DirKind, DirName, Actions.getCurScope(),
+                                      Loc);
+          int paren = 0;
----------------
jdoerfert wrote:
> alokmishra.besu wrote:
> > jdoerfert wrote:
> > > Should we not go back to the original code handling "directives" instead? This looks like it is copied here.
> > Unfortunately we cannot go to the original code handling since the original code handling assumes that the directive always ends with annot_pragma_openmp_end, while here it will always end with ')'.
> > In specification 5.0, since we are choosing only 1 directive, the body of the while block remains the same as the original code. Only the condition of the while block changes. In specification 5.1, we will need to generate code for dynamic handling and even the body will differ as we might need to generate AST node for multiple directives. It is best if we handle this code here for easier handling of 5.1 code, than in the original code space.
> > I will add a TODO comment here.
> > Unfortunately we cannot go to the original code handling since the original code handling assumes that the directive always ends with annot_pragma_openmp_end, while here it will always end with ')'.
> 
> Let's add a flag to the original handling to make this possible then. Copying it is going to create more long term problems.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944