[flang-commits] [flang] [Flang][OpenMP] Add Semantics support for Nested OpenMPLoopConstructs (PR #145917)
Jack Styles via flang-commits
flang-commits at lists.llvm.org
Fri Jun 27 02:28:37 PDT 2025
https://github.com/Stylie777 updated https://github.com/llvm/llvm-project/pull/145917
>From ded0a196e122427130f2c0b7edd57cb3c1dafdb5 Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Wed, 25 Jun 2025 12:29:27 +0100
Subject: [PATCH 1/5] [Flang][OpenMP] Add Semantics support for Nested
OpenMPLoopConstructs
In OpenMP Version 5.1, the tile and unroll directives were added. When
using these directives, it is possible to nest them within other OpenMP
Loop Constructs. This patch enables the semantics to allow for this
behaviour on these specific directives. Any nested loops will be stored
within the initial Loop Construct until reaching the DoConstruct itself.
Relevant tests have been added, and previous behaviour has been retained
with no changes.
See also, #110008
---
flang/include/flang/Parser/parse-tree.h | 2 +-
flang/lib/Lower/OpenMP/OpenMP.cpp | 6 ++
flang/lib/Parser/unparse.cpp | 2 +-
flang/lib/Semantics/canonicalize-omp.cpp | 60 +++++++++---
flang/lib/Semantics/check-omp-structure.cpp | 44 ++++++---
flang/lib/Semantics/resolve-directives.cpp | 97 +++++++++++--------
...nested-loop-transformation-construct01.f90 | 20 ++++
...nested-loop-transformation-construct02.f90 | 20 ++++
.../loop-transformation-construct01.f90 | 74 ++++++++++++++
.../loop-transformation-construct02.f90 | 85 ++++++++++++++++
.../loop-transformation-construct01.f90 | 28 ++++++
11 files changed, 373 insertions(+), 65 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/nested-loop-transformation-construct01.f90
create mode 100644 flang/test/Lower/OpenMP/nested-loop-transformation-construct02.f90
create mode 100644 flang/test/Parser/OpenMP/loop-transformation-construct01.f90
create mode 100644 flang/test/Parser/OpenMP/loop-transformation-construct02.f90
create mode 100644 flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 61f97b855b0e5..7aa9250234978 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -5025,7 +5025,7 @@ struct OpenMPLoopConstruct {
TUPLE_CLASS_BOILERPLATE(OpenMPLoopConstruct);
OpenMPLoopConstruct(OmpBeginLoopDirective &&a)
: t({std::move(a), std::nullopt, std::nullopt}) {}
- std::tuple<OmpBeginLoopDirective, std::optional<DoConstruct>,
+ std::tuple<OmpBeginLoopDirective, std::optional<std::variant<DoConstruct, common::Indirection<OpenMPLoopConstruct>>>,
std::optional<OmpEndLoopDirective>>
t;
};
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index e21d0effe2ef6..3e6f0ff2e1cba 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -4231,6 +4231,12 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
mlir::Location currentLocation =
converter.genLocation(beginLoopDirective.source);
+ auto &optLoopCons = std::get<1>(loopConstruct.t);
+ if(optLoopCons.has_value())
+ if(auto *ompNestedLoopCons{std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(&*optLoopCons)}) {
+ genOMP(converter, symTable, semaCtx, eval, ompNestedLoopCons->value());
+ }
+
llvm::omp::Directive directive =
std::get<parser::OmpLoopDirective>(beginLoopDirective.t).v;
const parser::CharBlock &source =
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index ed0f227fd5b98..276be878dd44e 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2926,7 +2926,7 @@ class UnparseVisitor {
Walk(std::get<OmpBeginLoopDirective>(x.t));
Put("\n");
EndOpenMP();
- Walk(std::get<std::optional<DoConstruct>>(x.t));
+ Walk(std::get<std::optional<std::variant<DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t));
Walk(std::get<std::optional<OmpEndLoopDirective>>(x.t));
}
void Unparse(const BasedPointer &x) {
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index 5164f1dc6faab..d6827435173d3 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -8,6 +8,7 @@
#include "canonicalize-omp.h"
#include "flang/Parser/parse-tree-visitor.h"
+#include "flang/Parser/parse-tree.h"
// After Loop Canonicalization, rewrite OpenMP parse tree to make OpenMP
// Constructs more structured which provide explicit scopes for later
@@ -106,6 +107,12 @@ class CanonicalizationOfOmp {
return nullptr;
}
+ void missingDoConstruct(parser::OmpLoopDirective &dir) {
+ messages_.Say(dir.source,
+ "A DO loop must follow the %s directive"_err_en_US,
+ parser::ToUpperCaseLetters(dir.source.ToString()));
+ }
+
void RewriteOpenMPLoopConstruct(parser::OpenMPLoopConstruct &x,
parser::Block &block, parser::Block::iterator it) {
// Check the sequence of DoConstruct and OmpEndLoopDirective
@@ -135,31 +142,62 @@ class CanonicalizationOfOmp {
if (auto *doCons{GetConstructIf<parser::DoConstruct>(*nextIt)}) {
if (doCons->GetLoopControl()) {
// move DoConstruct
- std::get<std::optional<parser::DoConstruct>>(x.t) =
+ std::get<std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t) =
std::move(*doCons);
nextIt = block.erase(nextIt);
// try to match OmpEndLoopDirective
- if (nextIt != block.end()) {
- if (auto *endDir{
- GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
- std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
- std::move(*endDir);
- block.erase(nextIt);
- }
+ if (auto *endDir{
+ GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
+ std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
+ std::move(*endDir);
+ nextIt = block.erase(nextIt);
}
} else {
messages_.Say(dir.source,
"DO loop after the %s directive must have loop control"_err_en_US,
parser::ToUpperCaseLetters(dir.source.ToString()));
}
+ } else if (auto *ompLoopCons{
+ GetOmpIf<parser::OpenMPLoopConstruct>(*nextIt)}) {
+ // We should allow UNROLL and TILE constructs to be inserted between an OpenMP Loop Construct and the DO loop itself
+ auto &beginDirective =
+ std::get<parser::OmpBeginLoopDirective>(ompLoopCons->t);
+ auto &beginLoopDirective =
+ std::get<parser::OmpLoopDirective>(beginDirective.t);
+ // iterate through the remaining block items to find the end directive for the unroll/tile directive.
+ parser::Block::iterator endIt;
+ endIt = nextIt;
+ while(endIt != block.end()) {
+ if (auto *endDir{
+ GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
+ auto &endLoopDirective = std::get<parser::OmpLoopDirective>(endDir->t);
+ if(endLoopDirective.v == dir.v) {
+ std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
+ std::move(*endDir);
+ endIt = block.erase(endIt);
+ continue;
+ }
+ }
+ ++endIt;
+ }
+ if ((beginLoopDirective.v == llvm::omp::Directive::OMPD_unroll ||
+ beginLoopDirective.v == llvm::omp::Directive::OMPD_tile)) {
+ RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
+ auto &ompLoop = std::get<std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t);
+ ompLoop = std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>{
+ std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>{
+ common::Indirection{std::move(*ompLoopCons)}}};
+ nextIt = block.erase(nextIt);
+ }
} else {
- messages_.Say(dir.source,
- "A DO loop must follow the %s directive"_err_en_US,
- parser::ToUpperCaseLetters(dir.source.ToString()));
+ missingDoConstruct(dir);
}
// If we get here, we either found a loop, or issued an error message.
return;
}
+ if (nextIt == block.end()) {
+ missingDoConstruct(dir);
+ }
}
void RewriteOmpAllocations(parser::ExecutionPart &body) {
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 3abb5a304b00c..b54cd30ec5138 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -762,10 +762,13 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
}
SetLoopInfo(x);
- if (const auto &doConstruct{
- std::get<std::optional<parser::DoConstruct>>(x.t)}) {
- const auto &doBlock{std::get<parser::Block>(doConstruct->t)};
- CheckNoBranching(doBlock, beginDir.v, beginDir.source);
+ auto &optLoopCons = std::get<1>(x.t);
+ if(optLoopCons.has_value()) {
+ if (const auto &doConstruct{
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ const auto &doBlock{std::get<parser::Block>(doConstruct->t)};
+ CheckNoBranching(doBlock, beginDir.v, beginDir.source);
+ }
}
CheckLoopItrVariableIsInt(x);
CheckAssociatedLoopConstraints(x);
@@ -779,6 +782,12 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
(beginDir.v == llvm::omp::Directive::OMPD_distribute_simd)) {
CheckDistLinear(x);
}
+ if (beginDir.v == llvm::omp::Directive::OMPD_tile) {
+ const auto &clauses{std::get<parser::OmpClauseList>(beginLoopDir.t)};
+ for (auto &clause : clauses.v) {
+
+ }
+ }
}
const parser::Name OmpStructureChecker::GetLoopIndex(
const parser::DoConstruct *x) {
@@ -786,12 +795,15 @@ const parser::Name OmpStructureChecker::GetLoopIndex(
return std::get<Bounds>(x->GetLoopControl()->u).name.thing;
}
void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) {
- if (const auto &loopConstruct{
- std::get<std::optional<parser::DoConstruct>>(x.t)}) {
- const parser::DoConstruct *loop{&*loopConstruct};
- if (loop && loop->IsDoNormal()) {
- const parser::Name &itrVal{GetLoopIndex(loop)};
- SetLoopIv(itrVal.symbol);
+ auto &optLoopCons = std::get<1>(x.t);
+ if (optLoopCons.has_value()) {
+ if (const auto &loopConstruct{
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ const parser::DoConstruct *loop{&*loopConstruct};
+ if (loop && loop->IsDoNormal()) {
+ const parser::Name &itrVal{GetLoopIndex(loop)};
+ SetLoopIv(itrVal.symbol);
+ }
}
}
}
@@ -857,8 +869,10 @@ void OmpStructureChecker::CheckIteratorModifier(const parser::OmpIterator &x) {
void OmpStructureChecker::CheckLoopItrVariableIsInt(
const parser::OpenMPLoopConstruct &x) {
- if (const auto &loopConstruct{
- std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+ auto &optLoopCons = std::get<1>(x.t);
+ if (optLoopCons.has_value()) {
+ if (const auto &loopConstruct{
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
if (loop->IsDoNormal()) {
@@ -878,6 +892,7 @@ void OmpStructureChecker::CheckLoopItrVariableIsInt(
const auto it{block.begin()};
loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
: nullptr;
+ }
}
}
}
@@ -1077,8 +1092,10 @@ void OmpStructureChecker::CheckDistLinear(
// Match the loop index variables with the collected symbols from linear
// clauses.
+ auto &optLoopCons = std::get<1>(x.t);
+ if (optLoopCons.has_value()) {
if (const auto &loopConstruct{
- std::get<std::optional<parser::DoConstruct>>(x.t)}) {
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
if (loop->IsDoNormal()) {
const parser::Name &itrVal{GetLoopIndex(loop)};
@@ -1096,6 +1113,7 @@ void OmpStructureChecker::CheckDistLinear(
const auto it{block.begin()};
loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
: nullptr;
+ }
}
}
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 885c02e6ec74b..36bea4fbe7ae5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1796,10 +1796,13 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
SetContextAssociatedLoopLevel(GetAssociatedLoopLevelFromClauses(clauseList));
if (beginDir.v == llvm::omp::Directive::OMPD_do) {
- if (const auto &doConstruct{
- std::get<std::optional<parser::DoConstruct>>(x.t)}) {
- if (doConstruct.value().IsDoWhile()) {
- return true;
+ auto &optLoopCons = std::get<1>(x.t);
+ if (optLoopCons.has_value()) {
+ if (const auto &doConstruct{
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ if (doConstruct->IsDoWhile()) {
+ return true;
+ }
}
}
}
@@ -1962,48 +1965,64 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
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()) {
- // 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);
+ auto &optLoopCons = std::get<1>(x.t);
+ if (optLoopCons.has_value()) {
+ if (const auto &outer{std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
+ if (loop->IsDoConcurrent()) {
+ // 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);
}
- } 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)};
- if (iv) {
- if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) {
- SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA});
- iv->symbol = symbol; // adjust the symbol within region
- AddToContextObjectWithDSA(*symbol, ivDSA);
}
+ // go through all the nested do-loops and resolve index variables
+ const parser::Name *iv{GetLoopIndex(*loop)};
+ if (iv) {
+ if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) {
+ SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA});
+ iv->symbol = symbol; // adjust the symbol within region
+ AddToContextObjectWithDSA(*symbol, ivDSA);
+ }
- const auto &block{std::get<parser::Block>(loop->t)};
- const auto it{block.begin()};
- loop = it != block.end() ? GetDoConstructIf(*it) : nullptr;
+ const auto &block{std::get<parser::Block>(loop->t)};
+ const auto it{block.begin()};
+ loop = it != block.end() ? GetDoConstructIf(*it) : nullptr;
+ }
+ }
+ CheckAssocLoopLevel(level, GetAssociatedClause());
+ } else if (const auto &loop{std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(&*optLoopCons)}) {
+ auto &beginDirective =
+ std::get<parser::OmpBeginLoopDirective>(loop->value().t);
+ auto &beginLoopDirective =
+ std::get<parser::OmpLoopDirective>(beginDirective.t);
+ if ((beginLoopDirective.v != llvm::omp::Directive::OMPD_unroll &&
+ beginLoopDirective.v != llvm::omp::Directive::OMPD_tile)) {
+ context_.Say(GetContext().directiveSource,
+ "Only UNROLL or TILE constructs are allowed between an OpenMP Loop Construct and a DO construct"_err_en_US,
+ parser::ToUpperCaseLetters(llvm::omp::getOpenMPDirectiveName(GetContext().directive, version).str()));
+ } else {
+ PrivatizeAssociatedLoopIndexAndCheckLoopLevel(loop->value());
}
+ } else {
+ context_.Say(GetContext().directiveSource,
+ "A DO loop must follow the %s directive"_err_en_US,
+ parser::ToUpperCaseLetters(
+ llvm::omp::getOpenMPDirectiveName(GetContext().directive, version)
+ .str()));
}
- CheckAssocLoopLevel(level, GetAssociatedClause());
- } else {
- context_.Say(GetContext().directiveSource,
- "A DO loop must follow the %s directive"_err_en_US,
- parser::ToUpperCaseLetters(
- llvm::omp::getOpenMPDirectiveName(GetContext().directive, version)
- .str()));
}
}
+
void OmpAttributeVisitor::CheckAssocLoopLevel(
std::int64_t level, const parser::OmpClause *clause) {
if (clause && level != 0) {
diff --git a/flang/test/Lower/OpenMP/nested-loop-transformation-construct01.f90 b/flang/test/Lower/OpenMP/nested-loop-transformation-construct01.f90
new file mode 100644
index 0000000000000..b24e2d62ac2aa
--- /dev/null
+++ b/flang/test/Lower/OpenMP/nested-loop-transformation-construct01.f90
@@ -0,0 +1,20 @@
+! Test to ensure TODO message is emitted for tile OpenMP 5.1 Directives when they are nested.
+
+!RUN: not %flang -fopenmp -fopenmp-version=51 %s 2<&1 | FileCheck %s
+
+subroutine loop_transformation_construct
+ implicit none
+ integer :: I = 10
+ integer :: x
+ integer :: y(I)
+
+ !$omp do
+ !$omp tile
+ do i = 1, I
+ y(i) = y(i) * 5
+ end do
+ !$omp end tile
+ !$omp end do
+end subroutine
+
+!CHECK: not yet implemented: Unhandled loop directive (tile)
diff --git a/flang/test/Lower/OpenMP/nested-loop-transformation-construct02.f90 b/flang/test/Lower/OpenMP/nested-loop-transformation-construct02.f90
new file mode 100644
index 0000000000000..bbb16a6a699e9
--- /dev/null
+++ b/flang/test/Lower/OpenMP/nested-loop-transformation-construct02.f90
@@ -0,0 +1,20 @@
+! Test to ensure TODO message is emitted for unroll OpenMP 5.1 Directives when they are nested.
+
+!RUN: not %flang -fopenmp -fopenmp-version=51 %s 2<&1 | FileCheck %s
+
+program loop_transformation_construct
+ implicit none
+ integer, parameter :: I = 10
+ integer :: x
+ integer :: y(I)
+
+ !$omp do
+ !$omp unroll
+ do x = 1, I
+ y(x) = y(x) * 5
+ end do
+ !$omp end unroll
+ !$omp end do
+end program loop_transformation_construct
+
+!CHECK: not yet implemented: Unhandled loop directive (unroll)
diff --git a/flang/test/Parser/OpenMP/loop-transformation-construct01.f90 b/flang/test/Parser/OpenMP/loop-transformation-construct01.f90
new file mode 100644
index 0000000000000..baffc2f6e2f1e
--- /dev/null
+++ b/flang/test/Parser/OpenMP/loop-transformation-construct01.f90
@@ -0,0 +1,74 @@
+! Test the Parse Tree to ensure the OpenMP Loop Transformation Constructs nest correctly with 1 nested loop.
+
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-PARSE
+! RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-UNPARSE
+
+subroutine loop_transformation_construct
+ implicit none
+ integer :: I = 10
+ integer :: x
+ integer :: y(I)
+
+ !$omp do
+ !$omp unroll
+ do i = 1, I
+ y(i) = y(i) * 5
+ end do
+ !$omp end unroll
+ !$omp end do
+end subroutine
+
+!CHECK-PARSE: | ExecutionPart -> Block
+!CHECK-PARSE-NEXT: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
+!CHECK-PARSE-NEXT: | | | OmpBeginLoopDirective
+!CHECK-PARSE-NEXT: | | | | OmpLoopDirective -> llvm::omp::Directive = do
+!CHECK-PARSE-NEXT: | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | OpenMPLoopConstruct
+!CHECK-PARSE-NEXT: | | | | OmpBeginLoopDirective
+!CHECK-PARSE-NEXT: | | | | | OmpLoopDirective -> llvm::omp::Directive = unroll
+!CHECK-PARSE-NEXT: | | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | | DoConstruct
+!CHECK-PARSE-NEXT: | | | | | NonLabelDoStmt
+!CHECK-PARSE-NEXT: | | | | | | LoopControl -> LoopBounds
+!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Expr = '1_4'
+!CHECK-PARSE-NEXT: | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Expr = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | Designator -> DataRef -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | Block
+!CHECK-PARSE-NEXT: | | | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'y(int(i,kind=8))=5_4*y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | Variable = 'y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | Designator -> DataRef -> ArrayElement
+!CHECK-PARSE-NEXT: | | | | | | | | | DataRef -> Name = 'y'
+!CHECK-PARSE-NEXT: | | | | | | | | | SectionSubscript -> Integer -> Expr = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | | Designator -> DataRef -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | Expr = '5_4*y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | Multiply
+!CHECK-PARSE-NEXT: | | | | | | | | | Expr = 'y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | | | Designator -> DataRef -> ArrayElement
+!CHECK-PARSE-NEXT: | | | | | | | | | | | DataRef -> Name = 'y'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | SectionSubscript -> Integer -> Expr = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | | Designator -> DataRef -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | Expr = '5_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | | LiteralConstant -> IntLiteralConstant = '5'
+!CHECK-PARSE-NEXT: | | | | | EndDoStmt ->
+!CHECK-PARSE-NEXT: | | | | OmpEndLoopDirective
+!CHECK-PARSE-NEXT: | | | | | OmpLoopDirective -> llvm::omp::Directive = unroll
+!CHECK-PARSE-NEXT: | | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | OmpEndLoopDirective
+!CHECK-PARSE-NEXT: | | | | OmpLoopDirective -> llvm::omp::Directive = do
+!CHECK-PARSE-NEXT: | | | | OmpClauseList ->
+
+!CHECK-UNPARSE: SUBROUTINE loop_transformation_construct
+!CHECK-UNPARSE-NEXT: IMPLICIT NONE
+!CHECK-UNPARSE-NEXT: INTEGER :: i = 10_4
+!CHECK-UNPARSE-NEXT: INTEGER x
+!CHECK-UNPARSE-NEXT: INTEGER y(i)
+!CHECK-UNPARSE-NEXT: !$OMP DO
+!CHECK-UNPARSE-NEXT: !$OMP UNROLL
+!CHECK-UNPARSE-NEXT: DO i=1_4,i
+!CHECK-UNPARSE-NEXT: y(int(i,kind=8))=5_4*y(int(i,kind=8))
+!CHECK-UNPARSE-NEXT: END DO
+!CHECK-UNPARSE-NEXT: !$OMP END UNROLL
+!CHECK-UNPARSE-NEXT: !$OMP END DO
+!CHECK-UNPARSE-NEXT: END SUBROUTINE
diff --git a/flang/test/Parser/OpenMP/loop-transformation-construct02.f90 b/flang/test/Parser/OpenMP/loop-transformation-construct02.f90
new file mode 100644
index 0000000000000..b50e7183841cc
--- /dev/null
+++ b/flang/test/Parser/OpenMP/loop-transformation-construct02.f90
@@ -0,0 +1,85 @@
+! Test the Parse Tree to ensure the OpenMP Loop Transformation Constructs nest correctly with multiple nested loops.
+
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-PARSE
+! RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-UNPARSE
+
+subroutine loop_transformation_construct
+ implicit none
+ integer :: I = 10
+ integer :: x
+ integer :: y(I)
+
+ !$omp do
+ !$omp unroll
+ !$omp tile
+ do i = 1, I
+ y(i) = y(i) * 5
+ end do
+ !$omp end tile
+ !$omp end unroll
+ !$omp end do
+end subroutine
+
+!CHECK-PARSE: | ExecutionPart -> Block
+!CHECK-PARSE-NEXT: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
+!CHECK-PARSE-NEXT: | | | OmpBeginLoopDirective
+!CHECK-PARSE-NEXT: | | | | OmpLoopDirective -> llvm::omp::Directive = do
+!CHECK-PARSE-NEXT: | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | OpenMPLoopConstruct
+!CHECK-PARSE-NEXT: | | | | OmpBeginLoopDirective
+!CHECK-PARSE-NEXT: | | | | | OmpLoopDirective -> llvm::omp::Directive = unroll
+!CHECK-PARSE-NEXT: | | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | | OpenMPLoopConstruct
+!CHECK-PARSE-NEXT: | | | | | OmpBeginLoopDirective
+!CHECK-PARSE-NEXT: | | | | | | OmpLoopDirective -> llvm::omp::Directive = tile
+!CHECK-PARSE-NEXT: | | | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | | | DoConstruct
+!CHECK-PARSE-NEXT: | | | | | | NonLabelDoStmt
+!CHECK-PARSE-NEXT: | | | | | | | LoopControl -> LoopBounds
+!CHECK-PARSE-NEXT: | | | | | | | | Scalar -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | Scalar -> Expr = '1_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!CHECK-PARSE-NEXT: | | | | | | | | Scalar -> Expr = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | Designator -> DataRef -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | Block
+!CHECK-PARSE-NEXT: | | | | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'y(int(i,kind=8))=5_4*y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | Variable = 'y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | | Designator -> DataRef -> ArrayElement
+!CHECK-PARSE-NEXT: | | | | | | | | | | DataRef -> Name = 'y'
+!CHECK-PARSE-NEXT: | | | | | | | | | | SectionSubscript -> Integer -> Expr = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | Designator -> DataRef -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | Expr = '5_4*y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | | Multiply
+!CHECK-PARSE-NEXT: | | | | | | | | | | Expr = 'y(int(i,kind=8))'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | Designator -> DataRef -> ArrayElement
+!CHECK-PARSE-NEXT: | | | | | | | | | | | | DataRef -> Name = 'y'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | | SectionSubscript -> Integer -> Expr = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | | | Designator -> DataRef -> Name = 'i'
+!CHECK-PARSE-NEXT: | | | | | | | | | | Expr = '5_4'
+!CHECK-PARSE-NEXT: | | | | | | | | | | | LiteralConstant -> IntLiteralConstant = '5'
+!CHECK-PARSE-NEXT: | | | | | | EndDoStmt ->
+!CHECK-PARSE-NEXT: | | | | | OmpEndLoopDirective
+!CHECK-PARSE-NEXT: | | | | | | OmpLoopDirective -> llvm::omp::Directive = tile
+!CHECK-PARSE-NEXT: | | | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | | OmpEndLoopDirective
+!CHECK-PARSE-NEXT: | | | | | OmpLoopDirective -> llvm::omp::Directive = unroll
+!CHECK-PARSE-NEXT: | | | | | OmpClauseList ->
+!CHECK-PARSE-NEXT: | | | OmpEndLoopDirective
+!CHECK-PARSE-NEXT: | | | | OmpLoopDirective -> llvm::omp::Directive = do
+!CHECK-PARSE-NEXT: | | | | OmpClauseList ->
+
+!CHECK-UNPARSE: SUBROUTINE loop_transformation_construct
+!CHECK-UNPARSE-NEXT: IMPLICIT NONE
+!CHECK-UNPARSE-NEXT: INTEGER :: i = 10_4
+!CHECK-UNPARSE-NEXT: INTEGER x
+!CHECK-UNPARSE-NEXT: INTEGER y(i)
+!CHECK-UNPARSE-NEXT: !$OMP DO
+!CHECK-UNPARSE-NEXT: !$OMP UNROLL
+!CHECK-UNPARSE-NEXT: !$OMP TILE
+!CHECK-UNPARSE-NEXT: DO i=1_4,i
+!CHECK-UNPARSE-NEXT: y(int(i,kind=8))=5_4*y(int(i,kind=8))
+!CHECK-UNPARSE-NEXT: END DO
+!CHECK-UNPARSE-NEXT: !$OMP END TILE
+!CHECK-UNPARSE-NEXT: !$OMP END UNROLL
+!CHECK-UNPARSE-NEXT: !$OMP END DO
+!CHECK-UNPARSE-NEXT: END SUBROUTINE
diff --git a/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90 b/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
new file mode 100644
index 0000000000000..fa87c443841d0
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
@@ -0,0 +1,28 @@
+! Testing the Semantics of nested Loop Transformation Constructs
+
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=51
+
+subroutine loop_transformation_construct1
+ implicit none
+
+ !$omp do
+ !ERROR: A DO loop must follow the UNROLL directive
+ !$omp unroll
+end subroutine
+
+subroutine loop_transformation_construct2
+ implicit none
+ integer :: i = 5
+ integer :: y
+ integer :: v(i)
+
+ !$omp do
+ !$omp tile
+ do x = 1, i
+ v(x) = x(x) * 2
+ end do
+ !$omp end tile
+ !$omp end do
+ !ERROR: The END TILE directive must follow the DO loop associated with the loop construct
+ !$omp end tile
+end subroutine
>From 305a0d845cb5b4a5200634ce8956047e6a838e76 Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Thu, 26 Jun 2025 16:32:05 +0100
Subject: [PATCH 2/5] Add error for nesting of non loop transformation
constructs
---
flang/lib/Semantics/canonicalize-omp.cpp | 36 ++++++++++---------
flang/lib/Semantics/check-omp-structure.cpp | 6 ----
.../loop-transformation-construct01.f90 | 18 ++++++++++
3 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index d6827435173d3..2867ebf5cbabf 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -164,30 +164,34 @@ class CanonicalizationOfOmp {
std::get<parser::OmpBeginLoopDirective>(ompLoopCons->t);
auto &beginLoopDirective =
std::get<parser::OmpLoopDirective>(beginDirective.t);
- // iterate through the remaining block items to find the end directive for the unroll/tile directive.
- parser::Block::iterator endIt;
- endIt = nextIt;
- while(endIt != block.end()) {
- if (auto *endDir{
- GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
- auto &endLoopDirective = std::get<parser::OmpLoopDirective>(endDir->t);
- if(endLoopDirective.v == dir.v) {
- std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
- std::move(*endDir);
- endIt = block.erase(endIt);
- continue;
+ if ((beginLoopDirective.v == llvm::omp::Directive::OMPD_unroll ||
+ beginLoopDirective.v == llvm::omp::Directive::OMPD_tile)) {
+ // iterate through the remaining block items to find the end directive for the unroll/tile directive.
+ parser::Block::iterator endIt;
+ endIt = nextIt;
+ while(endIt != block.end()) {
+ if (auto *endDir{
+ GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
+ auto &endLoopDirective = std::get<parser::OmpLoopDirective>(endDir->t);
+ if(endLoopDirective.v == dir.v) {
+ std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
+ std::move(*endDir);
+ endIt = block.erase(endIt);
+ continue;
+ }
}
+ ++endIt;
}
- ++endIt;
- }
- if ((beginLoopDirective.v == llvm::omp::Directive::OMPD_unroll ||
- beginLoopDirective.v == llvm::omp::Directive::OMPD_tile)) {
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
auto &ompLoop = std::get<std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t);
ompLoop = std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>{
std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>{
common::Indirection{std::move(*ompLoopCons)}}};
nextIt = block.erase(nextIt);
+ } else {
+ messages_.Say(dir.source,
+ "Only OpenMP Loop Transformation Constructs can be nested within OpenMPLoopConstruct's"_err_en_US,
+ parser::ToUpperCaseLetters(dir.source.ToString()));
}
} else {
missingDoConstruct(dir);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index b54cd30ec5138..9f4504bf3dcc6 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -782,12 +782,6 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
(beginDir.v == llvm::omp::Directive::OMPD_distribute_simd)) {
CheckDistLinear(x);
}
- if (beginDir.v == llvm::omp::Directive::OMPD_tile) {
- const auto &clauses{std::get<parser::OmpClauseList>(beginLoopDir.t)};
- for (auto &clause : clauses.v) {
-
- }
- }
}
const parser::Name OmpStructureChecker::GetLoopIndex(
const parser::DoConstruct *x) {
diff --git a/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90 b/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
index fa87c443841d0..92a2792ac05a3 100644
--- a/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
+++ b/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
@@ -26,3 +26,21 @@ subroutine loop_transformation_construct2
!ERROR: The END TILE directive must follow the DO loop associated with the loop construct
!$omp end tile
end subroutine
+
+subroutine loop_transformation_construct2
+ implicit none
+ integer :: i = 5
+ integer :: y
+ integer :: v(i)
+
+ !$omp do
+ !ERROR: Only OpenMP Loop Transformation Constructs can be nested within OpenMPLoopConstruct's
+ !$omp parallel do
+ do x = 1, i
+ v(x) = x(x) * 2
+ end do
+ !! This error occurs because the `parallel do` end directive never gets matched.
+ !ERROR: The END PARALLEL DO directive must follow the DO loop associated with the loop construct
+ !$omp end parallel do
+ !$omp end do
+end subroutine
>From bf0d3c601d925078211c5d8268f188a2bd1239d1 Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Thu, 26 Jun 2025 16:34:36 +0100
Subject: [PATCH 3/5] formatting fixes
---
flang/include/flang/Parser/parse-tree.h | 4 +-
flang/lib/Lower/OpenMP/OpenMP.cpp | 6 +-
flang/lib/Parser/unparse.cpp | 3 +-
flang/lib/Semantics/canonicalize-omp.cpp | 54 +++++++++-------
flang/lib/Semantics/check-omp-structure.cpp | 72 ++++++++++-----------
flang/lib/Semantics/resolve-directives.cpp | 21 +++---
6 files changed, 89 insertions(+), 71 deletions(-)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 7aa9250234978..8d87354247823 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -5025,7 +5025,9 @@ struct OpenMPLoopConstruct {
TUPLE_CLASS_BOILERPLATE(OpenMPLoopConstruct);
OpenMPLoopConstruct(OmpBeginLoopDirective &&a)
: t({std::move(a), std::nullopt, std::nullopt}) {}
- std::tuple<OmpBeginLoopDirective, std::optional<std::variant<DoConstruct, common::Indirection<OpenMPLoopConstruct>>>,
+ std::tuple<OmpBeginLoopDirective,
+ std::optional<
+ std::variant<DoConstruct, common::Indirection<OpenMPLoopConstruct>>>,
std::optional<OmpEndLoopDirective>>
t;
};
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 3e6f0ff2e1cba..c85d015b8279a 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -4232,8 +4232,10 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
converter.genLocation(beginLoopDirective.source);
auto &optLoopCons = std::get<1>(loopConstruct.t);
- if(optLoopCons.has_value())
- if(auto *ompNestedLoopCons{std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(&*optLoopCons)}) {
+ if (optLoopCons.has_value())
+ if (auto *ompNestedLoopCons{
+ std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
+ &*optLoopCons)}) {
genOMP(converter, symTable, semaCtx, eval, ompNestedLoopCons->value());
}
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 276be878dd44e..903287515e559 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2926,7 +2926,8 @@ class UnparseVisitor {
Walk(std::get<OmpBeginLoopDirective>(x.t));
Put("\n");
EndOpenMP();
- Walk(std::get<std::optional<std::variant<DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t));
+ Walk(std::get<std::optional<std::variant<DoConstruct,
+ common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t));
Walk(std::get<std::optional<OmpEndLoopDirective>>(x.t));
}
void Unparse(const BasedPointer &x) {
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index 2867ebf5cbabf..03d71eef4a7a8 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -109,8 +109,8 @@ class CanonicalizationOfOmp {
void missingDoConstruct(parser::OmpLoopDirective &dir) {
messages_.Say(dir.source,
- "A DO loop must follow the %s directive"_err_en_US,
- parser::ToUpperCaseLetters(dir.source.ToString()));
+ "A DO loop must follow the %s directive"_err_en_US,
+ parser::ToUpperCaseLetters(dir.source.ToString()));
}
void RewriteOpenMPLoopConstruct(parser::OpenMPLoopConstruct &x,
@@ -142,7 +142,8 @@ class CanonicalizationOfOmp {
if (auto *doCons{GetConstructIf<parser::DoConstruct>(*nextIt)}) {
if (doCons->GetLoopControl()) {
// move DoConstruct
- std::get<std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t) =
+ std::get<std::optional<std::variant<parser::DoConstruct,
+ common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t) =
std::move(*doCons);
nextIt = block.erase(nextIt);
// try to match OmpEndLoopDirective
@@ -159,39 +160,46 @@ class CanonicalizationOfOmp {
}
} else if (auto *ompLoopCons{
GetOmpIf<parser::OpenMPLoopConstruct>(*nextIt)}) {
- // We should allow UNROLL and TILE constructs to be inserted between an OpenMP Loop Construct and the DO loop itself
+ // We should allow UNROLL and TILE constructs to be inserted between an
+ // OpenMP Loop Construct and the DO loop itself
auto &beginDirective =
std::get<parser::OmpBeginLoopDirective>(ompLoopCons->t);
auto &beginLoopDirective =
- std::get<parser::OmpLoopDirective>(beginDirective.t);
+ std::get<parser::OmpLoopDirective>(beginDirective.t);
if ((beginLoopDirective.v == llvm::omp::Directive::OMPD_unroll ||
- beginLoopDirective.v == llvm::omp::Directive::OMPD_tile)) {
- // iterate through the remaining block items to find the end directive for the unroll/tile directive.
+ beginLoopDirective.v == llvm::omp::Directive::OMPD_tile)) {
+ // iterate through the remaining block items to find the end directive
+ // for the unroll/tile directive.
parser::Block::iterator endIt;
endIt = nextIt;
- while(endIt != block.end()) {
+ while (endIt != block.end()) {
if (auto *endDir{
- GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
- auto &endLoopDirective = std::get<parser::OmpLoopDirective>(endDir->t);
- if(endLoopDirective.v == dir.v) {
- std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
- std::move(*endDir);
- endIt = block.erase(endIt);
- continue;
- }
+ GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
+ auto &endLoopDirective =
+ std::get<parser::OmpLoopDirective>(endDir->t);
+ if (endLoopDirective.v == dir.v) {
+ std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
+ std::move(*endDir);
+ endIt = block.erase(endIt);
+ continue;
}
- ++endIt;
}
+ ++endIt;
+ }
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
- auto &ompLoop = std::get<std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t);
- ompLoop = std::optional<std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>>{
- std::variant<parser::DoConstruct, common::Indirection<parser::OpenMPLoopConstruct>>{
- common::Indirection{std::move(*ompLoopCons)}}};
+ auto &ompLoop =
+ std::get<std::optional<std::variant<parser::DoConstruct,
+ common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t);
+ ompLoop = std::optional<std::variant<parser::DoConstruct,
+ common::Indirection<parser::OpenMPLoopConstruct>>>{
+ std::variant<parser::DoConstruct,
+ common::Indirection<parser::OpenMPLoopConstruct>>{
+ common::Indirection{std::move(*ompLoopCons)}}};
nextIt = block.erase(nextIt);
} else {
messages_.Say(dir.source,
- "Only OpenMP Loop Transformation Constructs can be nested within OpenMPLoopConstruct's"_err_en_US,
- parser::ToUpperCaseLetters(dir.source.ToString()));
+ "Only OpenMP Loop Transformation Constructs can be nested within OpenMPLoopConstruct's"_err_en_US,
+ parser::ToUpperCaseLetters(dir.source.ToString()));
}
} else {
missingDoConstruct(dir);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9f4504bf3dcc6..aec992729e526 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -763,7 +763,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
SetLoopInfo(x);
auto &optLoopCons = std::get<1>(x.t);
- if(optLoopCons.has_value()) {
+ if (optLoopCons.has_value()) {
if (const auto &doConstruct{
std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
const auto &doBlock{std::get<parser::Block>(doConstruct->t)};
@@ -866,26 +866,26 @@ void OmpStructureChecker::CheckLoopItrVariableIsInt(
auto &optLoopCons = std::get<1>(x.t);
if (optLoopCons.has_value()) {
if (const auto &loopConstruct{
- std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
- for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
- if (loop->IsDoNormal()) {
- const parser::Name &itrVal{GetLoopIndex(loop)};
- if (itrVal.symbol) {
- const auto *type{itrVal.symbol->GetType()};
- if (!type->IsNumeric(TypeCategory::Integer)) {
- context_.Say(itrVal.source,
- "The DO loop iteration"
- " variable must be of the type integer."_err_en_US,
- itrVal.ToString());
+ for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
+ if (loop->IsDoNormal()) {
+ const parser::Name &itrVal{GetLoopIndex(loop)};
+ if (itrVal.symbol) {
+ const auto *type{itrVal.symbol->GetType()};
+ if (!type->IsNumeric(TypeCategory::Integer)) {
+ context_.Say(itrVal.source,
+ "The DO loop iteration"
+ " variable must be of the type integer."_err_en_US,
+ itrVal.ToString());
+ }
}
}
- }
- // Get the next DoConstruct if block is not empty.
- const auto &block{std::get<parser::Block>(loop->t)};
- const auto it{block.begin()};
- loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
- : nullptr;
+ // Get the next DoConstruct if block is not empty.
+ const auto &block{std::get<parser::Block>(loop->t)};
+ const auto it{block.begin()};
+ loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
+ : nullptr;
}
}
}
@@ -1088,25 +1088,25 @@ void OmpStructureChecker::CheckDistLinear(
// clauses.
auto &optLoopCons = std::get<1>(x.t);
if (optLoopCons.has_value()) {
- if (const auto &loopConstruct{
- std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
- for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
- if (loop->IsDoNormal()) {
- const parser::Name &itrVal{GetLoopIndex(loop)};
- if (itrVal.symbol) {
- // Remove the symbol from the collected set
- indexVars.erase(&itrVal.symbol->GetUltimate());
- }
- collapseVal--;
- if (collapseVal == 0) {
- break;
+ if (const auto &loopConstruct{
+ std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
+ if (loop->IsDoNormal()) {
+ const parser::Name &itrVal{GetLoopIndex(loop)};
+ if (itrVal.symbol) {
+ // Remove the symbol from the collected set
+ indexVars.erase(&itrVal.symbol->GetUltimate());
+ }
+ collapseVal--;
+ if (collapseVal == 0) {
+ break;
+ }
}
- }
- // Get the next DoConstruct if block is not empty.
- const auto &block{std::get<parser::Block>(loop->t)};
- const auto it{block.begin()};
- loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
- : nullptr;
+ // Get the next DoConstruct if block is not empty.
+ const auto &block{std::get<parser::Block>(loop->t)};
+ const auto it{block.begin()};
+ loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
+ : nullptr;
}
}
}
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 36bea4fbe7ae5..9b8686022f3c6 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1968,10 +1968,11 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
auto &optLoopCons = std::get<1>(x.t);
if (optLoopCons.has_value()) {
if (const auto &outer{std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
- for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
+ for (const parser::DoConstruct *loop{&*outer}; loop && level > 0;
+ --level) {
if (loop->IsDoConcurrent()) {
- // DO CONCURRENT is explicitly allowed for the LOOP construct so long as
- // there isn't a COLLAPSE clause
+ // 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
@@ -2000,16 +2001,20 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
}
}
CheckAssocLoopLevel(level, GetAssociatedClause());
- } else if (const auto &loop{std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(&*optLoopCons)}) {
+ } else if (const auto &loop{std::get_if<
+ common::Indirection<parser::OpenMPLoopConstruct>>(
+ &*optLoopCons)}) {
auto &beginDirective =
- std::get<parser::OmpBeginLoopDirective>(loop->value().t);
+ std::get<parser::OmpBeginLoopDirective>(loop->value().t);
auto &beginLoopDirective =
std::get<parser::OmpLoopDirective>(beginDirective.t);
if ((beginLoopDirective.v != llvm::omp::Directive::OMPD_unroll &&
- beginLoopDirective.v != llvm::omp::Directive::OMPD_tile)) {
+ beginLoopDirective.v != llvm::omp::Directive::OMPD_tile)) {
context_.Say(GetContext().directiveSource,
- "Only UNROLL or TILE constructs are allowed between an OpenMP Loop Construct and a DO construct"_err_en_US,
- parser::ToUpperCaseLetters(llvm::omp::getOpenMPDirectiveName(GetContext().directive, version).str()));
+ "Only UNROLL or TILE constructs are allowed between an OpenMP Loop Construct and a DO construct"_err_en_US,
+ parser::ToUpperCaseLetters(llvm::omp::getOpenMPDirectiveName(
+ GetContext().directive, version)
+ .str()));
} else {
PrivatizeAssociatedLoopIndexAndCheckLoopLevel(loop->value());
}
>From fec9503c9b354b1900369b2d441badc5d7269f9e Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Fri, 27 Jun 2025 09:52:16 +0100
Subject: [PATCH 4/5] Respond to Review Comments & fix failing test
---
flang/include/flang/Parser/parse-tree.h | 7 ++++---
flang/lib/Lower/OpenMP/OpenMP.cpp | 3 ++-
flang/lib/Semantics/canonicalize-omp.cpp | 19 ++++++++----------
flang/lib/Semantics/check-omp-structure.cpp | 8 ++++----
flang/lib/Semantics/resolve-directives.cpp | 4 ++--
.../loop-transformation-construct01.f90 | 20 ++++++++++++++-----
6 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 8d87354247823..7e752eeb4dfe4 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -267,6 +267,7 @@ struct AccEndCombinedDirective;
struct OpenACCDeclarativeConstruct;
struct OpenACCRoutineConstruct;
struct OpenMPConstruct;
+struct OpenMPLoopConstruct;
struct OpenMPDeclarativeConstruct;
struct OmpEndLoopDirective;
struct OmpMemoryOrderClause;
@@ -5021,13 +5022,13 @@ struct OpenMPBlockConstruct {
};
// OpenMP directives enclosing do loop
+using NestedConstruct =
+ std::variant<DoConstruct, common::Indirection<OpenMPLoopConstruct>>;
struct OpenMPLoopConstruct {
TUPLE_CLASS_BOILERPLATE(OpenMPLoopConstruct);
OpenMPLoopConstruct(OmpBeginLoopDirective &&a)
: t({std::move(a), std::nullopt, std::nullopt}) {}
- std::tuple<OmpBeginLoopDirective,
- std::optional<
- std::variant<DoConstruct, common::Indirection<OpenMPLoopConstruct>>>,
+ std::tuple<OmpBeginLoopDirective, std::optional<NestedConstruct>,
std::optional<OmpEndLoopDirective>>
t;
};
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index c85d015b8279a..a4ce7e8ff1c3a 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -4231,7 +4231,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
mlir::Location currentLocation =
converter.genLocation(beginLoopDirective.source);
- auto &optLoopCons = std::get<1>(loopConstruct.t);
+ auto &optLoopCons =
+ std::get<std::optional<parser::NestedConstruct>>(loopConstruct.t);
if (optLoopCons.has_value())
if (auto *ompNestedLoopCons{
std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index 03d71eef4a7a8..676f7b9599b61 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -166,8 +166,8 @@ class CanonicalizationOfOmp {
std::get<parser::OmpBeginLoopDirective>(ompLoopCons->t);
auto &beginLoopDirective =
std::get<parser::OmpLoopDirective>(beginDirective.t);
- if ((beginLoopDirective.v == llvm::omp::Directive::OMPD_unroll ||
- beginLoopDirective.v == llvm::omp::Directive::OMPD_tile)) {
+ if (beginLoopDirective.v == llvm::omp::Directive::OMPD_unroll ||
+ beginLoopDirective.v == llvm::omp::Directive::OMPD_tile) {
// iterate through the remaining block items to find the end directive
// for the unroll/tile directive.
parser::Block::iterator endIt;
@@ -188,18 +188,15 @@ class CanonicalizationOfOmp {
}
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
auto &ompLoop =
- std::get<std::optional<std::variant<parser::DoConstruct,
- common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t);
- ompLoop = std::optional<std::variant<parser::DoConstruct,
- common::Indirection<parser::OpenMPLoopConstruct>>>{
- std::variant<parser::DoConstruct,
- common::Indirection<parser::OpenMPLoopConstruct>>{
+ std::get<std::optional<parser::NestedConstruct>>(x.t);
+ ompLoop = std::optional<parser::NestedConstruct>{
+ parser::NestedConstruct{
common::Indirection{std::move(*ompLoopCons)}}};
nextIt = block.erase(nextIt);
} else {
- messages_.Say(dir.source,
- "Only OpenMP Loop Transformation Constructs can be nested within OpenMPLoopConstruct's"_err_en_US,
- parser::ToUpperCaseLetters(dir.source.ToString()));
+ messages_.Say(beginLoopDirective.source,
+ "Only Loop Transformation Constructs or Loop Nests can be nested within Loop Constructs"_err_en_US,
+ parser::ToUpperCaseLetters(beginLoopDirective.source.ToString()));
}
} else {
missingDoConstruct(dir);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index aec992729e526..e080bce3cac3a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -762,7 +762,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
}
SetLoopInfo(x);
- auto &optLoopCons = std::get<1>(x.t);
+ auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
if (optLoopCons.has_value()) {
if (const auto &doConstruct{
std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
@@ -789,7 +789,7 @@ const parser::Name OmpStructureChecker::GetLoopIndex(
return std::get<Bounds>(x->GetLoopControl()->u).name.thing;
}
void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) {
- auto &optLoopCons = std::get<1>(x.t);
+ auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
if (optLoopCons.has_value()) {
if (const auto &loopConstruct{
std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
@@ -863,7 +863,7 @@ void OmpStructureChecker::CheckIteratorModifier(const parser::OmpIterator &x) {
void OmpStructureChecker::CheckLoopItrVariableIsInt(
const parser::OpenMPLoopConstruct &x) {
- auto &optLoopCons = std::get<1>(x.t);
+ auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
if (optLoopCons.has_value()) {
if (const auto &loopConstruct{
std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
@@ -1086,7 +1086,7 @@ void OmpStructureChecker::CheckDistLinear(
// Match the loop index variables with the collected symbols from linear
// clauses.
- auto &optLoopCons = std::get<1>(x.t);
+ auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
if (optLoopCons.has_value()) {
if (const auto &loopConstruct{
std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 9b8686022f3c6..ffb3aa9a8822b 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1796,7 +1796,7 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
SetContextAssociatedLoopLevel(GetAssociatedLoopLevelFromClauses(clauseList));
if (beginDir.v == llvm::omp::Directive::OMPD_do) {
- auto &optLoopCons = std::get<1>(x.t);
+ auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
if (optLoopCons.has_value()) {
if (const auto &doConstruct{
std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
@@ -1965,7 +1965,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
bool hasCollapseClause{
clause ? (clause->Id() == llvm::omp::OMPC_collapse) : false};
- auto &optLoopCons = std::get<1>(x.t);
+ auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
if (optLoopCons.has_value()) {
if (const auto &outer{std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
for (const parser::DoConstruct *loop{&*outer}; loop && level > 0;
diff --git a/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90 b/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
index 92a2792ac05a3..eacb5607e1085 100644
--- a/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
+++ b/flang/test/Semantics/OpenMP/loop-transformation-construct01.f90
@@ -34,13 +34,23 @@ subroutine loop_transformation_construct2
integer :: v(i)
!$omp do
- !ERROR: Only OpenMP Loop Transformation Constructs can be nested within OpenMPLoopConstruct's
+ !ERROR: Only Loop Transformation Constructs or Loop Nests can be nested within Loop Constructs
!$omp parallel do
do x = 1, i
v(x) = x(x) * 2
end do
- !! This error occurs because the `parallel do` end directive never gets matched.
- !ERROR: The END PARALLEL DO directive must follow the DO loop associated with the loop construct
- !$omp end parallel do
- !$omp end do
+end subroutine
+
+subroutine loop_transformation_construct3
+ implicit none
+ integer :: i = 5
+ integer :: y
+ integer :: v(i)
+
+ !$omp do
+ do x = 1, i
+ v(x) = x(x) * 2
+ end do
+ !ERROR: A DO loop must follow the TILE directive
+ !$omp tile
end subroutine
>From 3fbadd2424bcf403d8377d5320ac7ab64b096cd8 Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.styles at arm.com>
Date: Fri, 27 Jun 2025 10:27:29 +0100
Subject: [PATCH 5/5] add missing formatting fixes
---
flang/lib/Semantics/canonicalize-omp.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index 676f7b9599b61..c6de3ef0af417 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -187,10 +187,9 @@ class CanonicalizationOfOmp {
++endIt;
}
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
- auto &ompLoop =
- std::get<std::optional<parser::NestedConstruct>>(x.t);
- ompLoop = std::optional<parser::NestedConstruct>{
- parser::NestedConstruct{
+ auto &ompLoop = std::get<std::optional<parser::NestedConstruct>>(x.t);
+ ompLoop =
+ std::optional<parser::NestedConstruct>{parser::NestedConstruct{
common::Indirection{std::move(*ompLoopCons)}}};
nextIt = block.erase(nextIt);
} else {
More information about the flang-commits
mailing list