[PATCH] D82906: [flang][openmp] Use common Directive and Clause enum from llvm/Frontend

[David Truby via Phabricator via llvm-commits]

DavidTruby accepted this revision.
DavidTruby added a subscriber: ichoyjx.
DavidTruby added a comment.
This revision is now accepted and ready to land.

This looks really great to me, thanks for all the work you've done here to unify with LLVM! The more code we can share the better.

I've just left one small comment inline that I'd like looked at; otherwise I'm very happy for this to go ahead but I think @ichoyjx and/or @kiranchandramohan should review as well.

As an aside, and I think this is outside the scope of this patch, it'd be great if we could do away with the doAllowedClauses... etc altogether and somehow generate the required information for those from tablegen as well? It seems we should (in most cases) want to share what clauses are allowed on e.g. loop clauses (here called do clauses, in clang called for clauses) where possible too. Is this something you've looked at at all? I'm mostly asking as otherwise I'll take a look but don't want to double up the work!



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:16
 
-static constexpr inline OmpClauseSet doAllowedClauses{OmpClause::PRIVATE,
-    OmpClause::FIRSTPRIVATE, OmpClause::LASTPRIVATE, OmpClause::LINEAR,
-    OmpClause::REDUCTION};
-static constexpr inline OmpClauseSet doAllowedOnceClauses{
-    OmpClause::SCHEDULE, OmpClause::COLLAPSE, OmpClause::ORDERED};
-
-static constexpr inline OmpClauseSet simdAllowedClauses{OmpClause::LINEAR,
-    OmpClause::ALIGNED, OmpClause::PRIVATE, OmpClause::LASTPRIVATE,
-    OmpClause::REDUCTION};
-static constexpr inline OmpClauseSet simdAllowedOnceClauses{
-    OmpClause::COLLAPSE, OmpClause::SAFELEN, OmpClause::SIMDLEN};
-
-static constexpr inline OmpClauseSet parallelAllowedClauses{OmpClause::DEFAULT,
-    OmpClause::PRIVATE, OmpClause::FIRSTPRIVATE, OmpClause::SHARED,
-    OmpClause::COPYIN, OmpClause::REDUCTION};
-static constexpr inline OmpClauseSet parallelAllowedOnceClauses{
-    OmpClause::IF, OmpClause::NUM_THREADS, OmpClause::PROC_BIND};
-
-static constexpr inline OmpClauseSet taskloopAllowedClauses{OmpClause::SHARED,
-    OmpClause::PRIVATE, OmpClause::FIRSTPRIVATE, OmpClause::LASTPRIVATE,
-    OmpClause::DEFAULT, OmpClause::UNTIED, OmpClause::MERGEABLE,
-    OmpClause::NOGROUP};
-static constexpr inline OmpClauseSet taskloopAllowedOnceClauses{
-    OmpClause::COLLAPSE, OmpClause::IF, OmpClause::FINAL, OmpClause::PRIORITY};
-static constexpr inline OmpClauseSet taskloopAllowedExclusiveClauses{
-    OmpClause::GRAINSIZE, OmpClause::NUM_TASKS};
-
-static constexpr inline OmpClauseSet distributeAllowedClauses{
-    OmpClause::PRIVATE, OmpClause::FIRSTPRIVATE, OmpClause::LASTPRIVATE};
-static constexpr inline OmpClauseSet distributeAllowedOnceClauses{
-    OmpClause::COLLAPSE, OmpClause::DIST_SCHEDULE};
-
-static constexpr inline OmpClauseSet targetAllowedClauses{OmpClause::IF,
-    OmpClause::PRIVATE, OmpClause::FIRSTPRIVATE, OmpClause::MAP,
-    OmpClause::IS_DEVICE_PTR, OmpClause::DEPEND};
-static constexpr inline OmpClauseSet targetAllowedOnceClauses{
-    OmpClause::DEVICE, OmpClause::DEFAULTMAP, OmpClause::NOWAIT};
-
-static constexpr inline OmpClauseSet teamsAllowedClauses{OmpClause::PRIVATE,
-    OmpClause::FIRSTPRIVATE, OmpClause::SHARED, OmpClause::REDUCTION};
-static constexpr inline OmpClauseSet teamsAllowedOnceClauses{
-    OmpClause::NUM_TEAMS, OmpClause::THREAD_LIMIT, OmpClause::DEFAULT};
-
-static constexpr inline OmpClauseSet sectionsAllowedClauses{OmpClause::PRIVATE,
-    OmpClause::FIRSTPRIVATE, OmpClause::LASTPRIVATE, OmpClause::REDUCTION};
+static OmpClauseSet doAllowedClauses{llvm::omp::Clause::OMPC_private,
+    llvm::omp::Clause::OMPC_firstprivate, llvm::omp::Clause::OMPC_lastprivate,
----------------
Is there a reason the constexpr is gone here? The inline was superfluous but there to signal my intent that these should only be used in inline contexts, but the constexpr is actually useful later when we call constexpr functions with these variables.

If it's because the llvm::omp::Clause::* variables aren't themselves constexpr, can they be made so? Their values would seem to me to be known at compile time


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82906