[PATCH] D91944: OpenMP 5.0 metadirective

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/AST/StmtOpenMP.h:373
+///
+class OMPMetaDirective final : public OMPExecutableDirective {
+  friend class ASTStmtReader;
----------------
alokmishra.besu wrote:
> ABataev wrote:
> > I think, metadirective should be a kind of a container for different sub-directives. The problem is that that subdirectives could be completely different, they may capture different variables, using different capture kind (by value or by reference) etc.So, you need to generate each possible sub-directive independently and store them in the meta directive node. Otherwise you won't be able to generate the code correctly.
> In OpenMP 5.0, we do not need to generate every sub-directive. Rather we need to select one (or none) directive and replace metadirective with it. So this is not needed.
> Yes with future specifications we will need to include a list of all valid directives which need to be resolved at runtime. That is when we will need to generate and store multiple sub-directives inside the OMPMetaDirective class.  
I think you still need to do it even for 5.0. I don't mean y need to emit the code for every sub-directive, but to generate them in the AST. Even in the example above, `when(user={condition(N>10)}` is used in the template and `N` is a template argument, the chosen directive can be selected on the instantiation and, thus, the original templated directive has to contain all possible sub-directives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944