[llvm-branch-commits] [flang] [flang][OpenMP] Fix subtle bug in GetAffectedNestDepthWithReason (PR #190645)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

<details>
<summary>Changes</summary>

For constructs that allow COLLAPSE or ORDERED clauses, the function would return an empty value for the affected depth if none of these clauses were actually present. What should happen is that the return value should be 1 without a specific reason.

This bug was not detectable with any source program, since the empty value caused depth checks to be skipped. Detecting the problem would require a loop nest with a lower depth than needed that the bug would cause not to be diagnosed. Since the correct value was 1, such a loop would need to have a depth of 0 and such a nest cannot be constructed.

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

---
Full diff: https://github.com/llvm/llvm-project/pull/190645.diff


1 Files Affected:

- (modified) flang/lib/Semantics/openmp-utils.cpp (+15-8) 


``````````diff
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 33707f9d0d95b..1d9de49e6cd8e 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -850,17 +850,24 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
       dir, llvm::omp::Clause::OMPC_ordered, version)};
 
   if (allowsCollapse || allowsOrdered) {
-    auto [count, reason]{GetArgumentValueWithReason(
+    auto [ccount, creason]{GetArgumentValueWithReason(
         spec, llvm::omp::Clause::OMPC_collapse, version)};
-    auto [vo, ro]{GetArgumentValueWithReason(
+    auto [ocount, oreason]{GetArgumentValueWithReason(
         spec, llvm::omp::Clause::OMPC_ordered, version)};
-    if (vo) {
-      if (!count || *count < *vo) {
-        count = vo;
-        reason = std::move(ro);
-      }
+    // Ignore invalid arguments.
+    if (ccount <= 0) {
+      ccount = std::nullopt;
+      creason = Reason();
+    }
+    if (ocount <= 0) {
+      ocount = std::nullopt;
+      oreason = Reason();
+    }
+    if (ccount < ocount) {
+      // `ocount` cannot be std::nullopt here (C++ std guarantee).
+      return {{ocount.value_or(1), std::move(oreason)}, true};
     }
-    return {{count, std::move(reason)}, true};
+    return {{ccount.value_or(1), std::move(creason)}, true};
   }
 
   if (IsLoopTransforming(dir)) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/190645