[flang-commits] [flang] [flang][semantics][OpenMP] no privatisation of stmt functions (PR #106550)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Thu Oct 3 08:52:04 PDT 2024
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/106550
>From 2c6b70ff1523501c39a3ec56a7a0ac9dacc32112 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 29 Aug 2024 10:19:56 +0000
Subject: [PATCH 1/5] [flang][semantics][OpenMP] no privatisation of stmt
functions
OpenMP prohibits privatisation of variables that appear in expressions
for statement functions.
This is a re-working of an old patch https://reviews.llvm.org/D93213 by
@praveen-g-ctt.
The old patch couldn't be landed because of ordering concerns. Statement
functions are rewritten during parse tree rewriting, but this was done
after resolve-directives and so some array expressions were incorrectly
identified as statement functions. For this reason **I have opted to
re-order the semantics driver so that resolve-directives is run after
parse tree rewriting**.
Closes #54677
Co-authored-by: Praveen <praveen at compilertree.com>
---
flang/lib/Semantics/resolve-directives.cpp | 8 +++++
flang/lib/Semantics/semantics.cpp | 2 +-
flang/test/Semantics/OpenMP/private03.f90 | 39 ++++++++++++++++++++++
3 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 flang/test/Semantics/OpenMP/private03.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 23a1ecbd2842d5..98cf54a3b71297 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2750,6 +2750,14 @@ void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
"Variable '%s' in ASSOCIATE cannot be in a %s clause"_err_en_US,
name.ToString(), clauseName.str());
}
+
+ if (stmtFunctionExprSymbols_.find(ultimateSymbol) !=
+ stmtFunctionExprSymbols_.end()) {
+ context_.Say(name.source,
+ "Variable '%s' in STATEMENT FUNCTION expression cannot be in a "
+ "%s clause"_err_en_US,
+ name.ToString(), clauseName.str());
+ }
}
void OmpAttributeVisitor::CheckSourceLabel(const parser::Label &label) {
diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
index 1f2980b07b3e0e..dcce1fd3b1c53a 100644
--- a/flang/lib/Semantics/semantics.cpp
+++ b/flang/lib/Semantics/semantics.cpp
@@ -642,8 +642,8 @@ bool Semantics::Perform() {
CanonicalizeAcc(context_.messages(), program_) &&
CanonicalizeOmp(context_.messages(), program_) &&
CanonicalizeCUDA(program_) &&
- CanonicalizeDirectives(context_.messages(), program_) &&
PerformStatementSemantics(context_, program_) &&
+ CanonicalizeDirectives(context_.messages(), program_) &&
ModFileWriter{context_}
.set_hermeticModuleFileOutput(hermeticModuleFileOutput_)
.WriteAll();
diff --git a/flang/test/Semantics/OpenMP/private03.f90 b/flang/test/Semantics/OpenMP/private03.f90
new file mode 100644
index 00000000000000..3ce9cf81bb73a5
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/private03.f90
@@ -0,0 +1,39 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 4.5
+! Variables that appear in expressions for statement function definitions
+! may not appear in private, firstprivate or lastprivate clauses.
+
+subroutine stmt_function(temp)
+
+ integer :: i, p, q, r
+ real :: c, f, s, v, t(10)
+ real, intent(in) :: temp
+
+ c(temp) = p * (temp - q) / r
+ f(temp) = q + (temp * r/p)
+ v(temp) = c(temp) + f(temp)/2 - s
+
+ p = 5
+ q = 32
+ r = 9
+
+ !ERROR: Variable 'p' in STATEMENT FUNCTION expression cannot be in a PRIVATE clause
+ !$omp parallel private(p)
+ s = c(temp)
+ !$omp end parallel
+
+ !ERROR: Variable 's' in STATEMENT FUNCTION expression cannot be in a FIRSTPRIVATE clause
+ !$omp parallel firstprivate(s)
+ s = s + f(temp)
+ !$omp end parallel
+
+ !ERROR: Variable 's' in STATEMENT FUNCTION expression cannot be in a LASTPRIVATE clause
+ !$omp parallel do lastprivate(s, t)
+ do i = 1, 10
+ t(i) = v(temp) + i - s
+ end do
+ !$omp end parallel do
+
+ print *, t
+
+end subroutine stmt_function
>From 9a43a1dd239fdc88822b10a7ad4871894d4412d3 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 29 Aug 2024 14:00:24 +0000
Subject: [PATCH 2/5] Don't capitalise 'statement function'
---
flang/lib/Semantics/resolve-directives.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 98cf54a3b71297..3ab5fde12558be 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2754,7 +2754,7 @@ void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
if (stmtFunctionExprSymbols_.find(ultimateSymbol) !=
stmtFunctionExprSymbols_.end()) {
context_.Say(name.source,
- "Variable '%s' in STATEMENT FUNCTION expression cannot be in a "
+ "Variable '%s' in statement function expression cannot be in a "
"%s clause"_err_en_US,
name.ToString(), clauseName.str());
}
>From d3aa6da834477f16e8d2edf959520bb4c24b78b4 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Mon, 2 Sep 2024 10:19:35 +0000
Subject: [PATCH 3/5] Fix test
---
flang/test/Semantics/OpenMP/private03.f90 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/flang/test/Semantics/OpenMP/private03.f90 b/flang/test/Semantics/OpenMP/private03.f90
index 3ce9cf81bb73a5..61f790fbb38b28 100644
--- a/flang/test/Semantics/OpenMP/private03.f90
+++ b/flang/test/Semantics/OpenMP/private03.f90
@@ -17,17 +17,17 @@ subroutine stmt_function(temp)
q = 32
r = 9
- !ERROR: Variable 'p' in STATEMENT FUNCTION expression cannot be in a PRIVATE clause
+ !ERROR: Variable 'p' in statement function expression cannot be in a PRIVATE clause
!$omp parallel private(p)
s = c(temp)
!$omp end parallel
- !ERROR: Variable 's' in STATEMENT FUNCTION expression cannot be in a FIRSTPRIVATE clause
+ !ERROR: Variable 's' in statement function expression cannot be in a FIRSTPRIVATE clause
!$omp parallel firstprivate(s)
s = s + f(temp)
!$omp end parallel
- !ERROR: Variable 's' in STATEMENT FUNCTION expression cannot be in a LASTPRIVATE clause
+ !ERROR: Variable 's' in statement function expression cannot be in a LASTPRIVATE clause
!$omp parallel do lastprivate(s, t)
do i = 1, 10
t(i) = v(temp) + i - s
>From 413e71cef4b76188aab915f96ed7b8e991711c3d Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 3 Oct 2024 13:03:09 +0000
Subject: [PATCH 4/5] Ensure that statement functions are not implicitly
privatised
---
.../lib/Lower/OpenMP/DataSharingProcessor.cpp | 12 ++---------
flang/lib/Semantics/resolve-directives.cpp | 21 ++++---------------
.../test/Lower/OpenMP/statement-function.f90 | 10 +++------
3 files changed, 9 insertions(+), 34 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 23a171c6576389..417bacf76aa07e 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -350,15 +350,6 @@ void DataSharingProcessor::collectSymbols(
/*collectSymbols=*/true,
/*collectHostAssociatedSymbols=*/true);
- // Add implicitly referenced symbols from statement functions.
- if (curScope) {
- for (const auto &sym : curScope->GetSymbols()) {
- if (sym->test(semantics::Symbol::Flag::OmpFromStmtFunction) &&
- sym->test(flag))
- allSymbols.insert(&*sym);
- }
- }
-
llvm::SetVector<const semantics::Symbol *> symbolsInNestedRegions;
collectSymbolsInNestedRegions(eval, flag, symbolsInNestedRegions);
@@ -374,7 +365,8 @@ void DataSharingProcessor::collectSymbols(
return !semantics::IsProcedure(sym) &&
!sym.GetUltimate().has<semantics::DerivedTypeDetails>() &&
!sym.GetUltimate().has<semantics::NamelistDetails>() &&
- !semantics::IsImpliedDoIndex(sym.GetUltimate());
+ !semantics::IsImpliedDoIndex(sym.GetUltimate()) &&
+ !semantics::IsStmtFunction(sym);
};
auto shouldCollectSymbol = [&](const semantics::Symbol *sym) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 3ab5fde12558be..3fe7d4d45a58e3 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -718,7 +718,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void CheckDataCopyingClause(
const parser::Name &, const Symbol &, Symbol::Flag);
void CheckAssocLoopLevel(std::int64_t level, const parser::OmpClause *clause);
- void CheckObjectInNamelistOrAssociate(
+ void CheckObjectIsPrivatisable(
const parser::Name &, const Symbol &, Symbol::Flag);
void CheckSourceLabel(const parser::Label &);
void CheckLabelContext(const parser::CharBlock, const parser::CharBlock,
@@ -2225,20 +2225,7 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
return;
}
- if (auto *stmtFunction{symbol->detailsIf<semantics::SubprogramDetails>()};
- stmtFunction && stmtFunction->stmtFunction()) {
- // Each non-dummy argument from a statement function must be handled too,
- // as if it was explicitly referenced.
- semantics::UnorderedSymbolSet symbols{
- CollectSymbols(stmtFunction->stmtFunction().value())};
- for (const auto &sym : symbols) {
- if (!IsStmtFunctionDummy(sym) && !IsObjectWithDSA(*sym)) {
- CreateImplicitSymbols(&*sym, Symbol::Flag::OmpFromStmtFunction);
- }
- }
- } else {
- CreateImplicitSymbols(symbol);
- }
+ CreateImplicitSymbols(symbol);
} // within OpenMP construct
}
@@ -2357,7 +2344,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
CheckMultipleAppearances(*name, *symbol, ompFlag);
}
if (privateDataSharingAttributeFlags.test(ompFlag)) {
- CheckObjectInNamelistOrAssociate(*name, *symbol, ompFlag);
+ CheckObjectIsPrivatisable(*name, *symbol, ompFlag);
}
if (ompFlag == Symbol::Flag::OmpAllocate) {
@@ -2729,7 +2716,7 @@ void OmpAttributeVisitor::CheckDataCopyingClause(
}
}
-void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
+void OmpAttributeVisitor::CheckObjectIsPrivatisable(
const parser::Name &name, const Symbol &symbol, Symbol::Flag ompFlag) {
const auto &ultimateSymbol{symbol.GetUltimate()};
llvm::StringRef clauseName{"PRIVATE"};
diff --git a/flang/test/Lower/OpenMP/statement-function.f90 b/flang/test/Lower/OpenMP/statement-function.f90
index 8d30450161d7d5..56601de2f4f0b8 100644
--- a/flang/test/Lower/OpenMP/statement-function.f90
+++ b/flang/test/Lower/OpenMP/statement-function.f90
@@ -1,13 +1,11 @@
-! Test privatization within OpenMP constructs containing statement functions.
+! Test statement functions are not privatised
! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
!CHECK-LABEL: func @_QPtest_implicit_use
!CHECK: %[[IEXP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_useEiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: %[[IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_useEiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK: omp.parallel private({{.*firstprivate.*}} %[[IEXP]]#0 -> %[[PRIV_IEXP:[^,]+]],
-!CHECK-SAME: {{.*firstprivate.*}} %[[IIMP]]#0 -> %[[PRIV_IIMP:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+!CHECK: omp.parallel private({{.*firstprivate.*}} %[[IEXP]]#0 -> %[[PRIV_IEXP:[^,]+]] : !fir.ref<i32>) {
!CHECK: %{{.*}}:2 = hlfir.declare %[[PRIV_IEXP]] {uniq_name = "_QFtest_implicit_useEiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK: %{{.*}}:2 = hlfir.declare %[[PRIV_IIMP]] {uniq_name = "_QFtest_implicit_useEiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
subroutine test_implicit_use()
implicit none
integer :: iexp, iimp
@@ -27,9 +25,7 @@ subroutine test_implicit_use()
!CHECK: %[[PRIV_IEXP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: %[[TEMP0:.*]] = fir.load %[[IEXP]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP0]] to %[[PRIV_IEXP]]#0 : i32, !fir.ref<i32>
-!CHECK: %[[PRIV_IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK: %[[TEMP1:.*]] = fir.load %[[IIMP]]#0 : !fir.ref<i32>
-!CHECK: hlfir.assign %[[TEMP1]] to %[[PRIV_IIMP]]#0 : i32, !fir.ref<i32>
+!CHECK-NOT: %[[PRIV_IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
subroutine test_implicit_use2()
implicit none
integer :: iexp, iimp
>From 99572e62c97023a5ec2532da34f631fddbbdf2f8 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 3 Oct 2024 15:48:15 +0000
Subject: [PATCH 5/5] Fix spelling and remove unused flag
---
flang/include/flang/Semantics/symbol.h | 2 +-
flang/lib/Semantics/resolve-directives.cpp | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index b4db6689a94271..cf0350735b5b94 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -755,7 +755,7 @@ class Symbol {
OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective,
OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction,
OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined,
- OmpImplicit, OmpFromStmtFunction);
+ OmpImplicit);
using Flags = common::EnumSet<Flag, Flag_enumSize>;
const Scope &owner() const { return *owner_; }
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 3fe7d4d45a58e3..ee2c55f41906b5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -718,7 +718,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void CheckDataCopyingClause(
const parser::Name &, const Symbol &, Symbol::Flag);
void CheckAssocLoopLevel(std::int64_t level, const parser::OmpClause *clause);
- void CheckObjectIsPrivatisable(
+ void CheckObjectIsPrivatizable(
const parser::Name &, const Symbol &, Symbol::Flag);
void CheckSourceLabel(const parser::Label &);
void CheckLabelContext(const parser::CharBlock, const parser::CharBlock,
@@ -2344,7 +2344,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
CheckMultipleAppearances(*name, *symbol, ompFlag);
}
if (privateDataSharingAttributeFlags.test(ompFlag)) {
- CheckObjectIsPrivatisable(*name, *symbol, ompFlag);
+ CheckObjectIsPrivatizable(*name, *symbol, ompFlag);
}
if (ompFlag == Symbol::Flag::OmpAllocate) {
@@ -2716,7 +2716,7 @@ void OmpAttributeVisitor::CheckDataCopyingClause(
}
}
-void OmpAttributeVisitor::CheckObjectIsPrivatisable(
+void OmpAttributeVisitor::CheckObjectIsPrivatizable(
const parser::Name &name, const Symbol &symbol, Symbol::Flag ompFlag) {
const auto &ultimateSymbol{symbol.GetUltimate()};
llvm::StringRef clauseName{"PRIVATE"};
More information about the flang-commits
mailing list