[Openmp-commits] [PATCH] D153556: [OPENMP52] Initial support for doacross clause.

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:4415-4439
+  } else if (Kind == OMPC_doacross) {
+    // Handle dependence type for the doacross clause.
+    ColonProtectionRAIIObject ColonRAII(*this);
+    Data.ExtraModifier = getOpenMPSimpleClauseType(
+        Kind, Tok.is(tok::identifier) ? PP.getSpelling(Tok) : "",
+        getLangOpts());
+    Data.ExtraModifierLoc = Tok.getLocation();
----------------
jyu2 wrote:
> ABataev wrote:
> > Can it be unified with depenbd clause parsing? (Maybe in a separate template function)
> I don't really has an idea on how to combine this two with template function.  Since depend clause in ordered is deprecated in 52, and will be removed, should we leave as this?
Even ff it will be removed in 52, it will still stay for OpenMP < 52. Would be good to try to unify it.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:20694-20700
+    auto *C = OMPDoacrossClause::Create(
+        Context, StartLoc, LParenLoc, EndLoc,
+        IsSource ? OMPC_DOACROSS_source : OMPC_DOACROSS_sink, DepLoc, ColonLoc,
+        Vars, TotalDepCount.getZExtValue());
+    if (DSAStack->isParentOrderedRegion())
+      DSAStack->addDoacrossDependClause(C, OpsOffs);
+    return C;
----------------
Better to create clauses in ActOnDoAcross and ActOnDepend, this function better to return required data as a struct/class/bolean, etc.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:20701
+    return C;
+  } else {
+    auto *C = OMPDependClause::Create(
----------------
No need for else


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

https://reviews.llvm.org/D153556