[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

490

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

295

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.

478

Nit: spelling collapse.

481–488

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

537

Nit: at -> in?

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

82–83

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

481–488

Thanks, modified.

537

Thanks, modified

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

82–83

Thanks, removed..

[kiranchandramohan]

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

477

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

477

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

[arnamoy10]

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

477

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

477

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

477

Thanks, updated as per suggestion.

[kiranchandramohan]

LGTM. I have a nit suggestion.

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

504

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 teamSet{Directive::OMPD_teams,
     Directive::OMPD_teams_distribute,
     Directive::OMPD_teams_distribute_parallel_do,
     Directive::OMPD_teams_distribute_parallel_do_simd,
     Directive::OMPD_teams_distribute_parallel_for,
     Directive::OMPD_teams_distribute_parallel_for_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 OmpDirectiveSet nestedWorkshareErrSet{
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);
   void CheckSIMDNest(const parser::OpenMPConstruct &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);

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);
   HasInvalidDistributeNesting(x);
   if (CurrentDirectiveIsNested() &&
       llvm::omp::teamSet.test(GetContextParent().directive)) {
     HasInvalidTeamsNesting(beginDir.v, beginDir.source);
   }
+  if ((beginDir.v == llvm::omp::Directive::OMPD_distribute_parallel_do_simd) ||
+      (beginDir.v == llvm::omp::Directive::OMPD_distribute_simd)) {
+    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) {
+
+  const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
+  const auto &clauses{std::get<parser::OmpClauseList>(beginLoopDir.t)};
+
+  semantics::UnorderedSymbolSet indexVars;
+
+  // Collect symbols of all the variables from linear clauses
+  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) {
+        indexVars.insert(*(v.symbol));
+      }
+    }
+  }
+
+  if (indexVars.empty())
+    return;
+
+  // Get collapse level, if given, to find which loops are "associated."
+  std::int64_t collapseVal{GetOrdCollapseLevel(x)};
+  // Include the top loop if no collapse is specified
+  if (collapseVal == 0)
+    collapseVal = 1;
+
+  // Match the loop index variables with the collected symbols from linear
+  // clauses.
+  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) {
+          // Remove the symbol from the collcted set
+          indexVars.erase(*(itrVal.symbol));
+        }
+        collapseVal--;
+        if (collapseVal == 0)
+          break;
+      }
+      // 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;
+    }
+  }
+
+  // Show error for the remaining variables
+  for (auto var : indexVars) {
+    const Symbol &root{GetAssociationRoot(var)};
+    context_.Say(parser::FindSourceLocation(x),
+        "Variable '%s' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`"_err_en_US,
+        root.name());
+  }
+}
+
 void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
   if (llvm::omp::simdSet.test(GetContext().directive)) {
     ExitSIMDNest();
   }
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OmpEndLoopDirective &x) {

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 in `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 in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !ERROR: Variable 'b' not allowed in `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 in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !ERROR: Variable 'b' not allowed in `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 
+
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+  !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+  !$omp distribute simd linear(i,j)
+   do i = 1, N
+      do j = 1, N
+         a = 3.14
+      enddo
+   enddo
+   !$omp end distribute simd
+
+   !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+   !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE`
+   !$omp distribute simd linear(i,j) collapse(1)
+   do i = 1, N
+      do j = 1, N
+         a = 3.14
+      enddo
+   enddo
+   !$omp end distribute simd
+
+   !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+   !$omp distribute simd linear(i,j) collapse(2)
+   do i = 1, N
+      do j = 1, N
+         a = 3.14
+      enddo
+   enddo
+   !$omp end distribute simd
+
+END SUBROUTINE LINEAR_BAD