[OpenMP] Apply COLLAPSE to innermost leaf that allows it

[kparzysz]

As per the wording from 5.2, the COLLAPSE clause applies once to the entire construct. The 6.0 spec has a somewhat similar wording with the same intent. In practice, apply the clause to the innermost leaf constituent that allows it, without requiring it to be the exact innermost leaf.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

As per the wording from 5.2, the COLLAPSE clause applies once to the entire construct. The 6.0 spec has a somewhat similar wording with the same intent. In practice, apply the clause to the innermost leaf constituent that allows it, without requiring it to be the exact innermost leaf.

---

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

1 Files Affected:

• (modified) llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h (+1-12)

diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
index 3918cecfc1e65..c8eebbf42a68e 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
@@ -501,18 +501,7 @@ template <typename C, typename H>
 bool ConstructDecompositionT<C, H>::applyClause(
     const tomp::clause::CollapseT<TypeTy, IdTy, ExprTy> &clause,
     const ClauseTy *node) {
-  // Apply "collapse" to the innermost directive. If it's not one that
-  // allows it flag an error.
-  if (!leafs.empty()) {
-    auto &last = leafs.back();
-
-    if (llvm::omp::isAllowedClauseForDirective(last.id, node->id, version)) {
-      last.clauses.push_back(node);
-      return true;
-    }
-  }
-
-  return false;
+  return applyToInnermost(node);
 }
 
 // DEFAULT

[tblah]

Is there no way to test this?

[kparzysz]

This is really a no-op change.

I took all the compound names from https://www.openmp.org/wp-content/uploads/OpenMP-API-Compound-Directives-6-0.pdf, deleted all directives that don't contain any of the leaf directives that allow collapse (i.e. distribute, do, for, loop, simd, taskloop). In the remaining set, every directive ended with a leaf that was one of those six. In other words, the "innermost leaf that allows collapse" is always the actual innermost leaf.

The reason I made this change is that the failure to apply collapse had its own error code. The error code was that the actual innermost leaf does not allow the clause, which is not really what the spec says about collapse. I changed it to something I already had an error code for and one that makes more sense.

[tblah]

Thank you for explaining

[llvm-ci]

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/25406

Here is the relevant piece of the build log for the reference

Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/164/418' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/unittests/Support/./SupportTests-LLVM-Unit-1104929-164-418.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=418 GTEST_SHARD_INDEX=164 /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/unittests/Support/./SupportTests
--

Script:
--
/home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/unittests/Support/./SupportTests --gtest_filter=ProgramEnvTest.TestLockFileExclusive
--
Note: Google Test filter = ProgramEnvTest.TestLockFileExclusive
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ProgramEnvTest
../llvm/llvm/unittests/Support/ProgramTest.cpp:594: Failure
Value of: fs::tryLockFile(FD2, std::chrono::seconds(0), fs::LockKind::Shared)
  Actual: true
Expected: false

../llvm/llvm/unittests/Support/ProgramTest.cpp:651: Failure
Value of: Error.empty()
  Actual: false
Expected: true


../llvm/llvm/unittests/Support/ProgramTest.cpp:651
Value of: Error.empty()
  Actual: false
Expected: true



********************