[flang-commits] [flang] [flang][OpenMP] Don't allow DO CONCURRENT inside of a loop nest (PR #144506)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Tue Jun 17 10:40:07 PDT 2025
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/144506
>From 3649ab11c52499a80146542cfd5fbed9260b2627 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Tue, 17 Jun 2025 11:16:51 +0000
Subject: [PATCH 1/4] [flang][OpenMP] Don't allow DO CONCURRENT inside of a
loop nest
I don't think DO CONCURRENT fits the definition of a Canonical Loop Nest
(OpenMP 6.0 section 6.4.1).
Fixes #144178
---
flang/lib/Semantics/resolve-directives.cpp | 6 ++++++
.../Lower/OpenMP/Todo/omp-doconcurrent.f90 | 10 ----------
.../OpenMP/do-concurrent-collapse.f90 | 19 +++++++++++++++++++
3 files changed, 25 insertions(+), 10 deletions(-)
delete mode 100644 flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
create mode 100644 flang/test/Semantics/OpenMP/do-concurrent-collapse.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index b5f8667fe36f2..b6ff7e77ba3fd 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1952,6 +1952,12 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
if (outer.has_value()) {
for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
+ if (loop->IsDoConcurrent()) {
+ auto &stmt =
+ std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
+ context_.Say(stmt.source,
+ "DO CONCURRENT loops cannot form part of a loop nest."_err_en_US);
+ }
// go through all the nested do-loops and resolve index variables
const parser::Name *iv{GetLoopIndex(*loop)};
if (iv) {
diff --git a/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90 b/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
deleted file mode 100644
index a6d70fa445928..0000000000000
--- a/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
+++ /dev/null
@@ -1,10 +0,0 @@
-! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-
-! CHECK: not yet implemented: Do Concurrent in Worksharing loop construct
-subroutine sb()
- !$omp do
- do concurrent(i=1:10)
- print *, i
- end do
-end subroutine
diff --git a/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90 b/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90
new file mode 100644
index 0000000000000..71e1928e1f245
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90
@@ -0,0 +1,19 @@
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp
+
+integer :: i, j
+!$omp parallel do collapse(2)
+do i = 1, 1
+ ! ERROR: DO CONCURRENT loops cannot form part of a loop nest.
+ do concurrent (j = 1:2)
+ print *, j
+ end do
+end do
+
+!$omp parallel do
+do i = 1, 1
+ ! This should not lead to an error because it is not part of a loop nest:
+ do concurrent (j = 1:2)
+ print *, j
+ end do
+end do
+end
>From ab3fba8533682774bb752d0656e3e8c716ec5eba Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Tue, 17 Jun 2025 15:27:08 +0000
Subject: [PATCH 2/4] Allow do concurrent in loop constructs, but not loop
nests
---
flang/lib/Semantics/resolve-directives.cpp | 3 ++-
flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90 | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
create mode 100644 flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index b6ff7e77ba3fd..05568cc0ba0ae 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1952,7 +1952,8 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
if (outer.has_value()) {
for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
- if (loop->IsDoConcurrent()) {
+ // DO CONCURRENT is allowed for loop constructs but not loop nests
+ if (loop->IsDoConcurrent() && GetContext().associatedLoopLevel != 1) {
auto &stmt =
std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
context_.Say(stmt.source,
diff --git a/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90 b/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
new file mode 100644
index 0000000000000..a6d70fa445928
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
@@ -0,0 +1,10 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Do Concurrent in Worksharing loop construct
+subroutine sb()
+ !$omp do
+ do concurrent(i=1:10)
+ print *, i
+ end do
+end subroutine
>From eed0acb23cd3b3759fc71ba8f3a90f963cad5647 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Tue, 17 Jun 2025 16:32:46 +0000
Subject: [PATCH 3/4] Revert "Allow do concurrent in loop constructs, but not
loop nests"
This reverts commit ab3fba8533682774bb752d0656e3e8c716ec5eba.
---
flang/lib/Semantics/resolve-directives.cpp | 3 +--
flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90 | 10 ----------
2 files changed, 1 insertion(+), 12 deletions(-)
delete mode 100644 flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 05568cc0ba0ae..b6ff7e77ba3fd 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1952,8 +1952,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
if (outer.has_value()) {
for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
- // DO CONCURRENT is allowed for loop constructs but not loop nests
- if (loop->IsDoConcurrent() && GetContext().associatedLoopLevel != 1) {
+ if (loop->IsDoConcurrent()) {
auto &stmt =
std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
context_.Say(stmt.source,
diff --git a/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90 b/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
deleted file mode 100644
index a6d70fa445928..0000000000000
--- a/flang/test/Lower/OpenMP/Todo/omp-doconcurrent.f90
+++ /dev/null
@@ -1,10 +0,0 @@
-! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-
-! CHECK: not yet implemented: Do Concurrent in Worksharing loop construct
-subroutine sb()
- !$omp do
- do concurrent(i=1:10)
- print *, i
- end do
-end subroutine
>From 5dbfe279c3813e8aea29203585fc058b05d9935a Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Tue, 17 Jun 2025 17:35:49 +0000
Subject: [PATCH 4/4] Allow do concurrent with LOOP construct
There's some obscure language in OpenMP 6.0:
> If the collapsed loop is a DO CONCURRENT loop, neither the
> data-sharing attribute clauses nor the collapse clause may be specified.
>From the surrounding context, I think "collapsed loop" just means the
loop that the LOOP construct applies to. So I will interpret this to
mean that DO CONCURRENT can only be used with the LOOP construct if it
does not contain the COLLAPSE clause.
This also fixes a bug where the associated clause was never cleared
after it was set.
---
flang/lib/Semantics/resolve-directives.cpp | 40 ++++++++++++++-----
.../OpenMP/do-concurrent-collapse.f90 | 20 ++++++++++
2 files changed, 50 insertions(+), 10 deletions(-)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index b6ff7e77ba3fd..d86cf5aca0d0b 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -23,6 +23,7 @@
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "llvm/Frontend/OpenMP/OMP.h.inc"
#include "llvm/Support/Debug.h"
#include <list>
#include <map>
@@ -737,9 +738,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
}
const parser::OmpClause *associatedClause{nullptr};
- void SetAssociatedClause(const parser::OmpClause &c) {
- associatedClause = &c;
- }
+ void SetAssociatedClause(const parser::OmpClause *c) { associatedClause = c; }
const parser::OmpClause *GetAssociatedClause() { return associatedClause; }
private:
@@ -1916,12 +1915,17 @@ std::int64_t OmpAttributeVisitor::GetAssociatedLoopLevelFromClauses(
}
if (orderedLevel && (!collapseLevel || orderedLevel >= collapseLevel)) {
- SetAssociatedClause(*ordClause);
+ SetAssociatedClause(ordClause);
return orderedLevel;
} else if (!orderedLevel && collapseLevel) {
- SetAssociatedClause(*collClause);
+ SetAssociatedClause(collClause);
return collapseLevel;
- } // orderedLevel < collapseLevel is an error handled in structural checks
+ } else {
+ SetAssociatedClause(nullptr);
+ }
+ // orderedLevel < collapseLevel is an error handled in structural
+ // checks
+
return 1; // default is outermost loop
}
@@ -1949,14 +1953,30 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
ivDSA = Symbol::Flag::OmpLastPrivate;
}
+ bool isLoopConstruct{
+ GetContext().directive == llvm::omp::Directive::OMPD_loop};
+ const parser::OmpClause *clause{GetAssociatedClause()};
+ bool hasCollapseClause{
+ clause ? (clause->Id() == llvm::omp::OMPC_collapse) : false};
+
const auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
if (outer.has_value()) {
for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
if (loop->IsDoConcurrent()) {
- auto &stmt =
- std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
- context_.Say(stmt.source,
- "DO CONCURRENT loops cannot form part of a loop nest."_err_en_US);
+ // DO CONCURRENT is explicitly allowed for the LOOP construct so long as
+ // there isn't a COLLAPSE clause
+ if (isLoopConstruct) {
+ if (hasCollapseClause) {
+ // hasCollapseClause implies clause != nullptr
+ context_.Say(clause->source,
+ "DO CONCURRENT loops cannot be used with the COLLAPSE clause."_err_en_US);
+ }
+ } else {
+ auto &stmt =
+ std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
+ context_.Say(stmt.source,
+ "DO CONCURRENT loops cannot form part of a loop nest."_err_en_US);
+ }
}
// go through all the nested do-loops and resolve index variables
const parser::Name *iv{GetLoopIndex(*loop)};
diff --git a/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90 b/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90
index 71e1928e1f245..bb1929249183b 100644
--- a/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90
+++ b/flang/test/Semantics/OpenMP/do-concurrent-collapse.f90
@@ -16,4 +16,24 @@
print *, j
end do
end do
+
+!$omp parallel do
+! ERROR: DO CONCURRENT loops cannot form part of a loop nest.
+do concurrent (j = 1:2)
+ print *, j
+end do
+
+!$omp loop
+! Do concurrent is explicitly allowed inside of omp loop
+do concurrent (j = 1:2)
+ print *, j
+end do
+
+! ERROR: DO CONCURRENT loops cannot be used with the COLLAPSE clause.
+!$omp loop collapse(2)
+do i = 1, 1
+ do concurrent (j = 1:2)
+ print *, j
+ end do
+end do
end
More information about the flang-commits
mailing list