[PATCH] D91944: OpenMP 5.0 metadirective

[Deepak Eachempati via Phabricator via cfe-commits]

dreachem added a comment.

In D91944#2414364 <https://reviews.llvm.org/D91944#2414364>, @jdoerfert wrote:

> This looks close to an OpenMP 5.0 implementation. I left comments inlined.
>
> We need tests that show how non-selected alternatives *do not* impact the program. As an example, a template instantiation inside of a non-selected alternative is not actually performed.
>
> We also need test with ill-formed metadirectives.

@jdoerfert I'm still trying to understand this thing regarding template instantiations. The spec says that a directive variant associated with a when clause can only affect the program if it is a dynamic replacement candidate. I had assumed this is referring to the runtime behavior of the program, and not (for instance) whether a compiler error is emitted due to a failing static assert.

Also, in order to determine what are the dynamic replacement candidates, and their order, all specified score expressions would need to be evaluated.  Then, you'd need to evaluate the static context selectors, in decreasing order of their score, to find which one is the last dynamic replacement candidate. So I think template instantiations could be possible for those expressions, even if the corresponding variants aren't selected?



================
Comment at: clang/include/clang/AST/StmtOpenMP.h:373
+///
+class OMPMetaDirective final : public OMPExecutableDirective {
+  friend class ASTStmtReader;
----------------
ABataev wrote:
> 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.
r


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91944