[flang-commits] [flang] [flang][OpenACC] Skip non-loop evaluations when descending a collapsed/tiled DO nest (PR #223579)
Ron Green [NVIDIA] via flang-commits
flang-commits at lists.llvm.org
Wed Sep 16 12:11:03 PDT 2026
https://github.com/ronGreenNV updated https://github.com/llvm/llvm-project/pull/223579
>From 07b89605188b53533d96d5836dbe66344f6b33dc Mon Sep 17 00:00:00 2001
From: Ronald Green <rogreen at nvidia.com>
Date: Mon, 14 Sep 2026 18:45:22 -0700
Subject: [PATCH 1/4] [flang][OpenACC] Skip non-loop evaluations when
descending a collapsed/tiled DO nest
The collapse/tile loop-nest descent in Bridge.cpp (used for OpenACC
collapse/tile and CUDA Fortran `!$cuf kernel do`) assumed each loop
level's nested evaluations have the fixed shape
{NonLabelDoStmt, DoConstruct, EndDoStmt} and unconditionally advanced
to the second nested evaluation to find the next inner DO. A compiler
directive (e.g. !DIR$ IVDEP) placed between loop levels inserts an
extra sibling evaluation there, so the descent landed on the directive
instead of the inner DoConstruct and lowered the directive's (empty)
nested evaluations as the loop body -- silently dropping the real
body with no diagnostic.
Add a helper that searches a level's nested evaluations for the actual
DoConstruct, matching the approach already used by visitLoopControl in
OpenACC.cpp, and use it at all three affected descent sites.
---
flang/lib/Lower/Bridge.cpp | 37 +++++++++++--
...-loop-collapse-directive-between-loops.f90 | 55 +++++++++++++++++++
2 files changed, 86 insertions(+), 6 deletions(-)
create mode 100644 flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 4b70fbe9ea106..2b5bdeab6d118 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -502,6 +502,20 @@ class TypeInfoConverter {
using IncrementLoopNestInfo = llvm::SmallVector<IncrementLoopInfo, 8>;
} // namespace
+/// Find the first nested DoConstruct evaluation directly under \p eval,
+/// skipping over any other sibling evaluations (e.g. a CompilerDirective
+/// such as !DIR$ IVDEP) that may appear between loop levels of a collapsed
+/// or tiled loop nest. Returns nullptr if none is found.
+static Fortran::lower::pft::Evaluation *
+findNestedDoConstructEvaluation(Fortran::lower::pft::Evaluation &eval) {
+ if (!eval.hasNestedEvaluations())
+ return nullptr;
+ for (Fortran::lower::pft::Evaluation &child : eval.getNestedEvaluations())
+ if (child.getIf<Fortran::parser::DoConstruct>())
+ return &child;
+ return nullptr;
+}
+
//===----------------------------------------------------------------------===//
// FirConverter
//===----------------------------------------------------------------------===//
@@ -3638,9 +3652,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
const auto *outerDo = curEval->getIf<Fortran::parser::DoConstruct>();
if (!(outerDo && outerDo->IsDoConcurrent()))
for (uint64_t i = 1; i < loopCount; i++) {
- if (!curEval->hasNestedEvaluations())
+ Fortran::lower::pft::Evaluation *nextDo =
+ findNestedDoConstructEvaluation(*curEval);
+ if (!nextDo)
break;
- curEval = &*std::next(curEval->getNestedEvaluations().begin());
+ curEval = nextDo;
}
}
}
@@ -3983,8 +3999,12 @@ class FirConverter : public Fortran::lower::AbstractConverter {
ivTypes.push_back(idxTy);
ivLocs.push_back(crtLoc);
- if (i < nestedLoops - 1)
- loopEval = &*std::next(loopEval->getNestedEvaluations().begin());
+ if (i < nestedLoops - 1) {
+ Fortran::lower::pft::Evaluation *nextDo =
+ findNestedDoConstructEvaluation(*loopEval);
+ assert(nextDo && "expected a nested DO CONSTRUCT");
+ loopEval = nextDo;
+ }
}
}
@@ -4012,8 +4032,13 @@ class FirConverter : public Fortran::lower::AbstractConverter {
if (crtEval->lowerAsStructured()) {
crtEval = &crtEval->getFirstNestedEvaluation();
if (!outerDoConstruct->IsDoConcurrent())
- for (int64_t i = 1; i < nestedLoops; i++)
- crtEval = &*std::next(crtEval->getNestedEvaluations().begin());
+ for (int64_t i = 1; i < nestedLoops; i++) {
+ Fortran::lower::pft::Evaluation *nextDo =
+ findNestedDoConstructEvaluation(*crtEval);
+ if (!nextDo)
+ break;
+ crtEval = nextDo;
+ }
}
// Generate loop body
diff --git a/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90 b/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90
new file mode 100644
index 0000000000000..89f551ae2492f
--- /dev/null
+++ b/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90
@@ -0,0 +1,55 @@
+! RUN: bbc -fopenacc -emit-hlfir %s -o - | FileCheck %s
+
+! Verify that a compiler directive (e.g. !DIR$ IVDEP) appearing between the
+! levels of a collapsed loop nest does not get mistaken for the next nested
+! DO CONSTRUCT. The directive is an extra sibling evaluation between the
+! NonLabelDoStmt and the inner DoConstruct; the collapse descent must skip
+! over it rather than absorbing it as the loop body.
+
+subroutine collapse2_directive_between_loops(n, a)
+ integer, intent(in) :: n
+ integer :: a(n,n)
+ integer :: i, j
+
+ !$acc parallel loop collapse(2) copy(a)
+ do i = 1, n
+!DIR$ IVDEP
+ do j = 1, n
+ a(j,i) = 1
+ end do
+ end do
+ !$acc end parallel loop
+end subroutine
+
+! CHECK-LABEL: func.func @_QPcollapse2_directive_between_loops(
+! CHECK: acc.parallel
+! CHECK: acc.loop combined(parallel)
+! CHECK: hlfir.designate
+! CHECK: hlfir.assign
+! CHECK: acc.yield
+! CHECK: collapse([2])
+
+subroutine collapse3_directive_between_loops(n, a)
+ integer, intent(in) :: n
+ integer :: a(n,n,n)
+ integer :: i, j, k
+
+ !$acc parallel loop collapse(3) copy(a)
+ do i = 1, n
+!DIR$ NOVECTOR
+ do j = 1, n
+ do k = 1, n
+ a(k,j,i) = 1
+ end do
+ end do
+ end do
+ !$acc end parallel loop
+end subroutine
+
+! CHECK-LABEL: func.func @_QPcollapse3_directive_between_loops(
+! CHECK: acc.parallel
+! CHECK: acc.loop combined(parallel)
+! CHECK: hlfir.designate
+! CHECK: hlfir.assign
+! CHECK: acc.yield
+! CHECK: collapse([3])
>From f0b7f148ae236be942ed2296ac0bad2e7d7b38f4 Mon Sep 17 00:00:00 2001
From: Ronald Green <rogreen at nvidia.com>
Date: Tue, 15 Sep 2026 14:15:17 -0700
Subject: [PATCH 2/4] [flang][OpenACC] Warn on directives dropped by
collapse/tile descent; share descent helper
Address review feedback on the collapse/tile loop-nest descent fix:
- A compiler directive (e.g. !DIR$ IVDEP) skipped over while descending to
the next loop level is neither part of the collapsed body nor attached to
a separately-lowered DO statement, so it previously had no effect and no
diagnostic. Emit a warning naming the directive's location instead of
silently dropping it.
- visitLoopControl in OpenACC.cpp had its own inline copy of the same
"find the next nested DoConstruct" search added to Bridge.cpp. Share one
implementation (Fortran::lower::findNestedDoConstructEvaluation, declared
in OpenACC.h) between the two so they cannot drift apart. The bounds-only
descent in visitLoopControl does not itself warn, since the body descent
in Bridge.cpp already does so for the same construct.
- Add coverage for a directive between the inner levels of a collapse(3)
nest (not just between the outer and first inner loop) and for
!$acc loop tile.
- Add a negative test confirming !$cuf kernel do rejects an intervening
directive at semantic-analysis time (DoConstructTightNesting), which is
why CUF's two similarly-fixed descent sites are not reachable via this
bug class today.
---
flang/include/flang/Lower/OpenACC.h | 12 +++++
flang/lib/Lower/Bridge.cpp | 44 +++++++++-------
flang/lib/Lower/OpenACC.cpp | 41 ++++++++++-----
...-loop-collapse-directive-between-loops.f90 | 50 +++++++++++++++++++
flang/test/Semantics/CUDA/cuf09.cuf | 10 ++++
5 files changed, 127 insertions(+), 30 deletions(-)
diff --git a/flang/include/flang/Lower/OpenACC.h b/flang/include/flang/Lower/OpenACC.h
index e0cc3aff898f8..fa29cfa68460d 100644
--- a/flang/include/flang/Lower/OpenACC.h
+++ b/flang/include/flang/Lower/OpenACC.h
@@ -18,6 +18,8 @@
namespace llvm {
template <typename T, unsigned N>
class SmallVector;
+template <typename T>
+class SmallVectorImpl;
class StringRef;
} // namespace llvm
@@ -129,6 +131,16 @@ bool isCollapsedDoConstruct(const Fortran::parser::DoConstruct &);
/// Clear the collapsed DoConstruct tracking set.
void clearCollapsedDoConstructs();
+/// Find the first nested DoConstruct evaluation directly under \p eval,
+/// skipping over any other sibling evaluations (e.g. a CompilerDirective
+/// such as !DIR$ IVDEP) that may appear between loop levels of a collapsed
+/// or tiled loop nest. Evaluations skipped over while searching are
+/// appended, in order, to \p skipped if it is non-null. Returns nullptr if
+/// no nested DoConstruct is found.
+pft::Evaluation *findNestedDoConstructEvaluation(
+ pft::Evaluation &eval,
+ llvm::SmallVectorImpl<pft::Evaluation *> *skipped = nullptr);
+
/// Checks whether the current insertion point is inside OpenACC compute
/// construct.
bool isInsideOpenACCComputeConstruct(fir::FirOpBuilder &);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 2b5bdeab6d118..2424d7c8fb8b1 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -502,20 +502,6 @@ class TypeInfoConverter {
using IncrementLoopNestInfo = llvm::SmallVector<IncrementLoopInfo, 8>;
} // namespace
-/// Find the first nested DoConstruct evaluation directly under \p eval,
-/// skipping over any other sibling evaluations (e.g. a CompilerDirective
-/// such as !DIR$ IVDEP) that may appear between loop levels of a collapsed
-/// or tiled loop nest. Returns nullptr if none is found.
-static Fortran::lower::pft::Evaluation *
-findNestedDoConstructEvaluation(Fortran::lower::pft::Evaluation &eval) {
- if (!eval.hasNestedEvaluations())
- return nullptr;
- for (Fortran::lower::pft::Evaluation &child : eval.getNestedEvaluations())
- if (child.getIf<Fortran::parser::DoConstruct>())
- return &child;
- return nullptr;
-}
-
//===----------------------------------------------------------------------===//
// FirConverter
//===----------------------------------------------------------------------===//
@@ -3475,6 +3461,21 @@ class FirConverter : public Fortran::lower::AbstractConverter {
attachToDoStmt(e);
}
+ /// Warn about each compiler directive (e.g. !DIR$ IVDEP) found in
+ /// \p skipped. These are evaluations that were skipped over while
+ /// descending a collapsed or tiled loop nest to find the next inner
+ /// DO CONSTRUCT (see findNestedDoConstructEvaluation): since the
+ /// directive is neither part of the collapsed loop's body nor attached
+ /// to a DO statement that is separately lowered, it has no effect.
+ void warnAboutSkippedDirectives(
+ llvm::ArrayRef<Fortran::lower::pft::Evaluation *> skipped) {
+ for (Fortran::lower::pft::Evaluation *e : skipped)
+ if (e->isDirective())
+ mlir::emitWarning(genLocation(e->position),
+ "compiler directive ignored: it appears between "
+ "loop levels of a collapsed or tiled loop nest");
+ }
+
void markCurrentFuncAsAlwaysInline(
const Fortran::parser::CompilerDirective::InlineAlways &dir) {
mlir::func::FuncOp func = builder->getFunction();
@@ -3652,8 +3653,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
const auto *outerDo = curEval->getIf<Fortran::parser::DoConstruct>();
if (!(outerDo && outerDo->IsDoConcurrent()))
for (uint64_t i = 1; i < loopCount; i++) {
+ llvm::SmallVector<Fortran::lower::pft::Evaluation *> skipped;
Fortran::lower::pft::Evaluation *nextDo =
- findNestedDoConstructEvaluation(*curEval);
+ Fortran::lower::findNestedDoConstructEvaluation(*curEval,
+ &skipped);
+ warnAboutSkippedDirectives(skipped);
if (!nextDo)
break;
curEval = nextDo;
@@ -4000,8 +4004,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
ivTypes.push_back(idxTy);
ivLocs.push_back(crtLoc);
if (i < nestedLoops - 1) {
+ llvm::SmallVector<Fortran::lower::pft::Evaluation *> skipped;
Fortran::lower::pft::Evaluation *nextDo =
- findNestedDoConstructEvaluation(*loopEval);
+ Fortran::lower::findNestedDoConstructEvaluation(*loopEval,
+ &skipped);
+ warnAboutSkippedDirectives(skipped);
assert(nextDo && "expected a nested DO CONSTRUCT");
loopEval = nextDo;
}
@@ -4033,8 +4040,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
crtEval = &crtEval->getFirstNestedEvaluation();
if (!outerDoConstruct->IsDoConcurrent())
for (int64_t i = 1; i < nestedLoops; i++) {
+ llvm::SmallVector<Fortran::lower::pft::Evaluation *> skipped;
Fortran::lower::pft::Evaluation *nextDo =
- findNestedDoConstructEvaluation(*crtEval);
+ Fortran::lower::findNestedDoConstructEvaluation(*crtEval,
+ &skipped);
+ warnAboutSkippedDirectives(skipped);
if (!nextDo)
break;
crtEval = nextDo;
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 5c23837d03471..7472b01b43e47 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -1768,21 +1768,21 @@ static void visitLoopControl(
callback(std::get<Fortran::parser::LoopControl::Bounds>(loopControl->u),
loc);
} else {
- // Safely locate the next inner DoConstruct within this eval.
- const Fortran::parser::DoConstruct *innerDo = nullptr;
- if (crtEval && crtEval->hasNestedEvaluations()) {
- for (Fortran::lower::pft::Evaluation &child :
- crtEval->getNestedEvaluations()) {
- if (auto *stmt = child.getIf<Fortran::parser::DoConstruct>()) {
- innerDo = stmt;
- // Prepare to descend for the next iteration
- crtEval = &child;
- break;
- }
- }
- }
+ // Safely locate the next inner DoConstruct within this eval, skipping
+ // over any intervening evaluations (e.g. a CompilerDirective such as
+ // !DIR$ IVDEP) that may sit between loop levels. The separate body
+ // descent in Bridge.cpp performs the same search over this same
+ // construct and is responsible for warning about skipped directives,
+ // so this bounds-only descent does not warn again here.
+ Fortran::lower::pft::Evaluation *nextEval =
+ crtEval ? Fortran::lower::findNestedDoConstructEvaluation(*crtEval)
+ : nullptr;
+ const Fortran::parser::DoConstruct *innerDo =
+ nextEval ? nextEval->getIf<Fortran::parser::DoConstruct>() : nullptr;
if (!innerDo)
break; // No deeper loop; stop collecting collapsed bounds.
+ // Prepare to descend for the next iteration.
+ crtEval = nextEval;
if (markInnerCollapsed)
Fortran::lower::markDoConstructAsCollapsed(*innerDo);
@@ -5452,6 +5452,21 @@ void Fortran::lower::clearCollapsedDoConstructs() {
collapsedDoConstructs.clear();
}
+Fortran::lower::pft::Evaluation *
+Fortran::lower::findNestedDoConstructEvaluation(
+ Fortran::lower::pft::Evaluation &eval,
+ llvm::SmallVectorImpl<Fortran::lower::pft::Evaluation *> *skipped) {
+ if (!eval.hasNestedEvaluations())
+ return nullptr;
+ for (Fortran::lower::pft::Evaluation &child : eval.getNestedEvaluations()) {
+ if (child.getIf<Fortran::parser::DoConstruct>())
+ return &child;
+ if (skipped)
+ skipped->push_back(&child);
+ }
+ return nullptr;
+}
+
bool Fortran::lower::isInsideOpenACCComputeConstruct(
fir::FirOpBuilder &builder) {
return mlir::isa_and_nonnull<ACC_COMPUTE_CONSTRUCT_OPS>(
diff --git a/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90 b/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90
index 89f551ae2492f..e2e13c2ee2984 100644
--- a/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90
+++ b/flang/test/Lower/OpenACC/acc-loop-collapse-directive-between-loops.f90
@@ -53,3 +53,53 @@ subroutine collapse3_directive_between_loops(n, a)
! CHECK: hlfir.assign
! CHECK: acc.yield
! CHECK: collapse([3])
+
+! A second directive between the inner loop levels (as opposed to between
+! the outer and first inner loop) exercises every iteration of the descent,
+! not just the first.
+subroutine collapse3_directive_between_inner_loops(n, a)
+ integer, intent(in) :: n
+ integer :: a(n,n,n)
+ integer :: i, j, k
+
+ !$acc parallel loop collapse(3) copy(a)
+ do i = 1, n
+ do j = 1, n
+!DIR$ UNROLL(2)
+ do k = 1, n
+ a(k,j,i) = 1
+ end do
+ end do
+ end do
+ !$acc end parallel loop
+end subroutine
+
+! CHECK-LABEL: func.func @_QPcollapse3_directive_between_inner_loops(
+! CHECK: acc.parallel
+! CHECK: acc.loop combined(parallel)
+! CHECK: hlfir.designate
+! CHECK: hlfir.assign
+! CHECK: acc.yield
+! CHECK: collapse([3])
+
+subroutine tile_directive_between_loops(n, a)
+ integer, intent(in) :: n
+ integer :: a(n,n)
+ integer :: i, j
+
+ !$acc parallel loop tile(2, 2) copy(a)
+ do i = 1, n
+!DIR$ IVDEP
+ do j = 1, n
+ a(j,i) = 1
+ end do
+ end do
+ !$acc end parallel loop
+end subroutine
+
+! CHECK-LABEL: func.func @_QPtile_directive_between_loops(
+! CHECK: acc.parallel
+! CHECK: acc.loop combined(parallel) {{.*}} tile(
+! CHECK: hlfir.designate
+! CHECK: hlfir.assign
+! CHECK: acc.yield
diff --git a/flang/test/Semantics/CUDA/cuf09.cuf b/flang/test/Semantics/CUDA/cuf09.cuf
index 9e2b583386f7a..09f1abce098eb 100644
--- a/flang/test/Semantics/CUDA/cuf09.cuf
+++ b/flang/test/Semantics/CUDA/cuf09.cuf
@@ -207,6 +207,16 @@ program main
end do
continue
end do
+ ! A compiler directive between the outer and inner DO is likewise not
+ ! tight nesting: the outer loop's body has two elements (the directive
+ ! and the inner DO), not one.
+ !ERROR: !$CUF KERNEL DO (2) must be followed by a DO construct with tightly nested outer levels of counted DO loops
+ !$cuf kernel do (2) <<< 1, 2 >>>
+ do j=1,10
+ !dir$ ivdep
+ do k=1,10
+ end do
+ end do
!$cuf kernel do <<< 1, 2 >>>
do j = 1, 10
!ERROR: 'foo' may not be called in device code
>From 387073535776db606f8ff1c8db8e867c3dd43cb0 Mon Sep 17 00:00:00 2001
From: Ronald Green <rogreen at nvidia.com>
Date: Wed, 16 Sep 2026 11:41:38 -0700
Subject: [PATCH 3/4] [flang][OpenACC] Document why the skipped-directive
warning uses mlir::emitWarning
Explored switching warnAboutSkippedDirectives to
SemanticsContext::Warn(UsageWarning::IgnoredDirective, ...), matching the
mechanism used by other "directive ignored" diagnostics so it would
participate in -W/-Wno-/-Werror. It doesn't work from lowering:
SemanticsContext::EmitMessages runs exactly once, immediately after
semantic analysis and before lowering starts, in all three call sites
(FrontendAction.cpp x2, bbc.cpp). A Warn() call made during lowering is
buffered into the SemanticsContext's message list and never flushed, so
the warning silently disappears. No other code under flang/lib/Lower
calls SemanticsContext::Warn/Say for this reason -- mlir::emitWarning is
the established mechanism for lowering-time diagnostics throughout the
codebase. Kept mlir::emitWarning and recorded why, so this doesn't come
up again as a "should be using X" nit.
---
flang/lib/Lower/Bridge.cpp | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 2424d7c8fb8b1..e588ff324aee2 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -3467,6 +3467,12 @@ class FirConverter : public Fortran::lower::AbstractConverter {
/// DO CONSTRUCT (see findNestedDoConstructEvaluation): since the
/// directive is neither part of the collapsed loop's body nor attached
/// to a DO statement that is separately lowered, it has no effect.
+ ///
+ /// This uses mlir::emitWarning rather than SemanticsContext::Warn:
+ /// SemanticsContext::EmitMessages runs once, immediately after semantic
+ /// analysis and before lowering ever starts (see FrontendAction.cpp and
+ /// bbc.cpp), so a Warn() call made here during lowering would be buffered
+ /// into the SemanticsContext's message list and never flushed to output.
void warnAboutSkippedDirectives(
llvm::ArrayRef<Fortran::lower::pft::Evaluation *> skipped) {
for (Fortran::lower::pft::Evaluation *e : skipped)
>From 68b43834240af38b53683303d15d445b30d717fb Mon Sep 17 00:00:00 2001
From: Ronald Green <rogreen at nvidia.com>
Date: Wed, 16 Sep 2026 12:10:20 -0700
Subject: [PATCH 4/4] [flang][OpenACC] Use a positive list for evaluations
skipped by the collapse/tile descent
Address review feedback: findNestedDoConstructEvaluation's skip loop
previously collected every non-DoConstruct sibling with no diagnostic
beyond the directive-only warning, so a genuinely unexpected node would
be silently dropped -- the same failure mode the original fix addresses,
just narrowed rather than eliminated. Use a positive list instead:
- NonLabelDoStmt heading the level being searched is always present and
is silently expected.
- A compiler directive is warned about (ordinary statements between loop
levels are already rejected earlier, in semantic analysis, so a
directive is the only other thing that can legitimately reach here).
- Anything else now fails loudly with TODO instead of being discarded.
Also note, per review, that a directive on the outer loop is moved ahead
of the OpenACC directive during parse-tree canonicalization (#106522)
and can be attached to the acc.loop as a real annotation via
attachDirectiveToLoop (#216769); an inner-loop directive as handled here
cannot be moved that way, so for now it is only warned about. Propagating
it onto the acc.loop the same way, instead of only warning, would be
worth doing later.
---
flang/lib/Lower/Bridge.cpp | 59 ++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 12 deletions(-)
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index e588ff324aee2..6c2f99248dd3b 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -3461,25 +3461,60 @@ class FirConverter : public Fortran::lower::AbstractConverter {
attachToDoStmt(e);
}
- /// Warn about each compiler directive (e.g. !DIR$ IVDEP) found in
- /// \p skipped. These are evaluations that were skipped over while
- /// descending a collapsed or tiled loop nest to find the next inner
- /// DO CONSTRUCT (see findNestedDoConstructEvaluation): since the
- /// directive is neither part of the collapsed loop's body nor attached
- /// to a DO statement that is separately lowered, it has no effect.
+ /// Diagnose each evaluation in \p skipped: these are evaluations that
+ /// were skipped over while descending a collapsed or tiled loop nest to
+ /// find the next inner DO CONSTRUCT (see findNestedDoConstructEvaluation).
+ ///
+ /// The NonLabelDoStmt heading the level being searched is always present
+ /// and is silently expected. A compiler directive (e.g. !DIR$ IVDEP) is
+ /// the only other kind of skipped evaluation known to legitimately
+ /// appear here (OpenACC's collapse/tile clauses do not otherwise require
+ /// the loop nest to be tightly nested, unlike e.g. CUDA Fortran's
+ /// `!$cuf kernel do`; an ordinary statement between loop levels is
+ /// already rejected earlier, in semantic analysis). A directive is
+ /// neither part of the collapsed loop's body nor attached to a DO
+ /// statement that is separately lowered, so it has no effect: warn about
+ /// it rather than silently dropping it.
+ ///
+ /// A directive on the outer loop of a construct is moved ahead of the
+ /// OpenACC directive during parse-tree canonicalization (#106522) and
+ /// can be attached to the resulting acc.loop as a real annotation via
+ /// attachDirectiveToLoop (#216769). A directive between inner loop
+ /// levels, as handled here, cannot be moved that way -- it must stay
+ /// directly above the loop it modifies -- so it is only warned about for
+ /// now. Propagating it onto the acc.loop the same way outer-loop
+ /// directives are, instead of only warning, would be worth doing later.
+ ///
+ /// Anything else means the loop nest was not tightly nested in a way
+ /// this code does not know how to handle -- rather than silently
+ /// discarding unknown statements (the original failure mode this whole
+ /// descent fix addresses), fail loudly with TODO so an unsupported case
+ /// gets reported instead of miscompiled.
///
/// This uses mlir::emitWarning rather than SemanticsContext::Warn:
/// SemanticsContext::EmitMessages runs once, immediately after semantic
/// analysis and before lowering ever starts (see FrontendAction.cpp and
/// bbc.cpp), so a Warn() call made here during lowering would be buffered
/// into the SemanticsContext's message list and never flushed to output.
- void warnAboutSkippedDirectives(
+ void diagnoseSkippedEvaluations(
llvm::ArrayRef<Fortran::lower::pft::Evaluation *> skipped) {
- for (Fortran::lower::pft::Evaluation *e : skipped)
- if (e->isDirective())
+ for (Fortran::lower::pft::Evaluation *e : skipped) {
+ // The NonLabelDoStmt heading the level being searched is always a
+ // sibling of whatever comes next (a directive, or the inner
+ // DoConstruct) and is expected here on every call, not just when a
+ // directive is present.
+ if (e->isA<Fortran::parser::NonLabelDoStmt>())
+ continue;
+ if (e->isDirective()) {
mlir::emitWarning(genLocation(e->position),
"compiler directive ignored: it appears between "
"loop levels of a collapsed or tiled loop nest");
+ continue;
+ }
+ TODO(genLocation(e->position),
+ "unsupported statement between the levels of a collapsed or "
+ "tiled loop nest");
+ }
}
void markCurrentFuncAsAlwaysInline(
@@ -3663,7 +3698,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
Fortran::lower::pft::Evaluation *nextDo =
Fortran::lower::findNestedDoConstructEvaluation(*curEval,
&skipped);
- warnAboutSkippedDirectives(skipped);
+ diagnoseSkippedEvaluations(skipped);
if (!nextDo)
break;
curEval = nextDo;
@@ -4014,7 +4049,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
Fortran::lower::pft::Evaluation *nextDo =
Fortran::lower::findNestedDoConstructEvaluation(*loopEval,
&skipped);
- warnAboutSkippedDirectives(skipped);
+ diagnoseSkippedEvaluations(skipped);
assert(nextDo && "expected a nested DO CONSTRUCT");
loopEval = nextDo;
}
@@ -4050,7 +4085,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
Fortran::lower::pft::Evaluation *nextDo =
Fortran::lower::findNestedDoConstructEvaluation(*crtEval,
&skipped);
- warnAboutSkippedDirectives(skipped);
+ diagnoseSkippedEvaluations(skipped);
if (!nextDo)
break;
crtEval = nextDo;
More information about the flang-commits
mailing list