[PATCH] D100224: [flang][OpenMP] Add semantic check for occurrence of variables other than loop iteration variable in a `linear` clause associated with a `distribute` construct.

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan requested changes to this revision.
kiranchandramohan added a comment.
This revision now requires changes to proceed.

Looks mostly OK. One change request and a few nits.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:236
   CheckCycleConstraints(x);
+  if (llvm::omp::distributeSet.test(beginDir.v)) {
+    CheckDistLinear(x);
----------------
The restriction applies to only the following two cases,
    Directive::OMPD_distribute_parallel_do_simd
    Directive::OMPD_distribute_simd

For Directive::OMPD_distribute, linear is not in the list of clauses.
For Directive::OMPD_distribute_parallel_do no linear clause can be specified.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:341
+
+  // Get collase level, if given, to find which loops are "associated."
+  auto collapseVal{1};
----------------
Nit: spelling collapse.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:344-351
+  for (const auto &clause : clauses.v) {
+    if (const auto *collapseClause{
+            std::get_if<parser::OmpClause::Collapse>(&clause.u)}) {
+      if (const auto cVal{GetIntValue(collapseClause->v)}) {
+        collapseVal = *cVal;
+      }
+    }
----------------
Nit: There is a function GetOrdCollapseLevel which you can possibly use.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:400
+          context_.Say(v.source,
+              "Variable '%s' not allowed at `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`"_err_en_US,
+              v.ToString());
----------------
Nit: at -> in?


================
Comment at: flang/lib/Semantics/check-omp-structure.h:77-78
+    Directive::OMPD_distribute_parallel_do_simd,
+    Directive::OMPD_distribute_parallel_for,
+    Directive::OMPD_distribute_parallel_for_simd,
+    Directive::OMPD_distribute_simd};
----------------
Nit: Are these two fors required? Do these apply for Fortran? 


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

https://reviews.llvm.org/D100224