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

[Brian Yang via Phabricator via llvm-commits]

ichoyjx accepted this revision.
ichoyjx marked an inline comment as done.
ichoyjx added a comment.

It took me a while to go through the changes. The merge with tablegen looks very nice!



================
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,
----------------
DavidTruby wrote:
> clementval wrote:
> > DavidTruby wrote:
> > > 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
> > It's because the enumset is now bigger than 64 and the implementation in `EnumSet.h` switch automatically to `std::bitset`. This is not possible to use `constexpr` and initialize a > 64 bitset at compile time. This willl anyway be removed in a follow up patch where it will be generated in TableGen. 
> Interesting, I didn't know EnumSet had that behaviour. If this is going to be removed in a future patch then I am not bothered about this.
> I'm not convinced that these things will be inlined in the way that I would want currently anyway.
Nice work on switching to the `std::bitset`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82906