Skip to content

Commit c734d77

Browse files
tblahpraveen-g-ctt
tblah
and
praveen-g-ctt
authored
[flang][semantics][OpenMP] no privatisation of stmt functions (#106550)
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 <[email protected]>
1 parent ef8d506 commit c734d77

File tree

6 files changed

+58
-36
lines changed

6 files changed

+58
-36
lines changed

‎flang/include/flang/Semantics/symbol.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,7 @@ class Symbol {
755755
OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective,
756756
OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction,
757757
OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined,
758-
OmpImplicit, OmpFromStmtFunction);
758+
OmpImplicit);
759759
using Flags = common::EnumSet<Flag, Flag_enumSize>;
760760

761761
const Scope &owner() const { return *owner_; }

‎flang/lib/Lower/OpenMP/DataSharingProcessor.cpp‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -352,15 +352,6 @@ void DataSharingProcessor::collectSymbols(
352352
/*collectSymbols=*/true,
353353
/*collectHostAssociatedSymbols=*/true);
354354

355-
// Add implicitly referenced symbols from statement functions.
356-
if (curScope) {
357-
for (const auto &sym : curScope->GetSymbols()) {
358-
if (sym->test(semantics::Symbol::Flag::OmpFromStmtFunction) &&
359-
sym->test(flag))
360-
allSymbols.insert(&*sym);
361-
}
362-
}
363-
364355
llvm::SetVector<const semantics::Symbol *> symbolsInNestedRegions;
365356
collectSymbolsInNestedRegions(eval, flag, symbolsInNestedRegions);
366357

@@ -376,7 +367,8 @@ void DataSharingProcessor::collectSymbols(
376367
return !semantics::IsProcedure(sym) &&
377368
!sym.GetUltimate().has<semantics::DerivedTypeDetails>() &&
378369
!sym.GetUltimate().has<semantics::NamelistDetails>() &&
379-
!semantics::IsImpliedDoIndex(sym.GetUltimate());
370+
!semantics::IsImpliedDoIndex(sym.GetUltimate()) &&
371+
!semantics::IsStmtFunction(sym);
380372
};
381373

382374
auto shouldCollectSymbol = [&](const semantics::Symbol *sym) {

‎flang/lib/Semantics/resolve-directives.cpp‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
719719
void CheckDataCopyingClause(
720720
const parser::Name &, const Symbol &, Symbol::Flag);
721721
void CheckAssocLoopLevel(std::int64_t level, const parser::OmpClause *clause);
722-
void CheckObjectInNamelistOrAssociate(
722+
void CheckObjectIsPrivatizable(
723723
const parser::Name &, const Symbol &, Symbol::Flag);
724724
void CheckSourceLabel(const parser::Label &);
725725
void CheckLabelContext(const parser::CharBlock, const parser::CharBlock,
@@ -2226,20 +2226,7 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
22262226
return;
22272227
}
22282228

2229-
if (auto *stmtFunction{symbol->detailsIf<semantics::SubprogramDetails>()};
2230-
stmtFunction && stmtFunction->stmtFunction()) {
2231-
// Each non-dummy argument from a statement function must be handled too,
2232-
// as if it was explicitly referenced.
2233-
semantics::UnorderedSymbolSet symbols{
2234-
CollectSymbols(stmtFunction->stmtFunction().value())};
2235-
for (const auto &sym : symbols) {
2236-
if (!IsStmtFunctionDummy(sym) && !IsObjectWithDSA(*sym)) {
2237-
CreateImplicitSymbols(&*sym, Symbol::Flag::OmpFromStmtFunction);
2238-
}
2239-
}
2240-
} else {
2241-
CreateImplicitSymbols(symbol);
2242-
}
2229+
CreateImplicitSymbols(symbol);
22432230
} // within OpenMP construct
22442231
}
22452232

@@ -2358,7 +2345,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
23582345
CheckMultipleAppearances(*name, *symbol, ompFlag);
23592346
}
23602347
if (privateDataSharingAttributeFlags.test(ompFlag)) {
2361-
CheckObjectInNamelistOrAssociate(*name, *symbol, ompFlag);
2348+
CheckObjectIsPrivatizable(*name, *symbol, ompFlag);
23622349
}
23632350

23642351
if (ompFlag == Symbol::Flag::OmpAllocate) {
@@ -2730,7 +2717,7 @@ void OmpAttributeVisitor::CheckDataCopyingClause(
27302717
}
27312718
}
27322719

2733-
void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
2720+
void OmpAttributeVisitor::CheckObjectIsPrivatizable(
27342721
const parser::Name &name, const Symbol &symbol, Symbol::Flag ompFlag) {
27352722
const auto &ultimateSymbol{symbol.GetUltimate()};
27362723
llvm::StringRef clauseName{"PRIVATE"};
@@ -2751,6 +2738,14 @@ void OmpAttributeVisitor::CheckObjectInNamelistOrAssociate(
27512738
"Variable '%s' in ASSOCIATE cannot be in a %s clause"_err_en_US,
27522739
name.ToString(), clauseName.str());
27532740
}
2741+
2742+
if (stmtFunctionExprSymbols_.find(ultimateSymbol) !=
2743+
stmtFunctionExprSymbols_.end()) {
2744+
context_.Say(name.source,
2745+
"Variable '%s' in statement function expression cannot be in a "
2746+
"%s clause"_err_en_US,
2747+
name.ToString(), clauseName.str());
2748+
}
27542749
}
27552750

27562751
void OmpAttributeVisitor::CheckSourceLabel(const parser::Label &label) {

‎flang/lib/Semantics/semantics.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,8 +645,8 @@ bool Semantics::Perform() {
645645
CanonicalizeAcc(context_.messages(), program_) &&
646646
CanonicalizeOmp(context_.messages(), program_) &&
647647
CanonicalizeCUDA(program_) &&
648-
CanonicalizeDirectives(context_.messages(), program_) &&
649648
PerformStatementSemantics(context_, program_) &&
649+
CanonicalizeDirectives(context_.messages(), program_) &&
650650
ModFileWriter{context_}
651651
.set_hermeticModuleFileOutput(hermeticModuleFileOutput_)
652652
.WriteAll();

‎flang/test/Lower/OpenMP/statement-function.f90‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
! Test privatization within OpenMP constructs containing statement functions.
1+
! Test statement functions are not privatised
22
! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
33

44
!CHECK-LABEL: func @_QPtest_implicit_use
55
!CHECK: %[[IEXP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_useEiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
66
!CHECK: %[[IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_useEiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
7-
!CHECK: omp.parallel private({{.*firstprivate.*}} %[[IEXP]]#0 -> %[[PRIV_IEXP:[^,]+]],
8-
!CHECK-SAME: {{.*firstprivate.*}} %[[IIMP]]#0 -> %[[PRIV_IIMP:.*]] : !fir.ref<i32>, !fir.ref<i32>)
7+
!CHECK: omp.parallel private({{.*firstprivate.*}} %[[IEXP]]#0 -> %[[PRIV_IEXP:[^,]+]] : !fir.ref<i32>) {
98
!CHECK: %{{.*}}:2 = hlfir.declare %[[PRIV_IEXP]] {uniq_name = "_QFtest_implicit_useEiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
10-
!CHECK: %{{.*}}:2 = hlfir.declare %[[PRIV_IIMP]] {uniq_name = "_QFtest_implicit_useEiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
119
subroutine test_implicit_use()
1210
implicit none
1311
integer :: iexp, iimp
@@ -27,9 +25,7 @@ subroutine test_implicit_use()
2725
!CHECK: %[[PRIV_IEXP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiexp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
2826
!CHECK: %[[TEMP0:.*]] = fir.load %[[IEXP]]#0 : !fir.ref<i32>
2927
!CHECK: hlfir.assign %[[TEMP0]] to %[[PRIV_IEXP]]#0 : i32, !fir.ref<i32>
30-
!CHECK: %[[PRIV_IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
31-
!CHECK: %[[TEMP1:.*]] = fir.load %[[IIMP]]#0 : !fir.ref<i32>
32-
!CHECK: hlfir.assign %[[TEMP1]] to %[[PRIV_IIMP]]#0 : i32, !fir.ref<i32>
28+
!CHECK-NOT: %[[PRIV_IIMP:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "_QFtest_implicit_use2Eiimp"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
3329
subroutine test_implicit_use2()
3430
implicit none
3531
integer :: iexp, iimp

‎flang/test/Semantics/OpenMP/private03.f90‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
! RUN: %python %S/../test_errors.py %s %flang -fopenmp
2+
! OpenMP Version 4.5
3+
! Variables that appear in expressions for statement function definitions
4+
! may not appear in private, firstprivate or lastprivate clauses.
5+
6+
subroutine stmt_function(temp)
7+
8+
integer :: i, p, q, r
9+
real :: c, f, s, v, t(10)
10+
real, intent(in) :: temp
11+
12+
c(temp) = p * (temp - q) / r
13+
f(temp) = q + (temp * r/p)
14+
v(temp) = c(temp) + f(temp)/2 - s
15+
16+
p = 5
17+
q = 32
18+
r = 9
19+
20+
!ERROR: Variable 'p' in statement function expression cannot be in a PRIVATE clause
21+
!$omp parallel private(p)
22+
s = c(temp)
23+
!$omp end parallel
24+
25+
!ERROR: Variable 's' in statement function expression cannot be in a FIRSTPRIVATE clause
26+
!$omp parallel firstprivate(s)
27+
s = s + f(temp)
28+
!$omp end parallel
29+
30+
!ERROR: Variable 's' in statement function expression cannot be in a LASTPRIVATE clause
31+
!$omp parallel do lastprivate(s, t)
32+
do i = 1, 10
33+
t(i) = v(temp) + i - s
34+
end do
35+
!$omp end parallel do
36+
37+
print *, t
38+
39+
end subroutine stmt_function

0 commit comments

Comments
 (0)