[Flang][OpenMP][OpenACC] Error for loop with no control

[kiranchandramohan]

Summary

Issue error if a DO construct associated with a loop does not have
loop control. Currently, it is issued only for the loop immediately
following the loop construct. This patch extends it to cases like
collapse where there is more than one loop associated. It also fixes
a crash since the existing code always expects loop control.

This is covered in OpenMP 4.5 standard, Section 2.7.1.
"The do-loop cannot be a DO WHILE or a DO loop without loop control."

OpenACC 3.3 covers this indirectly in Section 2.9.1.
The trip count for all loops associated with the collapse clause must
be computable and invariant in all the loops".

[clementval]

LGTM

flang/lib/Semantics/resolve-directives.cpp

protected:
   }
   void AddDataSharingAttributeObject(SymbolRef object) {
     dataSharingAttributeObjects_.insert(object);
   }
   void ClearDataSharingAttributeObjects() {
     dataSharingAttributeObjects_.clear();
   }
   bool HasDataSharingAttributeObject(const Symbol &);
-  const parser::Name &GetLoopIndex(const parser::DoConstruct &);
+  const parser::Name *GetLoopIndex(const parser::DoConstruct &);
   const parser::DoConstruct *GetDoConstructIf(
       const parser::ExecutionPartConstruct &);
   Symbol *DeclarePrivateAccessEntity(
       const parser::Name &, Symbol::Flag, Scope &);
   Symbol *DeclarePrivateAccessEntity(Symbol &, Symbol::Flag, Scope &);
   Symbol *DeclareOrMarkOtherAccessEntity(const parser::Name &, Symbol::Flag);
 
   UnorderedSymbolSet dataSharingAttributeObjects_; // on one directive

 template <typename T>
 bool DirectiveAttributeVisitor<T>::HasDataSharingAttributeObject(
     const Symbol &object) {
   auto it{dataSharingAttributeObjects_.find(object)};
   return it != dataSharingAttributeObjects_.end();
 }
 
 template <typename T>
-const parser::Name &DirectiveAttributeVisitor<T>::GetLoopIndex(
+const parser::Name *DirectiveAttributeVisitor<T>::GetLoopIndex(
     const parser::DoConstruct &x) {
   using Bounds = parser::LoopControl::Bounds;
-  return std::get<Bounds>(x.GetLoopControl()->u).name.thing;
+  if (x.GetLoopControl()) {
+    return &std::get<Bounds>(x.GetLoopControl()->u).name.thing;
+  } else {
+    context_
+        .Say(std::get<parser::Statement<parser::NonLabelDoStmt>>(x.t).source,
+            "Loop control is not present in the DO LOOP"_err_en_US)
+        .Attach(GetContext().directiveSource,
+            "associated with the enclosing LOOP construct"_en_US);
+    return nullptr;
+  }
 }
 
 template <typename T>
 const parser::DoConstruct *DirectiveAttributeVisitor<T>::GetDoConstructIf(
     const parser::ExecutionPartConstruct &x) {
   return parser::Unwrap<parser::DoConstruct>(x);
 }
 
for (const auto &entry : block) {
       }
     }
     return nullptr;
   };
 
   const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
   for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
     // go through all the nested do-loops and resolve index variables
-    const parser::Name &iv{GetLoopIndex(*loop)};
-    if (auto *symbol{ResolveAcc(iv, ivDSA, currScope())}) {
+    const parser::Name *iv{GetLoopIndex(*loop)};
+    if (iv) {
+      if (auto *symbol{ResolveAcc(*iv, ivDSA, currScope())}) {
         symbol->set(Symbol::Flag::AccPreDetermined);
-      iv.symbol = symbol; // adjust the symbol within region
+        iv->symbol = symbol; // adjust the symbol within region
         AddToContextObjectWithDSA(*symbol, ivDSA);
       }
+    }
 
     const auto &block{std::get<parser::Block>(loop->t)};
     loop = getNextDoConstruct(block);
   }
   CHECK(level == 0);
 }
 
 void AccAttributeVisitor::EnsureAllocatableOrPointer(

 // Loop iteration variables are not well defined for DO WHILE loop.
 // Use of DO CONCURRENT inside OpenMP construct is unspecified behavior
 // till OpenMP-5.0 standard.
 // In above both cases we skip the privatization of iteration variables.
 bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
   // TODO:[OpenMP 5.1] DO CONCURRENT indices are private
   if (x.IsDoNormal()) {
     if (!dirContext_.empty() && GetContext().withinConstruct) {
-      if (const auto &iv{GetLoopIndex(x)}; iv.symbol) {
-        if (!iv.symbol->test(Symbol::Flag::OmpPreDetermined)) {
-          ResolveSeqLoopIndexInParallelOrTaskConstruct(iv);
+      const parser::Name *iv{GetLoopIndex(x)};
+      if (iv && iv->symbol) {
+        if (!iv->symbol->test(Symbol::Flag::OmpPreDetermined)) {
+          ResolveSeqLoopIndexInParallelOrTaskConstruct(*iv);
         } else {
           // TODO: conflict checks with explicitly determined DSA
         }
         ordCollapseLevel--;
         if (ordCollapseLevel) {
-          if (const auto *details{iv.symbol->detailsIf<HostAssocDetails>()}) {
+          if (const auto *details{iv->symbol->detailsIf<HostAssocDetails>()}) {
             const Symbol *tpSymbol = &details->symbol();
             if (tpSymbol->test(Symbol::Flag::OmpThreadprivate)) {
-              context_.Say(iv.source,
+              context_.Say(iv->source,
                   "Loop iteration variable %s is not allowed in THREADPRIVATE."_err_en_US,
-                  iv.ToString());
+                  iv->ToString());
             }
           }
         }
       }
     }
   }
   return true;
 }
if (!llvm::omp::simdSet.test(GetContext().directive)) {
     ivDSA = Symbol::Flag::OmpLinear;
   } else {
     ivDSA = Symbol::Flag::OmpLastPrivate;
   }
 
   const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
   for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
     // go through all the nested do-loops and resolve index variables
-    const parser::Name &iv{GetLoopIndex(*loop)};
-    if (auto *symbol{ResolveOmp(iv, ivDSA, currScope())}) {
+    const parser::Name *iv{GetLoopIndex(*loop)};
+    if (iv) {
+      if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) {
         symbol->set(Symbol::Flag::OmpPreDetermined);
-      iv.symbol = symbol; // adjust the symbol within region
+        iv->symbol = symbol; // adjust the symbol within region
         AddToContextObjectWithDSA(*symbol, ivDSA);
       }
 
       const auto &block{std::get<parser::Block>(loop->t)};
       const auto it{block.begin()};
       loop = it != block.end() ? GetDoConstructIf(*it) : nullptr;
     }
+  }
   CheckAssocLoopLevel(level, GetAssociatedClause());
 }
 void OmpAttributeVisitor::CheckAssocLoopLevel(
     std::int64_t level, const parser::OmpClause *clause) {
   if (clause && level != 0) {
     context_.Say(clause->source,
         "The value of the parameter in the COLLAPSE or ORDERED clause must"
         " not be larger than the number of nested loops"

flang/test/Semantics/OpenACC/acc-loop.f90

program openacc_loop_validity
   !ERROR: Clause IF is not allowed after clause DEVICE_TYPE on the PARALLEL directive
   !$acc parallel device_type(*) if(.TRUE.)
   !$acc loop
   do i = 1, N
     a(i) = 3.14
   end do
   !$acc end parallel
 
+  !$acc loop collapse(2)
+  do i = 1, N
+    !ERROR: Loop control is not present in the DO LOOP
+    do
+      a(i) = 3.14
+    end do
+  end do
 end program openacc_loop_validity

flang/test/Semantics/OpenMP/do-collapse.f90

do j = 1, 10
       !ERROR: The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct.
       !$omp do collapse(2)
       do k = 1, 10
         print *, "hello"
       end do
       !$omp end do
     end do
   end do
+
+  !$omp parallel do collapse(2)
+    do i = 1, 3
+      !ERROR: Loop control is not present in the DO LOOP
+      do
+      end do
+    end do
 end program omp_doCollapse
 

flang/test/Semantics/OpenMP/omp-do-collapse1.f90

+! RUN: not %flang_fc1 -fdebug-unparse-with-symbols -fopenmp %s 2>&1 | FileCheck %s
+! OpenMP Version 4.5
+! 2.7.1 Loop Construct
+program omp_doCollapse
+  integer:: i
+  !$omp parallel do collapse(2)
+    do i = 1, 3
+      !CHECK: Loop control is not present in the DO LOOP
+      !CHECK: associated with the enclosing LOOP construct
+      do
+      end do
+    end do
+end program omp_doCollapse
+