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

[arnamoy10]

Summary

Implement the following semantic check:

"A list item may not appear in a linear clause, unless it is the loop iteration variable."

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

351

You have to check whether the nested loops are associated with the distribute simd construct. If they are associated then they should be picked up, otherwise they should not be. Association here is probably only through the collapse clause.

For e.g: The following test should provide an error.

!$omp distribute simd linear(i,j) 
do i = 1, N
  do j = 1, N
     a = 3.14
  enddo
enddo
!$omp end distribute simd

The following test should not.

!$omp distribute simd collapse(2) linear(i,j) 
 do i = 1, N
   do j = 1, N
      a = 3.14
   enddo
 enddo
 !$omp end distribute simd

[arnamoy10]

Update as per reviewers' comments to check "association" of loops with the distribute construct based on the value in the collapse clause.

Also added a test case.

[kiranchandramohan]

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

flang/lib/Semantics/check-omp-structure.cpp

236

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.

339

Nit: spelling collapse.

342–349

Nit: There is a function GetOrdCollapseLevel which you can possibly use.

398

Nit: at -> in?

flang/lib/Semantics/check-omp-structure.h

77–78

Nit: Are these two fors required? Do these apply for Fortran?

[arnamoy10]

Addressing reviewers comments:

1. Change the directive set to check for
2. Use of existing functions to get the collapse level.
3. Minor typo fixes.

flang/lib/Semantics/check-omp-structure.cpp

342–349

Thanks, modified.

398

Thanks, modified

flang/lib/Semantics/check-omp-structure.h

77–78

Thanks, removed..

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

338

Would it be possible to go through the linear clause first? If there is no linear clause there is no need to go through the iteration variables.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

338

If you do this check in Leave then you can probably use FindClause/FindClauses.

[arnamoy10]

flang/lib/Semantics/check-omp-structure.cpp

338

Unfortunately, the check cannot be done in Leave(), because EndLoopDirective is Enter()-ed before, which erases the clauselist using ResetPartialContext(). So FindClause/FindClauses cannot retrieve the clause list anymore

So checking for linear clause first also is not helpful here.

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.cpp

338

OK.
What do you think about the following?
Collect all symbols in the linear clause in linearSet. If not empty, go through the associated loops and remove iteration variable symbols from the linearSet. For anything remaining in the linearSet, print errors.
This will only be a rearrangement of the current code.

[arnamoy10]

Updating the algorithm as per suggestion, to skip collection of loop induction variable if no linear clause is found

flang/lib/Semantics/check-omp-structure.cpp

338

Thanks, updated as per suggestion.

[kiranchandramohan]

LGTM. I have a nit suggestion.

flang/lib/Semantics/check-omp-structure.cpp

365

Nit: I think early returns are only sparingly used in the flang code.

flang/lib/Semantics/check-omp-structure.h

static OmpDirectiveSet simdSet{Directive::OMPD_distribute_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_simd, Directive::OMPD_target_simd,
     Directive::OMPD_taskloop_simd,
     Directive::OMPD_teams_distribute_parallel_do_simd,
     Directive::OMPD_teams_distribute_simd};
 static OmpDirectiveSet taskGeneratingSet{
     OmpDirectiveSet{Directive::OMPD_task} | taskloopSet};
 static OmpDirectiveSet nestedOrderedErrSet{Directive::OMPD_critical,
     Directive::OMPD_ordered, Directive::OMPD_atomic, Directive::OMPD_task,
     Directive::OMPD_taskloop};
 static OmpClauseSet privateSet{
     Clause::OMPC_private, Clause::OMPC_firstprivate, Clause::OMPC_lastprivate};
 static OmpClauseSet privateReductionSet{
     OmpClauseSet{Clause::OMPC_reduction} | privateSet};
 } // namespace omp
 } // namespace llvm
 
 namespace Fortran::semantics {
private:
   void SetLoopInfo(const parser::OpenMPLoopConstruct &x);
   void CheckIsLoopIvPartOfClause(
       llvmOmpClause clause, const parser::OmpObjectList &ompObjectList);
   void CheckWorkshareBlockStmts(const parser::Block &, parser::CharBlock);
 
   void CheckLoopItrVariableIsInt(const parser::OpenMPLoopConstruct &x);
   void CheckDoWhile(const parser::OpenMPLoopConstruct &x);
   void CheckCycleConstraints(const parser::OpenMPLoopConstruct &x);
+  void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
   std::int64_t GetOrdCollapseLevel(const parser::OpenMPLoopConstruct &x);
   void CheckIfDoOrderedClause(const parser::OmpBlockDirective &blkDirectiv);
   bool CheckReductionOperators(const parser::OmpClause::Reduction &);
   bool CheckIntrinsicOperator(
       const parser::DefinedOperator::IntrinsicOperator &);
   void CheckReductionTypeList(const parser::OmpClause::Reduction &);
   void CheckReductionArraySection(const parser::OmpObjectList &ompObjectList);
   void CheckIntentInPointerAndDefinable(

flang/lib/Semantics/check-omp-structure.cpp

void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
   if (const auto &doConstruct{
           std::get<std::optional<parser::DoConstruct>>(x.t)}) {
     const auto &doBlock{std::get<parser::Block>(doConstruct->t)};
     CheckNoBranching(doBlock, beginDir.v, beginDir.source);
   }
   CheckDoWhile(x);
   CheckLoopItrVariableIsInt(x);
   CheckCycleConstraints(x);
+  CheckDistLinear(x);
 }
 const parser::Name OmpStructureChecker::GetLoopIndex(
     const parser::DoConstruct *x) {
   using Bounds = parser::LoopControl::Bounds;
   return std::get<Bounds>(x->GetLoopControl()->u).name.thing;
 }
 void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) {
   if (const auto &loopConstruct{

 
 void OmpStructureChecker::CheckCycleConstraints(
     const parser::OpenMPLoopConstruct &x) {
   std::int64_t ordCollapseLevel{GetOrdCollapseLevel(x)};
   OmpCycleChecker ompCycleChecker{context_, ordCollapseLevel};
   parser::Walk(x, ompCycleChecker);
 }
 
+void OmpStructureChecker::CheckDistLinear(
+    const parser::OpenMPLoopConstruct &x) {
+  OmpDirectiveSet distributeSet{llvm::omp::Directive::OMPD_distribute,
+      llvm::omp::Directive::OMPD_distribute_parallel_do,
+      llvm::omp::Directive::OMPD_distribute_parallel_do_simd,
+      llvm::omp::Directive::OMPD_distribute_parallel_for,
+      llvm::omp::Directive::OMPD_distribute_parallel_for_simd,
+      llvm::omp::Directive::OMPD_distribute_simd};
+  const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
+  const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
+
+  if (distributeSet.test(beginDir.v)) {
+    // Extract iteration variables of all the loops in a map
+    semantics::UnorderedSymbolSet indexVars;
+
+    if (const auto &loopConstruct{
+            std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+
+      for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
+        if (loop->IsDoNormal()) {
+          const parser::Name &itrVal{GetLoopIndex(loop)};
+
+          if (itrVal.symbol) {
+            indexVars.insert(*(itrVal.symbol));
+          }
+        }
+        // Get the next DoConstruct if block is not empty.
+        const auto &block{std::get<parser::Block>(loop->t)};
+        const auto it{block.begin()};
+        loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
+                                 : nullptr;
+      }
+    }
+
+    // A variable may not appear in a linear clause, unless it is the loop
+    // iteration variable.
+    const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
+    const auto &clauses{std::get<parser::OmpClauseList>(beginLoopDir.t)};
+
+    for (const auto &clause : clauses.v) {
+      if (const auto *linearClause{
+              std::get_if<parser::OmpClause::Linear>(&clause.u)}) {
+
+        std::list<parser::Name> values;
+        // Get the variant type
+        if (std::holds_alternative<parser::OmpLinearClause::WithModifier>(
+                linearClause->v.u)) {
+          const auto &withM{std::get<parser::OmpLinearClause::WithModifier>(
+              linearClause->v.u)};
+          values = withM.names;
+        } else {
+          const auto &withOutM{
+              std::get<parser::OmpLinearClause::WithoutModifier>(
+                  linearClause->v.u)};
+          values = withOutM.names;
+        }
+        for (auto const &v : values) {
+          // Check if the variable is not inside the collected index variables
+          // of the loop
+          if (indexVars.count(*(v.symbol)) < 1) {
+            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());
+          }
+        }
+      }
+    }
+  }
+}
+
 void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OmpEndLoopDirective &x) {
   const auto &dir{std::get<parser::OmpLoopDirective>(x.t)};
   ResetPartialContext(dir.source);
   switch (dir.v) {

flang/test/Semantics/omp-linear-iter.f90

+! RUN: %S/test_errors.sh %s %t %flang -fopenmp
+! OpenMP Version 4.5
+! Various checks with the ordered construct
+
+SUBROUTINE LINEAR_GOOD(N)
+  INTEGER N, i, j, a, b(10)
+  !$omp target
+  !$omp teams
+  !$omp distribute parallel do simd linear(i) 
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute parallel do simd
+  !$omp end teams
+  !$omp end target
+END SUBROUTINE LINEAR_GOOD
+
+SUBROUTINE LINEAR_BAD(N)
+  INTEGER N, i, j, a, b(10)
+
+  !$omp target
+  !$omp teams
+  !ERROR: Variable 'j' not allowed at `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !$omp distribute parallel do simd linear(j) 
+  do i = 1, N
+      a = 3.14
+  enddo
+  !$omp end distribute parallel do simd
+  !$omp end teams
+  !$omp end target
+
+  !$omp target
+  !$omp teams
+  !ERROR: Variable 'j' not allowed at `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !ERROR: Variable 'b' not allowed at `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !$omp distribute parallel do simd linear(j) linear(b)
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute parallel do simd
+  !$omp end teams
+  !$omp end target 
+
+  !$omp target
+  !$omp teams
+  !ERROR: Variable 'j' not allowed at `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !ERROR: Variable 'b' not allowed at `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !$omp distribute parallel do simd linear(j, b)
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute parallel do simd
+  !$omp end teams
+  !$omp end target 
+
+END SUBROUTINE LINEAR_BAD