[clang] [OpenACC] 'collapse' clause 'force' tag (PR #110906)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

<details>
<summary>Changes</summary>

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
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 at +1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    // expected-error at +1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+    for(;;){}
+  }
+
+  // expected-note at +1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(Two)
+  for(;;) {
+    // expected-error at +1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+    for(;;){}
+  }
+
+  // expected-note at +1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    for(;;){}
+    // expected-error at +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 at +1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(Two)
+  for(;;) {
+    for(;;){}
+    // expected-error at +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 at +1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    // expected-error at +1{{inner loops must be tightly nested inside a 'collapse' clause on a 'loop' construct}}
+    call();
+    for(;;){}
+  }
+
+  // expected-note at +1{{active 'collapse' clause defined here}}
+#pragma acc loop collapse(2)
+  for(;;) {
+    for(;;){}
+    // expected-error at +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();
+    }
+  }
+}
+

``````````

</details>


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