Skip to content

[OpenACC] 'collapse' clause 'force' tag #110906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

erichkeane
Copy link
Collaborator

The 'force' tag permits intervening code on the applicable 'loop' construct,
so this implements the restriction when the 'force' tag isn't present.

The 'force' tag permits intervening code on the applicable 'loop'
construct, so this implements the restriction when the 'force' tag isn't
present.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

The 'force' tag permits intervening code on the applicable 'loop' construct,
so this implements the restriction when the 'force' tag isn't present.


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

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
  • (modified) clang/lib/Sema/SemaOpenACC.cpp (+50)
  • (modified) clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp (+143)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index dae357eb2d370e..066c4aa13c467d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12628,6 +12628,9 @@ def err_acc_collapse_multiple_loops
 def err_acc_collapse_insufficient_loops
     : Error<"'collapse' clause specifies a loop count greater than the number "
             "of available loops">;
+def err_acc_collapse_intervening_code
+    : Error<"inner loops must be tightly nested inside a 'collapse' clause on "
+            "a 'loop' construct">;
 
 // AMDGCN builtins diagnostics
 def err_amdgcn_global_load_lds_size_invalid_value : Error<"invalid size value">;
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 47b4bd77d86d18..dfaf726891cfc8 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -1770,11 +1770,61 @@ void SemaOpenACC::ActOnForStmtBegin(SourceLocation ForLoc) {
   CollapseInfo.CurLevelHasLoopAlready = false;
 }
 
+namespace {
+SourceLocation FindInterveningCodeInCollapseLoop(const Stmt *CurStmt) {
+  // We should diagnose on anything except `CompoundStmt`, `NullStmt`,
+  // `ForStmt`, `CXXForRangeStmt`, since those are legal, and `WhileStmt` and
+  // `DoStmt`, as those are caught as a violation elsewhere.
+  // For `CompoundStmt` we need to search inside of it.
+  if (!CurStmt ||
+      isa<ForStmt, NullStmt, ForStmt, CXXForRangeStmt, WhileStmt, DoStmt>(
+          CurStmt))
+    return SourceLocation{};
+
+  // Any other construct is an error anyway, so it has already been diagnosed.
+  if (isa<OpenACCConstructStmt>(CurStmt))
+    return SourceLocation{};
+
+  // Search inside the compound statement, this allows for arbitrary nesting
+  // of compound statements, as long as there isn't any code inside.
+  if (const auto *CS = dyn_cast<CompoundStmt>(CurStmt)) {
+    for (const auto *ChildStmt : CS->children()) {
+      SourceLocation ChildStmtLoc =
+          FindInterveningCodeInCollapseLoop(ChildStmt);
+      if (ChildStmtLoc.isValid())
+        return ChildStmtLoc;
+    }
+    // Empty/not invalid compound statements are legal.
+    return SourceLocation{};
+  }
+  return CurStmt->getBeginLoc();
+}
+} // namespace
+
 void SemaOpenACC::ActOnForStmtEnd(SourceLocation ForLoc, StmtResult Body) {
   if (!getLangOpts().OpenACC)
     return;
   // Set this to 'true' so if we find another one at this level we can diagnose.
   CollapseInfo.CurLevelHasLoopAlready = true;
+
+  if (!Body.isUsable())
+    return;
+
+  if (CollapseInfo.CurCollapseCount && *CollapseInfo.CurCollapseCount > 0 &&
+      !CollapseInfo.ActiveCollapse->hasForce()) {
+    // OpenACC 3.3: 2.9.1
+    // If the 'force' modifier does not appear, then the associated loops must
+    // be tightly nested.  If the force modifier appears, then any intervening
+    // code may be executed multiple times as needed to perform the collapse.
+
+    SourceLocation OtherStmtLoc = FindInterveningCodeInCollapseLoop(Body.get());
+
+    if (OtherStmtLoc.isValid()) {
+      Diag(OtherStmtLoc, diag::err_acc_collapse_intervening_code);
+      Diag(CollapseInfo.ActiveCollapse->getBeginLoc(),
+           diag::note_acc_collapse_clause_here);
+    }
+  }
 }
 
 bool SemaOpenACC::ActOnStartStmtDirective(OpenACCDirectiveKind K,
diff --git a/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp b/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
index bbb7abf24ffdf6..953775a423cdba 100644
--- a/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
+++ b/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
@@ -336,3 +336,146 @@ void no_other_directives() {
   }
 }
 
+void call();
+
+template<unsigned Two>
+void intervening_without_force_templ() {
+  // expected-note@+1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    // expected-error@+1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+    for(;;){}
+  }
+
+  // expected-note@+1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(Two)
+  for(;;) {
+    // expected-error@+1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+    for(;;){}
+  }
+
+  // expected-note@+1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    for(;;){}
+    // expected-error@+1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+  }
+
+#pragma acc loop collapse(force:2)
+  for(;;) {
+    call();
+    for(;;){}
+  }
+
+#pragma acc loop collapse(force:Two)
+  for(;;) {
+    call();
+    for(;;){}
+  }
+
+
+#pragma acc loop collapse(force:2)
+  for(;;) {
+    for(;;){}
+    call();
+  }
+
+#pragma acc loop collapse(force:Two)
+  for(;;) {
+    for(;;){}
+    call();
+  }
+
+#pragma acc loop collapse(Two)
+  for(;;) {
+    for(;;){
+    call();
+    }
+  }
+
+#pragma acc loop collapse(Two)
+  for(;;) {
+    {
+      {
+        for(;;){
+        call();
+        }
+      }
+    }
+  }
+
+#pragma acc loop collapse(force:Two)
+  for(;;) {
+    for(;;){
+    call();
+    }
+  }
+
+  // expected-note@+1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(Two)
+  for(;;) {
+    for(;;){}
+    // expected-error@+1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+  }
+}
+
+void intervening_without_force() {
+  intervening_without_force_templ<2>(); // expected-note{{in instantiation of function template specialization}}
+  // expected-note@+1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    // expected-error@+1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+    for(;;){}
+  }
+
+  // expected-note@+1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    for(;;){}
+    // expected-error@+1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+  }
+
+  // The below two are fine, as they use the 'force' tag.
+#pragma acc loop collapse(force:2)
+  for(;;) {
+    call();
+    for(;;){}
+  }
+
+#pragma acc loop collapse(force:2)
+  for(;;) {
+    for(;;){}
+    call();
+  }
+
+#pragma acc loop collapse(2)
+  for(;;) {
+    for(;;){
+    call();
+    }
+  }
+#pragma acc loop collapse(2)
+  for(;;) {
+    {
+      {
+        for(;;){
+        call();
+        }
+      }
+    }
+  }
+
+#pragma acc loop collapse(force:2)
+  for(;;) {
+    for(;;){
+    call();
+    }
+  }
+}
+

@erichkeane erichkeane merged commit 4f82f27 into llvm:main Oct 3, 2024
11 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 8, 2024

LLVM Buildbot has detected a new failure on builder clang-ppc64-aix running on aix-ppc64 while building clang at step 6 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lit :: googletest-timeout.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 9
not env -u FILECHECK_OPTS "/opt/freeware/bin/python3.9" /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout    --param gtest_filter=InfiniteLoopSubTest --timeout=1 > /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out
# executed command: not env -u FILECHECK_OPTS /opt/freeware/bin/python3.9 /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --timeout=1
# .---command stderr------------
# | lit.py: /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# `-----------------------------
# RUN: at line 11
FileCheck --check-prefix=CHECK-INF < /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/googletest-timeout.py
# RUN: at line 16
not env -u FILECHECK_OPTS "/opt/freeware/bin/python3.9" /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout   --param gtest_filter=InfiniteLoopSubTest  --param set_timeout=1   > /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/Output/googletest-timeout.py.tmp.cfgset.out
# executed command: not env -u FILECHECK_OPTS /opt/freeware/bin/python3.9 /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --param set_timeout=1
# .---command stderr------------
# | Traceback (most recent call last):
# |   File "/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 304, in post_process_shard_results
# |     testsuites = json.load(f)["testsuites"]
# |   File "/opt/freeware/lib64/python3.9/json/__init__.py", line 293, in load
# |     return loads(fp.read(),
# |   File "/opt/freeware/lib64/python3.9/json/__init__.py", line 346, in loads
# |     return _default_decoder.decode(s)
# |   File "/opt/freeware/lib64/python3.9/json/decoder.py", line 337, in decode
# |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
# |   File "/opt/freeware/lib64/python3.9/json/decoder.py", line 355, in raw_decode
# |     raise JSONDecodeError("Expecting value", s, err.value) from None
# | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
# | 
# | During handling of the above exception, another exception occurred:
# | 
# | Traceback (most recent call last):
# |   File "/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py", line 6, in <module>
# |     main()
# |   File "/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit/main.py", line 130, in main
# |     selected_tests, discovered_tests = GoogleTest.post_process_shard_results(
# |   File "/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 306, in post_process_shard_results
# |     raise RuntimeError(
# | RuntimeError: Failed to parse json file: /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py-googletest-timeout-53608940-1-2.json
# | 
# `-----------------------------
# RUN: at line 19
FileCheck --check-prefix=CHECK-INF < /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/Output/googletest-timeout.py.tmp.cfgset.out /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/googletest-timeout.py
# .---command stderr------------
# | /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/googletest-timeout.py:34:14: error: CHECK-INF: expected string not found in input
# | # CHECK-INF: Timed Out: 1
# |              ^
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants