[flang-commits] [flang] 85725a6 - [Flang][OpenMP] Avoid abort when collapse clause value is negative

[Valentin Clement via flang-commits]

Author: Valentin Clement
Date: 2020-05-14T21:37:25-04:00
New Revision: 85725a67c745cb833174a7eae2aa53e74753bcb1

URL: https://github.com/llvm/llvm-project/commit/85725a67c745cb833174a7eae2aa53e74753bcb1
DIFF: https://github.com/llvm/llvm-project/commit/85725a67c745cb833174a7eae2aa53e74753bcb1.diff

LOG: [Flang][OpenMP] Avoid abort when collapse clause value is negative

Summary:
If the value in the collapse close is negative f18 abort without the correct error message. This PR change the size_t in name resolution to a int64_t and check appropriately for negative or zero before the privatization of induction variable.
The correct error is then catch by the OpenMP structure check.

This diff is migrated from the GitHub pull request https://github.com/flang-compiler/f18/pull/1098

Reviewers: ichoyjx, jdoerfert, sscalpone, DavidTruby

Reviewed By: ichoyjx, sscalpone, DavidTruby

Subscribers: sscalpone, klausler, yaxunl, guansong, llvm-commits

Tags: #llvm, #flang

Differential Revision: https://reviews.llvm.org/D77821

Added: 
    

Modified: 
    flang/lib/Semantics/resolve-names.cpp
    flang/test/Semantics/omp-clause-validity01.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 4bab50931ffb..f4768bc426c2 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1239,7 +1239,7 @@ class OmpAttributeVisitor {
     // variables on Data-sharing attribute clauses
     std::map<const Symbol *, Symbol::Flag> objectWithDSA;
     bool withinConstruct{false};
-    std::size_t associatedLoopLevel{0};
+    std::int64_t associatedLoopLevel{0};
   };
   // back() is the top of the stack
   OmpContext &GetContext() {
@@ -1272,10 +1272,10 @@ class OmpAttributeVisitor {
     return it != GetContext().objectWithDSA.end();
   }
 
-  void SetContextAssociatedLoopLevel(std::size_t level) {
+  void SetContextAssociatedLoopLevel(std::int64_t level) {
     GetContext().associatedLoopLevel = level;
   }
-  std::size_t GetAssociatedLoopLevelFromClauses(const parser::OmpClauseList &);
+  std::int64_t GetAssociatedLoopLevelFromClauses(const parser::OmpClauseList &);
 
   Symbol &MakeAssocSymbol(const SourceName &name, Symbol &prev, Scope &scope) {
     const auto pair{scope.try_emplace(name, Attrs{}, HostAssocDetails{prev})};
@@ -6526,10 +6526,10 @@ const parser::DoConstruct *OmpAttributeVisitor::GetDoConstructIf(
   return nullptr;
 }
 
-std::size_t OmpAttributeVisitor::GetAssociatedLoopLevelFromClauses(
+std::int64_t OmpAttributeVisitor::GetAssociatedLoopLevelFromClauses(
     const parser::OmpClauseList &x) {
-  std::size_t orderedLevel{0};
-  std::size_t collapseLevel{0};
+  std::int64_t orderedLevel{0};
+  std::int64_t collapseLevel{0};
   for (const auto &clause : x.v) {
     if (const auto *orderedClause{
             std::get_if<parser::OmpClause::Ordered>(&clause.u)}) {
@@ -6564,11 +6564,13 @@ std::size_t OmpAttributeVisitor::GetAssociatedLoopLevelFromClauses(
 //   - The loop iteration variables in the associated do-loops of a simd
 //     construct with multiple associated do-loops are lastprivate.
 //
-// TODO: This assumes that the do-loops association for collapse/ordered
-//       clause has been performed (the number of nested do-loops >= n).
+// TODO: revisit after semantics checks are completed for do-loop association of
+//       collapse and ordered
 void OmpAttributeVisitor::PrivatizeAssociatedLoopIndex(
     const parser::OpenMPLoopConstruct &x) {
-  std::size_t level{GetContext().associatedLoopLevel};
+  std::int64_t level{GetContext().associatedLoopLevel};
+  if (level <= 0)
+    return;
   Symbol::Flag ivDSA{Symbol::Flag::OmpPrivate};
   if (simdSet.test(GetContext().directive)) {
     if (level == 1) {

diff  --git a/flang/test/Semantics/omp-clause-validity01.f90 b/flang/test/Semantics/omp-clause-validity01.f90
index 2714f7fc4153..06fd7203763d 100644
--- a/flang/test/Semantics/omp-clause-validity01.f90
+++ b/flang/test/Semantics/omp-clause-validity01.f90
@@ -49,6 +49,15 @@
   enddo
   !$omp end parallel
 
+  !ERROR: The parameter of the COLLAPSE clause must be a constant positive integer expression
+  !$omp do collapse(-1)
+  do i = 1, N
+    do j = 1, N
+      a = 3.14
+    enddo
+  enddo
+  !$omp end do
+
   a = 1.0
   !$omp parallel firstprivate(a)
   do i = 1, N