[flang-commits] [flang] [flang][acc] allow and ignore DIR between ACC and loops (PR #106522)
via flang-commits
flang-commits at lists.llvm.org
Thu Aug 29 03:07:53 PDT 2024
https://github.com/jeanPerier created https://github.com/llvm/llvm-project/pull/106522
The current pattern was failing OpenACC semantics in acc parse tree canonicalization:
```
!acc loop
!dir vector aligned
do i=1,n
...
```
Fix it by moving the directive before the OpenACC construct node.
Note that I think it could make sense to propagate the $dir info to the acc.loop, at least with classic flang, the $dir seems to make a difference. This is not done here since few directives are supported anyway.
>From 1798ccda804ff7fb298700efd04a61d2332c3697 Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Thu, 29 Aug 2024 02:48:25 -0700
Subject: [PATCH] [flang][acc] allow and ignore between and loops
---
flang/lib/Lower/OpenACC.cpp | 7 ++
flang/lib/Semantics/canonicalize-acc.cpp | 16 ++++
.../lib/Semantics/canonicalize-directives.cpp | 19 ++---
.../Lower/OpenACC/acc-loop-and-cpu-dir.f90 | 75 +++++++++++++++++++
flang/test/Semantics/loop-directives.f90 | 11 +++
5 files changed, 116 insertions(+), 12 deletions(-)
create mode 100644 flang/test/Lower/OpenACC/acc-loop-and-cpu-dir.f90
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index be184aeead6ee5..431fab52872d33 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -2080,6 +2080,13 @@ static mlir::acc::LoopOp createLoopOp(
loopOp.setCombinedAttr(mlir::acc::CombinedConstructsTypeAttr::get(
builder.getContext(), *combinedConstructs));
+ // TODO: retrieve directives from NonLabelDoStmt pft::Evaluation, and add them
+ // as attribute to the acc.loop as an extra attribute. It is not quite clear
+ // how useful these $dir are in acc contexts, but they could still provide
+ // more information about the loop acc codegen. They can be obtained by
+ // looking for the first lexicalSuccessor of eval that is a NonLabelDoStmt,
+ // and using the related `dirs` member.
+
return loopOp;
}
diff --git a/flang/lib/Semantics/canonicalize-acc.cpp b/flang/lib/Semantics/canonicalize-acc.cpp
index f9e44b9540dbb9..108fd33c2ed949 100644
--- a/flang/lib/Semantics/canonicalize-acc.cpp
+++ b/flang/lib/Semantics/canonicalize-acc.cpp
@@ -107,6 +107,20 @@ class CanonicalizationOfAcc {
}
}
+ // Utility to move all parser::CompilerDirective right after it to right
+ // before it. This allows preserving loop directives $DIR that may lie
+ // between an $acc directive and loop and leave lowering decide if it should
+ // ignore them or lower/apply them to the acc loops.
+ void moveCompilerDirectivesBefore(
+ parser::Block &block, parser::Block::iterator it) {
+ parser::Block::iterator nextIt = std::next(it);
+ while (nextIt != block.end() &&
+ parser::Unwrap<parser::CompilerDirective>(*nextIt)) {
+ block.emplace(it, std::move(*nextIt));
+ nextIt = block.erase(nextIt);
+ }
+ }
+
void RewriteOpenACCLoopConstruct(parser::OpenACCLoopConstruct &x,
parser::Block &block, parser::Block::iterator it) {
parser::Block::iterator nextIt;
@@ -115,6 +129,7 @@ class CanonicalizationOfAcc {
auto &nestedDo{std::get<std::optional<parser::DoConstruct>>(x.t)};
if (!nestedDo) {
+ moveCompilerDirectivesBefore(block, it);
nextIt = it;
if (++nextIt != block.end()) {
if (auto *doCons{parser::Unwrap<parser::DoConstruct>(*nextIt)}) {
@@ -151,6 +166,7 @@ class CanonicalizationOfAcc {
auto &nestedDo{std::get<std::optional<parser::DoConstruct>>(x.t)};
if (!nestedDo) {
+ moveCompilerDirectivesBefore(block, it);
nextIt = it;
if (++nextIt != block.end()) {
if (auto *doCons{parser::Unwrap<parser::DoConstruct>(*nextIt)}) {
diff --git a/flang/lib/Semantics/canonicalize-directives.cpp b/flang/lib/Semantics/canonicalize-directives.cpp
index ae691ab612ba27..739bc3c1992ba6 100644
--- a/flang/lib/Semantics/canonicalize-directives.cpp
+++ b/flang/lib/Semantics/canonicalize-directives.cpp
@@ -8,6 +8,7 @@
#include "canonicalize-directives.h"
#include "flang/Parser/parse-tree-visitor.h"
+#include "flang/Semantics/tools.h"
namespace Fortran::semantics {
@@ -82,25 +83,19 @@ bool CanonicalizationOfDirectives::Pre(parser::ExecutionPart &x) {
return true;
}
-template <typename T> T *GetConstructIf(parser::ExecutionPartConstruct &x) {
- if (auto *y{std::get_if<parser::ExecutableConstruct>(&x.u)}) {
- if (auto *z{std::get_if<common::Indirection<T>>(&y->u)}) {
- return &z->value();
- }
- }
- return nullptr;
-}
-
void CanonicalizationOfDirectives::CheckLoopDirective(
parser::CompilerDirective &dir, parser::Block &block,
std::list<parser::ExecutionPartConstruct>::iterator it) {
// Skip over this and other compiler directives
- while (GetConstructIf<parser::CompilerDirective>(*it)) {
+ while (it != block.end() && parser::Unwrap<parser::CompilerDirective>(*it)) {
++it;
}
- if (it == block.end() || !GetConstructIf<parser::DoConstruct>(*it)) {
+ if (it == block.end() ||
+ (!parser::Unwrap<parser::DoConstruct>(*it) &&
+ !parser::Unwrap<parser::OpenACCLoopConstruct>(*it) &&
+ !parser::Unwrap<parser::OpenACCCombinedConstruct>(*it))) {
std::string s{parser::ToUpperCaseLetters(dir.source.ToString())};
s.pop_back(); // Remove trailing newline from source string
messages_.Say(
@@ -110,7 +105,7 @@ void CanonicalizationOfDirectives::CheckLoopDirective(
void CanonicalizationOfDirectives::Post(parser::Block &block) {
for (auto it{block.begin()}; it != block.end(); ++it) {
- if (auto *dir{GetConstructIf<parser::CompilerDirective>(*it)}) {
+ if (auto *dir{parser::Unwrap<parser::CompilerDirective>(*it)}) {
std::visit(
common::visitors{[&](parser::CompilerDirective::VectorAlways &) {
CheckLoopDirective(*dir, block, it);
diff --git a/flang/test/Lower/OpenACC/acc-loop-and-cpu-dir.f90 b/flang/test/Lower/OpenACC/acc-loop-and-cpu-dir.f90
new file mode 100644
index 00000000000000..51c6c367d653e0
--- /dev/null
+++ b/flang/test/Lower/OpenACC/acc-loop-and-cpu-dir.f90
@@ -0,0 +1,75 @@
+! Test that $dir loop directives (known or unknown) are not clashing
+! with $acc lowering.
+
+! RUN: %flang_fc1 -fopenacc -emit-hlfir %s -o - | FileCheck %s
+
+subroutine test_before_acc_loop(a, b, c)
+ real, dimension(10) :: a,b,c
+ !dir$ myloop_directive_1
+ !dir$ myloop_directive_2
+ !$acc loop
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
+! CHECK-LABEL: test_before_acc_loop
+! CHECK: acc.loop
+
+subroutine test_after_acc_loop(a, b, c)
+ real, dimension(10) :: a,b,c
+ !$acc loop
+ !dir$ myloop_directive_1
+ !dir$ myloop_directive_2
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
+! CHECK-LABEL: test_after_acc_loop
+! CHECK: acc.loop
+
+subroutine test_before_acc_combined(a, b, c)
+ real, dimension(10) :: a,b,c
+ !dir$ myloop_directive_1
+ !dir$ myloop_directive_2
+ !$acc parallel loop
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
+! CHECK-LABEL: test_before_acc_combined
+! CHECK: acc.parallel combined(loop)
+
+subroutine test_after_acc_combined(a, b, c)
+ real, dimension(10) :: a,b,c
+ !$acc parallel loop
+ !dir$ myloop_directive_1
+ !dir$ myloop_directive_2
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
+! CHECK-LABEL: test_after_acc_combined
+! CHECK: acc.parallel combined(loop)
+
+
+subroutine test_vector_always_after_acc(a, b, c)
+ real, dimension(10) :: a,b,c
+ !$acc loop
+ !dir$ vector always
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
+! CHECK-LABEL: test_vector_always_after_acc
+! CHECK: acc.loop
+
+subroutine test_vector_always_before_acc(a, b, c)
+ real, dimension(10) :: a,b,c
+ !dir$ vector always
+ !$acc loop
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
+! CHECK-LABEL: test_vector_always_before_acc
+! CHECK: acc.loop
diff --git a/flang/test/Semantics/loop-directives.f90 b/flang/test/Semantics/loop-directives.f90
index 6f994c767d45fe..58fb9b8082bc1a 100644
--- a/flang/test/Semantics/loop-directives.f90
+++ b/flang/test/Semantics/loop-directives.f90
@@ -1,4 +1,5 @@
! RUN: %python %S/test_errors.py %s %flang_fc1 -Werror
+! RUN: %python %S/test_errors.py %s %flang_fc1 -fopenacc -Werror
subroutine empty
! WARNING: A DO loop must follow the VECTOR ALWAYS directive
@@ -17,3 +18,13 @@ subroutine execution_part
!dir$ vector always
end do
end subroutine execution_part
+
+! OK
+subroutine test_vector_always_before_acc(a, b, c)
+ real, dimension(10) :: a,b,c
+ !dir$ vector always
+ !$acc loop
+ do i=1,N
+ a(i) = b(i) + c(i)
+ enddo
+end subroutine
More information about the flang-commits
mailing list