[flang][OpenMP] Check if loop nest/sequence is well-formed (#188025)

Check if the code associated with a nest or sequence construct is well
formed. Emit diagnostic messages if not.

Make a clearer separation for checks of loop-nest-associated and loop-
sequence-associated constructs.

Unify structure of some of the more common messages.

Issue: https://github.com/llvm/llvm-project/issues/185287

flang/include/flang/Semantics/openmp-utils.h

@@ -203,6 +203,9 @@ struct LoopSequence {
   const Depth &depth() const { return depth_; }
   const std::vector<LoopSequence> &children() const { return children_; }
 
+  WithReason<bool> isWellFormedSequence() const;
+  WithReason<bool> isWellFormedNest() const;
+
 private:
   using Construct = ExecutionPartIterator::Construct;
 

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

@@ -61,16 +61,6 @@ public:
       constructNamesAndLevels_.emplace(
           constructName.value().ToString(), level_);
     }
-    if (level_ >= 0) {
-      if (dc.IsDoWhile()) {
-        context_.Say(doStmt.source,
-            "The associated loop of a loop-associated directive cannot be a DO WHILE."_err_en_US);
-      }
-      if (!dc.GetLoopControl()) {
-        context_.Say(doStmt.source,
-            "The associated loop of a loop-associated directive cannot be a DO without control."_err_en_US);
-      }
-    }
     return true;
   }
 
@@ -268,93 +258,84 @@ void OmpStructureChecker::CheckNestedConstruct(
 
   // Check constructs contained in the body of the loop construct.
   auto &body{std::get<parser::Block>(x.t)};
+
   for (auto &stmt : BlockRange(body, BlockRange::Step::Over)) {
     if (auto *d{parser::Unwrap<parser::CompilerDirective>(stmt)}) {
       context_.Say(d->source,
           "Compiler directives are not allowed inside OpenMP loop constructs"_warn_en_US);
-    } else if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(stmt)}) {
-      if (!IsLoopTransforming(omp->BeginDir().DirId())) {
-        context_.Say(omp->source,
-            "Only loop-transforming OpenMP constructs are allowed inside OpenMP loop constructs"_err_en_US);
-      }
-      if (IsFullUnroll(*omp)) {
-        context_.Say(x.source,
-            "OpenMP loop construct cannot apply to a fully unrolled loop"_err_en_US);
-      }
-    } else if (!parser::Unwrap<parser::DoConstruct>(stmt)) {
-      parser::CharBlock source{parser::GetSource(stmt).value_or(x.source)};
-      context_.Say(source,
-          "OpenMP loop construct can only contain DO loops or loop-nest-generating OpenMP constructs"_err_en_US);
     }
   }
 
   LoopSequence sequence(body, version, true);
 
-  // Check if a loop-nest-associated construct has only one top-level loop
-  // in it.
+  auto assoc{llvm::omp::getDirectiveAssociation(dir)};
   auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)};
+  auto haveLength{sequence.length()};
 
-  if (auto haveLength{sequence.length()}) {
-    if (*haveLength.value == 0) {
-      context_.Say(beginSource,
-          "This construct should contain a DO-loop or a loop-nest-generating OpenMP construct"_err_en_US);
-    } else {
-      auto assoc{llvm::omp::getDirectiveAssociation(dir)};
-      if (*haveLength.value > 1 && assoc == llvm::omp::Association::LoopNest) {
-        auto &msg{context_.Say(beginSource,
-            "This construct applies to a loop nest, but has a loop sequence of "
-            "length %" PRId64 ""_err_en_US,
-            *haveLength.value)};
-        haveLength.reason.AttachTo(msg);
-      }
-      if (assoc == llvm::omp::Association::LoopSeq) {
-        if (auto requiredCount{GetRequiredCount(needRange.value)}) {
-          if (*requiredCount > 0 && *haveLength.value < *requiredCount) {
-            auto &msg{context_.Say(beginSource,
-                "This construct requires a sequence of %" PRId64
-                " loops, but the loop sequence has a length of %" PRId64
-                ""_err_en_US,
-                *requiredCount, *haveLength.value)};
-            haveLength.reason.AttachTo(msg);
-            needRange.reason.AttachTo(msg);
-          }
-        }
-      }
+  const auto MsgShouldContainDoOr{
+      "This construct should contain a DO-loop or a loop-%s-generating construct"_err_en_US};
+  const auto MsgRequiresCanonical{
+      "This construct requires a canonical loop %s"_err_en_US};
+
+  if (assoc == llvm::omp::Association::LoopNest) {
+    if (sequence.children().size() == 0) {
+      context_.Say(beginSource, MsgShouldContainDoOr, "nest");
+    } else if (haveLength.value > 1) {
+      auto &msg{context_.Say(beginSource,
+          "This construct applies to a loop nest, but has a loop sequence of "
+          "length %" PRId64 ""_err_en_US,
+          *haveLength.value)};
+      haveLength.reason.AttachTo(msg);
+    }
+    auto [isWellFormed, whyNot]{sequence.isWellFormedNest()};
+    if (isWellFormed && !*isWellFormed) {
+      auto &msg{context_.Say(beginSource, MsgRequiresCanonical, "nest")};
+      whyNot.AttachTo(msg);
     }
-  }
 
-  // Check requirements on nest depth.
-  auto [needDepth, needPerfect]{
-      GetAffectedNestDepthWithReason(beginSpec, version)};
-  auto &[haveSema, havePerf]{sequence.depth()};
+    // Check requirements on nest depth.
+    auto [needDepth, needPerfect]{
+        GetAffectedNestDepthWithReason(beginSpec, version)};
+    auto &[haveSema, havePerf]{sequence.depth()};
+
+    auto haveDepth{needPerfect ? havePerf : haveSema};
+    std::string_view perfectTxt{needPerfect ? " perfect" : ""};
 
-  if (dir != llvm::omp::Directive::OMPD_fuse) {
-    auto haveDepth = needPerfect ? havePerf : haveSema;
     // If the present depth is 0, it's likely that the construct doesn't
     // have any loops in it, which would be diagnosed above.
     if (needDepth && haveDepth.value > 0) {
       if (*needDepth.value > *haveDepth.value) {
-        if (needPerfect) {
+        auto &msg{context_.Say(beginSource,
+            "This construct requires a%s nest of depth %" PRId64
+            ", but the associated nest is a%s nest of depth %" PRId64
+            ""_err_en_US,
+            perfectTxt, *needDepth.value, perfectTxt, *haveDepth.value)};
+        haveDepth.reason.AttachTo(msg);
+        needDepth.reason.AttachTo(msg);
+      }
+    }
+
+  } else if (assoc == llvm::omp::Association::LoopSeq) {
+    if (haveLength.value == 0) {
+      context_.Say(beginSource, MsgShouldContainDoOr, "sequence");
+    } else {
+      auto [isWellFormed, whyNot]{sequence.isWellFormedSequence()};
+      if (isWellFormed && !*isWellFormed) {
+        auto &msg{context_.Say(beginSource, MsgRequiresCanonical, "sequence")};
+        whyNot.AttachTo(msg);
+      }
+      if (auto requiredCount{GetRequiredCount(needRange.value)}) {
+        if (*requiredCount > 0 && haveLength.value < *requiredCount) {
           auto &msg{context_.Say(beginSource,
-              "This construct requires a perfect nest of depth %" PRId64
-              ", but the associated nest is a perfect nest of depth %" PRId64
+              "This construct requires a sequence of %" PRId64
+              " loops, but the loop sequence has a length of %" PRId64
               ""_err_en_US,
-              *needDepth.value, *haveDepth.value)};
-          haveDepth.reason.AttachTo(msg);
-          needDepth.reason.AttachTo(msg);
-        } else {
-          auto &msg{context_.Say(beginSource,
-              "This construct requires a nest of depth %" PRId64
-              ", but the associated nest has a depth of %" PRId64 ""_err_en_US,
-              *needDepth.value, *haveDepth.value)};
-          haveDepth.reason.AttachTo(msg);
-          needDepth.reason.AttachTo(msg);
+              *requiredCount, *haveLength.value)};
+          haveLength.reason.AttachTo(msg);
+          needRange.reason.AttachTo(msg);
         }
       }
     }
-  } else {
-    // FUSE requires a sequence of perfect nests.
-    // TODO: Defer this check for now.
   }
 }
 

flang/lib/Semantics/openmp-utils.cpp

@@ -554,6 +554,13 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) {
       instance.u);
 }
 
+static const auto MsgNotValidAffectedLoop{
+    "%s is not a valid affected loop"_because_en_US};
+static const auto MsgClauseAbsentAssume{
+    "%s clause was not specified, %s is assumed"_because_en_US};
+static const auto MsgConstructDoesNotResult{
+    "%s does not result in %s"_because_en_US};
+
 Reason::Reason(const Reason &other) { //
   CopyFrom(other);
 }
@@ -587,7 +594,7 @@ WithReason<int64_t> GetArgumentValueWithReason(
         Reason reason;
         reason.Say(clause->source,
             "%s clause was specified with argument %" PRId64 ""_because_en_US,
-            name.c_str(), *value);
+            name, *value);
         return {*value, std::move(reason)};
       }
     }
@@ -603,7 +610,7 @@ static WithReason<int64_t> GetNumArgumentsWithReasonForType(
     Reason reason;
     reason.Say(clause.source,
         "%s clause was specified with %" PRId64 " arguments"_because_en_US,
-        name.c_str(), num);
+        name, num);
     return {num, std::move(reason)};
   }
   return {};
@@ -879,9 +886,8 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
       std::string name{parser::omp::GetUpperName(
           llvm::omp::Clause::OMPC_permutation, version)};
       Reason reason;
-      reason.Say(spec.source,
-          "%s clause was not specified, %s(2, 1) was assumed"_because_en_US,
-          name.c_str(), name.c_str());
+      reason.Say(
+          spec.source, MsgClauseAbsentAssume, name, "a permutation (2, 1)");
       return {{2, std::move(reason)}, true};
     }
     case llvm::omp::Directive::OMPD_stripe:
@@ -901,9 +907,7 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
       std::string name{
           parser::omp::GetUpperName(llvm::omp::Clause::OMPC_depth, version)};
       Reason reason;
-      reason.Say(spec.source,
-          "%s clause was not specified, a value of 1 was assumed"_because_en_US,
-          name.c_str());
+      reason.Say(spec.source, MsgClauseAbsentAssume, name, "a value of 1");
       return {{1, std::move(reason)}, true};
     }
     case llvm::omp::Directive::OMPD_reverse:
@@ -941,15 +945,14 @@ WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
       reason.Say(clause->source,
           "%s clause was specified with a count of %" PRId64
           " starting at loop %" PRId64 ""_because_en_US,
-          name.c_str(), *count, *first);
+          name, *count, *first);
       return {std::make_pair(*first, *count), std::move(reason)};
     }
     // If LOOPRANGE was not found, return {1, -1}, where -1 means "the whole
     // associated sequence".
     Reason reason;
-    reason.Say(spec.source,
-        "%s clause was not specified, the entire sequence is affected by"_because_en_US,
-        name.c_str());
+    reason.Say(
+        spec.source, MsgClauseAbsentAssume, name, "the entire loop sequence");
     return {std::make_pair(1, -1), std::move(reason)};
   }
 
@@ -1118,8 +1121,8 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
   llvm::omp::Directive dir{beginSpec.DirId()};
   if (!IsLoopTransforming(dir)) {
     Reason reason;
-    reason.Say(beginSpec.DirName().source,
-        "This construct does not result in a loop nest or a loop sequence"_because_en_US);
+    reason.Say(beginSpec.DirName().source, MsgConstructDoesNotResult,
+        GetUpperName(dir, version_), "a loop nest or a loop sequence");
     return {0, std::move(reason)};
   }
 
@@ -1148,9 +1151,9 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
         parser::omp::FindClause(beginSpec, llvm::omp::Clause::OMPC_looprange)};
     if (!clause) {
       Reason reason;
-      reason.Say(beginSpec.DirName().source,
-          "%s clause was not specified, all loops in the sequence are fused"_because_en_US,
-          GetUpperName(llvm::omp::Clause::OMPC_looprange, version_));
+      reason.Say(beginSpec.DirName().source, MsgClauseAbsentAssume,
+          GetUpperName(llvm::omp::Clause::OMPC_looprange, version_),
+          "the entire loop sequence");
       return {1, std::move(reason)};
     }
 
@@ -1163,7 +1166,7 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
       int64_t result{1 + *nestedLength.value - *count};
       Reason reason;
       reason.Say(beginSpec.DirName().source,
-          "Out of %" PRId64 " loops, %" PRId64 " were fused"_because_en_US,
+          "Out of %" PRId64 " loops, %" PRId64 " are fused"_because_en_US,
           *nestedLength.value, *count);
       return {result, std::move(reason)};
     }
@@ -1190,6 +1193,25 @@ WithReason<int64_t> LoopSequence::getNestedLength() const {
   return sum;
 }
 
+static void ResetIfPositiveWithReason(
+    WithReason<int64_t> &quantity, const Reason &reason) {
+  if (quantity.value > 0) {
+    quantity.value = 0;
+    quantity.reason.Append(reason);
+  }
+}
+
+static void ResetIfPositiveWithReason(WithReason<int64_t> &quantity,
+    parser::CharBlock source, parser::MessageFixedText msg) {
+  if (quantity.value > 0) {
+    quantity.value = 0;
+    quantity.reason.Say(source, msg);
+  }
+}
+
+static Reason WhyNotWellFormed(
+    const parser::ExecutionPartConstruct &badCode, bool isSequence);
+
 LoopSequence::Depth LoopSequence::calculateDepths() const {
   // Get the length of the nested sequence. The invalidIC_ and opaqueIC_
   // members do not count canonical loop nests, but there can only be one
@@ -1199,38 +1221,22 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
   // entry_), and use it as the basis for the depths of entry_->owner.
   auto [semaDepth, perfDepth]{getNestedDepths()};
   if (invalidIC_) {
-    parser::CharBlock source{*parser::GetSource(*invalidIC_)};
-    if (semaDepth.value > 0) {
-      semaDepth.value = 0;
-      semaDepth.reason.Say(
-          source, "This is not a valid intervening code"_because_en_US);
-    }
-    if (perfDepth.value > 0) {
-      perfDepth.value = 0;
-      perfDepth.reason.Say(
-          source, "This is not a valid intervening code"_because_en_US);
-    }
+    auto whyNot{WhyNotWellFormed(*invalidIC_, false)};
+    ResetIfPositiveWithReason(semaDepth, whyNot);
+    ResetIfPositiveWithReason(perfDepth, whyNot);
   } else if (opaqueIC_) {
+    auto message{"This code prevents perfect nesting"_because_en_US};
     parser::CharBlock source{*parser::GetSource(*opaqueIC_)};
-    if (perfDepth.value > 0) {
-      perfDepth.value = 0;
-      perfDepth.reason.Say(
-          source, "This code prevents perfect nesting"_because_en_US);
-    }
+    ResetIfPositiveWithReason(perfDepth, source, message);
   }
   if (nestedLength.value.value_or(0) != 1) {
     // This may simply be the bottom of the loop nest. Only emit messages
     // if the depths are reset back to 0.
     if (entry_->owner) {
+      auto message{"This construct does not contain a loop nest"_because_en_US};
       parser::CharBlock source{*parser::GetSource(*entry_->owner)};
-      if (semaDepth.value > 0) {
-        semaDepth.reason.Say(source,
-            "This construct does not contain a loop nest"_because_en_US);
-      }
-      if (perfDepth.value > 0) {
-        perfDepth.reason.Say(source,
-            "This construct does not contain a loop nest"_because_en_US);
-      }
+      ResetIfPositiveWithReason(semaDepth, source, message);
+      ResetIfPositiveWithReason(perfDepth, source, message);
     }
     semaDepth.value = perfDepth.value = 0;
   }
@@ -1271,10 +1277,12 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
           if (*required == -1 || *required == *nestedLength.value) {
             return Depth{value, value};
           }
+          std::string name{
+              GetUpperName(llvm::omp::Directive::OMPD_fuse, version_)};
           Reason reason(std::move(range.reason));
-          reason.Say(beginSpec.DirName().source,
-              "%s construct results in a proper loop-sequence"_because_en_US,
-              GetUpperName(llvm::omp::Directive::OMPD_fuse, version_));
+          reason.Say(beginSpec.DirName().source, MsgConstructDoesNotResult,
+              "This " + name + " construct",
+              "a loop nest, but a proper loop sequence");
           return Depth{{1, reason}, {1, reason}};
         }
       }
@@ -1303,8 +1311,8 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
   case llvm::omp::Directive::OMPD_unroll:
     if (isFullUnroll) {
       Reason reason;
-      reason.Say(beginSpec.DirName().source,
-          "Fully unrolled loop does not result in a loop nest"_because_en_US);
+      reason.Say(beginSpec.DirName().source, MsgConstructDoesNotResult,
+          "Fully unrolled loop", "a loop nest");
       return Depth{{0, reason}, {0, reason}};
     }
     // If this is not a full unroll then look for a PARTIAL clause.
@@ -1348,4 +1356,64 @@ LoopSequence::Depth LoopSequence::getNestedDepths() const {
   }
   return children_.front().depth_;
 }
+
+static bool IsDoConcurrent(const parser::ExecutionPartConstruct &x) {
+  if (auto *loop{parser::Unwrap<parser::DoConstruct>(x)}) {
+    return loop->IsDoConcurrent();
+  }
+  return false;
+}
+
+static Reason WhyNotWellFormed(
+    const parser::ExecutionPartConstruct &badCode, bool isSequence) {
+  Reason reason;
+  parser::CharBlock source{*parser::GetSource(badCode)};
+  if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(badCode)}) {
+    if (IsFullUnroll(*omp)) {
+      reason.Say(source, MsgConstructDoesNotResult, "Fully unrolled loop",
+          isSequence ? "a loop nest or a loop sequence" : "a loop nest");
+    } else if (!IsLoopTransforming(omp->BeginDir().DirId())) {
+      reason.Say(source,
+          "Only loop-transforming constructs are allowed inside loop constructs"_because_en_US);
+    }
+    return reason;
+  }
+
+  if (auto *loop{parser::Unwrap<parser::DoConstruct>(badCode)}) {
+    if (loop->IsDoWhile()) {
+      reason.Say(source, MsgNotValidAffectedLoop, "DO WHILE loop");
+    } else if (loop->IsDoConcurrent()) {
+      reason.Say(source, MsgNotValidAffectedLoop, "DO CONCURRENT loop");
+    } else if (!loop->GetLoopControl()) {
+      reason.Say(
+          source, MsgNotValidAffectedLoop, "DO loop without loop control");
+    }
+    if (reason) {
+      return reason;
+    }
+  }
+  reason.Say(source,
+      "The %s contains code that prevents it from being canonical at this nesting level"_because_en_US,
+      isSequence ? "sequence" : "nest");
+  return reason;
+}
+
+WithReason<bool> LoopSequence::isWellFormedSequence() const {
+  const parser::ExecutionPartConstruct *badCode{
+      invalidIC_ ? invalidIC_ : opaqueIC_};
+  if (badCode) {
+    return {false, WhyNotWellFormed(*badCode, true)};
+  }
+  return {true, Reason()};
+}
+
+WithReason<bool> LoopSequence::isWellFormedNest() const {
+  // DO CONCURRENT is allowed at the top level in OpenMP 6.0+.
+  if (invalidIC_) {
+    if (version_ < 60 || !IsDoConcurrent(*invalidIC_)) {
+      return {false, WhyNotWellFormed(*invalidIC_, false)};
+    }
+  }
+  return {true, Reason()};
+}
 } // namespace Fortran::semantics::omp

flang/lib/Semantics/resolve-directives.cpp

@@ -2395,11 +2395,6 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
               context_.Say(clause->source,
                   "DO CONCURRENT loops cannot be used with the COLLAPSE clause."_err_en_US);
             }
-          } else {
-            auto &stmt =
-                std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
-            context_.Say(stmt.source,
-                "DO CONCURRENT loops cannot form part of a loop nest."_err_en_US);
           }
         }
         // go through all the nested do-loops and resolve index variables

flang/test/Parser/OpenMP/interchange-fail.f90

@@ -1,31 +0,0 @@
-! RUN: split-file %s %t
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_end1.f90 2>&1 | FileCheck %t/stray_end1.f90
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_end2.f90 2>&1 | FileCheck %t/stray_end2.f90
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_begin.f90 2>&1 | FileCheck %t/stray_begin.f90
-
-
-!--- stray_end1.f90
-! Parser error
-
-subroutine stray_end1
-  !CHECK: error: Misplaced OpenMP end-directive
-  !$omp end interchange
-end subroutine
-
-
-!--- stray_end2.f90
-
-subroutine stray_end2
-  print *
-  !CHECK: error: Misplaced OpenMP end-directive
-  !$omp end interchange
-end subroutine
-
-
-!--- stray_begin.f90
-
-subroutine stray_begin
-  !CHECK: error: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
-  !$omp interchange permutation(2,1)
-end subroutine
-

flang/test/Parser/OpenMP/tile-fail.f90

@@ -1,31 +0,0 @@
-! RUN: split-file %s %t
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_end1.f90 2>&1 | FileCheck %t/stray_end1.f90
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_end2.f90 2>&1 | FileCheck %t/stray_end2.f90
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_begin.f90 2>&1 | FileCheck %t/stray_begin.f90
-
-
-!--- stray_end1.f90
-! Parser error
-
-subroutine stray_end1
-  !CHECK: error: Misplaced OpenMP end-directive
-  !$omp end tile
-end subroutine
-
-
-!--- stray_end2.f90
-
-subroutine stray_end2
-  print *
-  !CHECK: error: Misplaced OpenMP end-directive
-  !$omp end tile
-end subroutine
-
-
-!--- stray_begin.f90
-
-subroutine stray_begin
-  !CHECK: error: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
-  !$omp tile sizes(2)
-end subroutine
-

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

@@ -29,9 +29,8 @@ program omp_doCollapse
   !BECAUSE: COLLAPSE clause was specified with argument 2
   !$omp parallel do collapse(2)
     do i = 1, 3
-      !BECAUSE: This is not a valid intervening code
+      !BECAUSE: DO loop without loop control is not a valid affected loop
       !ERROR: Loop control is not present in the DO LOOP
-      !ERROR: The associated loop of a loop-associated directive cannot be a DO without control.
       do
       end do
     end do

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

@@ -6,8 +6,7 @@ integer :: i, j
 ! ERROR: DO CONCURRENT loops cannot be used with the COLLAPSE clause.
 !$omp parallel do collapse(2)
 do i = 1, 1
-  ! BECAUSE: This is not a valid intervening code
-  ! ERROR: DO CONCURRENT loops cannot form part of a loop nest.
+  ! BECAUSE: DO CONCURRENT loop is not a valid affected loop
   do concurrent (j = 1:2)
     print *, j
   end do
@@ -21,14 +20,16 @@ do i = 1, 1
   end do
 end do
 
+! ERROR: This construct requires a canonical loop nest
 !$omp parallel do
-! ERROR: DO CONCURRENT loops cannot form part of a loop nest.
+! BECAUSE: DO CONCURRENT loop is not a valid affected loop
 do concurrent (j = 1:2)
   print *, j
 end do
 
+! ERROR: This construct requires a canonical loop nest
 !$omp loop
-! Do concurrent is explicitly allowed inside of omp loop
+! BECAUSE: DO CONCURRENT loop is not a valid affected loop
 do concurrent (j = 1:2)
   print *, j
 end do
@@ -38,7 +39,7 @@ end do
 ! ERROR: DO CONCURRENT loops cannot be used with the COLLAPSE clause.
 !$omp loop collapse(2)
 do i = 1, 1
-  ! BECAUSE: This is not a valid intervening code
+  ! BECAUSE: DO CONCURRENT loop is not a valid affected loop
   do concurrent (j = 1:2)
     print *, j
   end do

flang/test/Semantics/OpenMP/do09.f90

@@ -5,16 +5,18 @@
 
 program omp_do
   integer ::  i = 0,k
+  !ERROR: This construct requires a canonical loop nest
   !$omp do
-  !ERROR: The associated loop of a loop-associated directive cannot be a DO WHILE.
+  !BECAUSE: DO WHILE loop is not a valid affected loop
   do while (i <= 10)
     print *, "it",i
     i = i+1
   end do
   !$omp end do
 
+  !ERROR: This construct requires a canonical loop nest
   !$omp do
-  !ERROR: The associated loop of a loop-associated directive cannot be a DO WHILE.
+  !BECAUSE: DO WHILE loop is not a valid affected loop
   do while (i <= 10)
     do while (j <= 10)
       print *, "it",k

flang/test/Semantics/OpenMP/do13.f90

@@ -9,7 +9,7 @@ program omp
   !BECAUSE: COLLAPSE clause was specified with argument 3
   !$omp do  collapse(3)
   do i = 0, 10
-    !BECAUSE: This is not a valid intervening code
+    !BECAUSE: The nest contains code that prevents it from being canonical at this nesting level
     !ERROR: CYCLE statement to non-innermost associated loop of an OpenMP DO construct
     cycle
     do j = 0, 10
@@ -25,7 +25,7 @@ program omp
   !$omp do  collapse(3)
   do i = 0, 10
     do j = 0, 10
-      !BECAUSE: This is not a valid intervening code
+      !BECAUSE: The nest contains code that prevents it from being canonical at this nesting level
       !ERROR: CYCLE statement to non-innermost associated loop of an OpenMP DO construct
       cycle
       do k  = 0, 10
@@ -39,7 +39,7 @@ program omp
   !BECAUSE: COLLAPSE clause was specified with argument 2
   !$omp do  collapse(2)
   do i = 0, 10
-    !BECAUSE: This is not a valid intervening code
+    !BECAUSE: The nest contains code that prevents it from being canonical at this nesting level
     !ERROR: CYCLE statement to non-innermost associated loop of an OpenMP DO construct
     cycle
     do j = 0, 10
@@ -55,7 +55,7 @@ program omp
   !BECAUSE: COLLAPSE clause was specified with argument 2
   !$omp do  collapse(2)
   foo: do i = 0, 10
-    !BECAUSE: This is not a valid intervening code
+    !BECAUSE: The nest contains code that prevents it from being canonical at this nesting level
     !ERROR: CYCLE statement to non-innermost associated loop of an OpenMP DO construct
     cycle foo
     do j = 0, 10
@@ -72,7 +72,7 @@ program omp
   !$omp do collapse(3)
   do 60 i=1,10
     do j=1,10
-      !BECAUSE: This is not a valid intervening code
+      !BECAUSE: The nest contains code that prevents it from being canonical at this nesting level
       !ERROR: CYCLE statement to non-innermost associated loop of an OpenMP DO construct
       cycle
       do k=1,10

flang/test/Semantics/OpenMP/do21.f90

@@ -2,26 +2,26 @@
 ! Check for existence of loop following a DO directive
 
 subroutine do1
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp do
 end subroutine
 
 subroutine do2
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp parallel do
 end subroutine
 
 subroutine do3
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp simd
 end subroutine
 
 subroutine do4
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp do simd
 end subroutine
 
 subroutine do5
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp loop
 end subroutine

flang/test/Semantics/OpenMP/fuse1.f90

@@ -10,7 +10,7 @@ subroutine f
   !ERROR: This construct requires a sequence of 2 loops, but the loop sequence has a length of 1
   !BECAUSE: LOOPRANGE clause was specified with a count of 2 starting at loop 1
   !$omp fuse looprange(1, 2)
-  !BECAUSE: LOOPRANGE clause was not specified, all loops in the sequence are fused
+  !BECAUSE: LOOPRANGE clause was not specified, the entire loop sequence is assumed
   !$omp fuse
   do i = 1, 10
   end do

flang/test/Semantics/OpenMP/interchange-fail.f90

@@ -0,0 +1,18 @@
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=60
+
+subroutine stray_end1
+  !ERROR: Misplaced OpenMP end-directive
+  !$omp end interchange
+end subroutine
+
+subroutine stray_end2
+  print *
+  !ERROR: Misplaced OpenMP end-directive
+  !$omp end interchange
+end subroutine
+
+subroutine stray_begin
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
+  !$omp interchange permutation(2,1)
+end subroutine
+

flang/test/Semantics/OpenMP/interchange01.f90

@@ -6,8 +6,9 @@ subroutine on_unroll
   implicit none
   integer i, j
 
-  !ERROR: OpenMP loop construct cannot apply to a fully unrolled loop
+  !ERROR: This construct requires a canonical loop nest
   !$omp interchange
+  !BECAUSE: Fully unrolled loop does not result in a loop nest
   !$omp unroll
   do i = 1, 5
     do j = 1, 5
@@ -20,8 +21,9 @@ subroutine loop_assoc
   implicit none
   integer :: i, j
 
+  !ERROR: This construct requires a canonical loop nest
   !$omp interchange
-  !ERROR: The associated loop of a loop-associated directive cannot be a DO WHILE.
+  !BECAUSE: DO WHILE loop is not a valid affected loop
   do while (i <= 10)
     do j = 1, 5
       i = i + 1
@@ -35,7 +37,7 @@ subroutine insufficient_loops
   integer i
 
   !ERROR: This construct requires a perfect nest of depth 2, but the associated nest is a perfect nest of depth 1
-  !BECAUSE: PERMUTATION clause was not specified, PERMUTATION(2, 1) was assumed
+  !BECAUSE: PERMUTATION clause was not specified, a permutation (2, 1) is assumed
   !$omp interchange 
   do i = 1, 5
     print *, i

flang/test/Semantics/OpenMP/loop-association.f90

@@ -16,14 +16,16 @@
 10   print *, a
   !$omp end parallel
 
+  !ERROR: This construct requires a canonical loop nest
   !$omp parallel do
-  !ERROR: DO CONCURRENT loops cannot form part of a loop nest.
+  !ERROR: DO CONCURRENT loop is not a valid affected loop
   DO CONCURRENT (i = 1:N)
      a = 3.14
   END DO
 
+  !ERROR: This construct requires a canonical loop nest
   !$omp parallel do simd
-  !ERROR: The associated loop of a loop-associated directive cannot be a DO WHILE.
+  !BECAUSE: DO WHILE loop is not a valid affected loop
   outer: DO WHILE (c > 1)
      inner: do while (b > 100)
         a = 3.14
@@ -43,9 +45,10 @@
   !$OMP END PARALLEL DO
 
   c = 16
+  !ERROR: This construct requires a canonical loop nest
   !$omp parallel do
+  !BECAUSE: DO loop without loop control is not a valid affected loop
   !ERROR: Loop control is not present in the DO LOOP
-  !ERROR: The associated loop of a loop-associated directive cannot be a DO without control.
   do
      a = 3.14
      c = c - 1
@@ -104,7 +107,7 @@
   !$omp parallel do private(c)
   do i = 1, N
      do j = 1, N
-        !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+        !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
         !$omp parallel do shared(b)
         a = 3.14
      enddo
@@ -124,7 +127,7 @@
   !ERROR: Misplaced OpenMP end-directive
   !$omp end parallel do
 
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp parallel do private(c)
 5 FORMAT (1PE12.4, I10)
   do i=1, N
@@ -141,7 +144,7 @@
   !ERROR: Misplaced OpenMP end-directive
   !$omp end parallel do simd
 
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp simd
     a = i + 1
   !ERROR: Misplaced OpenMP end-directive

flang/test/Semantics/OpenMP/loop-transformation-construct01.f90

@@ -5,9 +5,10 @@
 subroutine loop_transformation_construct1
   implicit none
 
-  !ERROR: OpenMP loop construct cannot apply to a fully unrolled loop
+  !ERROR: This construct requires a canonical loop nest
   !$omp do
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !BECAUSE: Fully unrolled loop does not result in a loop nest
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !$omp unroll
 end subroutine
 
@@ -33,9 +34,9 @@ subroutine loop_transformation_construct3
   integer :: x
   integer :: v(i)
 
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct requires a canonical loop nest
   !$omp do
-  !ERROR: Only loop-transforming OpenMP constructs are allowed inside OpenMP loop constructs
+  !ERROR: Only loop-transforming constructs are allowed inside loop constructs
   !$omp parallel do
   do x = 1, i
     v(x) = v(x) * 2
@@ -52,7 +53,7 @@ subroutine loop_transformation_construct4
   do x = 1, i
     v(x) = v(x) * 2
   end do
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
   !ERROR: At least one of SIZES clause must appear on the TILE directive
   !$omp tile
 end subroutine
@@ -64,9 +65,10 @@ subroutine loop_transformation_construct5
   integer :: v(i)
 
   !$omp do
-  !ERROR: OpenMP loop construct cannot apply to a fully unrolled loop
+  !ERROR: This construct requires a canonical loop nest
   !ERROR: At least one of SIZES clause must appear on the TILE directive
   !$omp tile
+  !BECAUSE: Fully unrolled loop does not result in a loop nest
   !$omp unroll full
   do x = 1, i
     v(x) = v(x) * 2
@@ -80,9 +82,10 @@ subroutine loop_transformation_construct6
   integer :: v(i)
 
   !$omp do
-  !ERROR: OpenMP loop construct cannot apply to a fully unrolled loop
+  !ERROR: This construct requires a canonical loop nest
   !ERROR: At least one of SIZES clause must appear on the TILE directive
   !$omp tile
+  !BECAUSE: Fully unrolled loop does not result in a loop nest
   !$omp unroll
   do x = 1, i
     v(x) = v(x) * 2

flang/test/Semantics/OpenMP/loop-transformation-construct02.f90

@@ -7,7 +7,7 @@ subroutine loop_transformation_construct1
   implicit none
 
   !$omp do
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-sequence-generating construct
   !$omp fuse 
 end subroutine
 
@@ -15,7 +15,7 @@ subroutine loop_transformation_construct2
   implicit none
 
   !$omp do
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-sequence-generating construct
   !$omp fuse 
   !$omp end fuse
 end subroutine
@@ -50,7 +50,7 @@ subroutine loop_transformation_construct4
   do x = 1, i
     v(x) = v(x) * 2
   end do
-  !ERROR: This construct should contain a DO-loop or a loop-nest-generating OpenMP construct
+  !ERROR: This construct should contain a DO-loop or a loop-sequence-generating construct
   !$omp fuse
   !$omp end fuse
 end subroutine
@@ -62,8 +62,9 @@ subroutine loop_transformation_construct5
   integer :: v(i)
 
   !$omp do
-  !ERROR: OpenMP loop construct cannot apply to a fully unrolled loop
+  !ERROR: This construct requires a canonical loop sequence
   !$omp fuse
+  !BECAUSE: Fully unrolled loop does not result in a loop nest or a loop sequence
   !$omp unroll full
   do x = 1, i
     v(x) = v(x) * 2
@@ -82,7 +83,7 @@ subroutine loop_transformation_construct6
 
   !ERROR: This construct applies to a loop nest, but has a loop sequence of length 2
   !$omp do
-  !BECAUSE: Out of 2 loops, 1 were fused
+  !BECAUSE: Out of 2 loops, 1 are fused
   !$omp fuse looprange(1,1)
   !$omp unroll partial(2)
   do x = 1, i

flang/test/Semantics/OpenMP/loop-transformation-construct04.f90

@@ -10,7 +10,7 @@ subroutine loop_transformation_construct3
 
   !ERROR: This construct applies to a loop nest, but has a loop sequence of length 2
   !$omp do
-  !BECAUSE: Out of 3 loops, 2 were fused
+  !BECAUSE: Out of 3 loops, 2 are fused
   !$omp fuse looprange(1,2)
   do x = 1, i
     v(x) = x * 2
@@ -33,7 +33,7 @@ subroutine loop_transformation_construct4
 
   !ERROR: This construct applies to a loop nest, but has a loop sequence of length 2
   !$omp tile sizes(2)
-  !BECAUSE: Out of 3 loops, 2 were fused
+  !BECAUSE: Out of 3 loops, 2 are fused
   !$omp fuse looprange(1,2)
   do x = 1, i
     v(x) = x * 2

flang/test/Semantics/OpenMP/tile-fail.f90

@@ -0,0 +1,18 @@
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=60
+
+subroutine stray_end1
+  !ERROR: Misplaced OpenMP end-directive
+  !$omp end tile
+end subroutine
+
+subroutine stray_end2
+  print *
+  !ERROR: Misplaced OpenMP end-directive
+  !$omp end tile
+end subroutine
+
+subroutine stray_begin
+  !ERROR: This construct should contain a DO-loop or a loop-nest-generating construct
+  !$omp tile sizes(2)
+end subroutine
+

flang/test/Semantics/OpenMP/tile02.f90

@@ -6,8 +6,9 @@ subroutine on_unroll
   implicit none
   integer i
 
-  !ERROR: OpenMP loop construct cannot apply to a fully unrolled loop
+  !ERROR: This construct requires a canonical loop nest
   !$omp tile sizes(2)
+  !BECAUSE: Fully unrolled loop does not result in a loop nest
   !$omp unroll
   do i = 1, 5
     print *, i

flang/test/Semantics/OpenMP/tile03.f90

@@ -6,8 +6,9 @@ subroutine loop_assoc
   implicit none
   integer :: i = 0
 
+  !ERROR: This construct requires a canonical loop nest
   !$omp tile sizes(2)
-  !ERROR: The associated loop of a loop-associated directive cannot be a DO WHILE.
+  !BECAUSE: DO WHILE loop is not a valid affected loop
   do while (i <= 10)
     i = i + 1
     print *, i

flang/test/Semantics/OpenMP/tile08.f90

@@ -6,9 +6,9 @@ subroutine do_concurrent
   implicit none
   integer i, j
 
-
+  !ERROR: This construct requires a canonical loop nest
   !$omp tile sizes(2,2)
-  !ERROR: DO CONCURRENT loops cannot form part of a loop nest.
+  !BECAUSE: DO CONCURRENT loop is not a valid affected loop
   do concurrent (i = 1:42, j = 1:42)
     print *, i, j
   end do

flang/test/Semantics/OpenMP/tile09.f90

@@ -48,7 +48,7 @@ subroutine f03
   !BECAUSE: This construct does not contain a loop nest
   do i = 1, 10
     !BECAUSE: LOOPRANGE clause was specified with a count of 1 starting at loop 1
-    !BECAUSE: FUSE construct results in a proper loop-sequence
+    !BECAUSE: This FUSE construct does not result in a loop nest, but a proper loop sequence
     !$omp fuse depth(2) looprange(1, 1)
     do j = 1, 10
       do k = 1, 10