[Openmp-commits] [PATCH] D76211: OpenMP Metadirective with user defined condition

[Alok Mishra via Phabricator via Openmp-commits]

alokmishra.besu added a comment.



In D76211#2212203 <https://reviews.llvm.org/D76211#2212203>, @jdoerfert wrote:

> Are you planning to add support for the actual OpenMP selectors as well? 
> All the other traits, scoring, etc. I left a comment below that points you to the right place as we have almost all of that already ready to be reused.
> On this note, I would recommend you take a look at the OpenMP 5.1 draft, it contains proper wording for a dynamic context extension, and we will need support in the declare variant as well. That means we need to allow dynamic expressions in `OMPTraitInfo` and similar places eventually anyway.

The idea of this patch is to implement user condition for metadirective. 
Anyways I will be adding support for other OpenMP selectors as well. I think doing that in a different patch will be a better idea, as there are lots of things to consider. 
I do take this point and will update this patch to support future extension to other selectors.

> We need more tests:
>
> - verify all parser errors
> - verify semantic errors wrt the directives
> - verify the condition is captured properly, e.g., if it is in a parallel region
> - verify complex conditions work, e.g., globals, calls, constexpr expressions, template arguments, ...
> - verify an "empty" metadirective works
> - verify we exclude directives & conditions that should not be emitted
>
>> Fixed an issue where correct code was not generated if directive variant was not provided in when/default clause
>
> We need a test for that too.

Will update all these test cases soon.
.

In D76211#2212214 <https://reviews.llvm.org/D76211#2212214>, @jdoerfert wrote:

> Update for @alokmishra.besu via arcanist as the webinterface doesn't allow patches over 2MB

Currently trying to figure out how to use arcanist. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76211