[flang] [OpenMP] Predetermined rule for sequential loop index (flang-compiler/f18#976)

This commit implements rule:
A loop iteration variable for a sequential loop in a parallel or
task generating construct is private in the innermost such construct
that encloses the loop.

A Simple example:
```
  i = -1                    <== Scope 0
  j = -1
  !$omp parallel            <== Scope 1
  print *,i,j      <-- both are shared (Scope 0)
  !$omp parallel            <== Scope 2
  print *,i,j      <-- a) i is shared (Scope 0), j is private (Scope 2)
  !$omp do                  <== Scope 3
  do i=1, 10       <-- i is private (Scope 3)
     do j=1, 10    <-- b) j is private (Scope 2, not 3!)
     enddo
  enddo
  print *,i,j      <-- c) i is shared (Scope 0), j is private (Scope 2)
  !$omp end parallel
  print *,i,j      <-- both are shared (Scope 0)
  !$omp end parallel
  print *,i,j      <-- both are shared (Scope 0)
end
```

Ideally the above rule solves a), b), and c) but a) is left as a TODO
because it is better to handle the data-sharing attribute conflicts
along with the rules for "Predetermined DSA on Clauses".

The basic idea is when visiting the `DoConstruct` node within an OpenMP
construct, if the do-loop is not associated (like `i` loop is associated
with `!$omp do`) AND the do-loop is in the parallel/task generating
construct, resolve the loop index to be private to that innermost construct.

In the above example, `j` loop is not associated (then it is sequential) and
the innermost parallel/task generating construct that encloses the `j` loop
is the `parallel` construct marked with `<== Scope 2`, so `j` is private
to that construct. To do that, I also need to change the prototype of those
`ResolveOmp*` functions to allow specifiying the `scope` because the new
symbol for `j` should be created in Scope 2 and all the `symbol` field of
`Name j` in that `parallel` construct should be fixed, such as c).

Original-commit: flang-compiler/f18@69a845283b
Reviewed-on: https://github.com/flang-compiler/f18/pull/976
This commit is contained in:
Jinxin (Brian) Yang 2020-02-18 16:27:43 -08:00 committed by GitHub
parent 840e19eed8
commit aa9fc5bddc
7 changed files with 166 additions and 51 deletions

View File

@ -463,7 +463,7 @@ public:
// OpenMP miscellaneous flags
OmpCommonBlock, OmpReduction, OmpDeclareSimd, OmpDeclareTarget,
OmpThreadprivate, OmpDeclareReduction, OmpFlushed, OmpCriticalLock,
OmpIfSpecified, OmpNone);
OmpIfSpecified, OmpNone, OmpPreDetermined);
using Flags = common::EnumSet<Flag, Flag_enumSize>;
const Scope &owner() const { return *owner_; }

View File

@ -88,6 +88,8 @@ static constexpr OmpDirectiveSet simdSet{
OmpDirective::TASKLOOP_SIMD,
OmpDirective::TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD,
OmpDirective::TEAMS_DISTRIBUTE_SIMD};
static constexpr OmpDirectiveSet taskGeneratingSet{
OmpDirectiveSet{OmpDirective::TASK} | taskloopSet};
class OmpStructureChecker : public virtual BaseChecker {
public:

View File

@ -1167,6 +1167,7 @@ public:
void Post(const parser::OmpBeginLoopDirective &) {
GetContext().withinConstruct = true;
}
bool Pre(const parser::DoConstruct &);
bool Pre(const parser::OpenMPSectionsConstruct &);
void Post(const parser::OpenMPSectionsConstruct &) { PopContext(); }
@ -1224,27 +1225,34 @@ private:
void SetContextDirectiveEnum(OmpDirective dir) {
GetContext().directive = dir;
}
const Scope &currScope() { return GetContext().scope; }
Scope &currScope() { return GetContext().scope; }
void SetContextDefaultDSA(Symbol::Flag flag) {
GetContext().defaultDSA = flag;
}
void AddToContextObjectWithDSA(
const Symbol &symbol, Symbol::Flag flag, OmpContext &context) {
context.objectWithDSA.emplace(&symbol, flag);
}
void AddToContextObjectWithDSA(const Symbol &symbol, Symbol::Flag flag) {
GetContext().objectWithDSA.emplace(&symbol, flag);
AddToContextObjectWithDSA(symbol, flag, GetContext());
}
bool IsObjectWithDSA(const Symbol &symbol) {
auto it{GetContext().objectWithDSA.find(&symbol)};
return it != GetContext().objectWithDSA.end();
}
void SetContextAssociatedLoopLevel(std::size_t level) {
GetContext().associatedLoopLevel = level;
}
std::size_t GetAssociatedLoopLevelFromClauses(const parser::OmpClauseList &);
Symbol &MakeAssocSymbol(const SourceName &name, Symbol &prev) {
const auto pair{
GetContext().scope.try_emplace(name, Attrs{}, HostAssocDetails{prev})};
Symbol &MakeAssocSymbol(const SourceName &name, Symbol &prev, Scope &scope) {
const auto pair{scope.try_emplace(name, Attrs{}, HostAssocDetails{prev})};
return *pair.first->second;
}
Symbol &MakeAssocSymbol(const SourceName &name, Symbol &prev) {
return MakeAssocSymbol(name, prev, currScope());
}
static const parser::Name *GetDesignatorNameIfDataRef(
const parser::Designator &designator) {
@ -1277,14 +1285,17 @@ private:
const parser::ExecutionPartConstruct &);
// Predetermined DSA rules
void PrivatizeAssociatedLoopIndex(const parser::OpenMPLoopConstruct &);
const parser::Name &GetLoopIndex(const parser::DoConstruct &);
void ResolveSeqLoopIndexInParallelOrTaskConstruct(const parser::Name &);
void ResolveOmpObjectList(const parser::OmpObjectList &, Symbol::Flag);
void ResolveOmpObject(const parser::OmpObject &, Symbol::Flag);
Symbol *ResolveOmp(const parser::Name &, Symbol::Flag);
Symbol *ResolveOmp(Symbol &, Symbol::Flag);
Symbol *ResolveOmp(const parser::Name &, Symbol::Flag, Scope &);
Symbol *ResolveOmp(Symbol &, Symbol::Flag, Scope &);
Symbol *ResolveOmpCommonBlockName(const parser::Name *);
Symbol *DeclarePrivateAccessEntity(const parser::Name &, Symbol::Flag);
Symbol *DeclarePrivateAccessEntity(Symbol &, Symbol::Flag);
Symbol *DeclarePrivateAccessEntity(
const parser::Name &, Symbol::Flag, Scope &);
Symbol *DeclarePrivateAccessEntity(Symbol &, Symbol::Flag, Scope &);
Symbol *DeclareOrMarkOtherAccessEntity(const parser::Name &, Symbol::Flag);
Symbol *DeclareOrMarkOtherAccessEntity(Symbol &, Symbol::Flag);
void CheckMultipleAppearances(
@ -1380,6 +1391,7 @@ private:
void FinishSpecificationParts(const ProgramTree &);
void FinishDerivedTypeInstantiation(Scope &);
void ResolveExecutionParts(const ProgramTree &);
void ResolveOmpParts(const parser::ProgramUnit &);
};
// ImplicitRules implementation
@ -5766,7 +5778,7 @@ bool ResolveNamesVisitor::Pre(const parser::ProgramUnit &x) {
ResolveSpecificationParts(root);
FinishSpecificationParts(root);
ResolveExecutionParts(root);
OmpAttributeVisitor{context(), *this}.Walk(x);
ResolveOmpParts(x);
return false;
}
@ -6072,6 +6084,53 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
return true;
}
const parser::Name &OmpAttributeVisitor::GetLoopIndex(
const parser::DoConstruct &x) {
auto &loopControl{x.GetLoopControl().value()};
using Bounds = parser::LoopControl::Bounds;
const Bounds &bounds{std::get<Bounds>(loopControl.u)};
return bounds.name.thing;
}
void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
const parser::Name &iv) {
auto targetIt{ompContext_.rbegin()};
for (;; ++targetIt) {
if (targetIt == ompContext_.rend()) {
return;
}
if (parallelSet.test(targetIt->directive) ||
taskGeneratingSet.test(targetIt->directive)) {
break;
}
}
if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
targetIt++;
symbol->set(Symbol::Flag::OmpPreDetermined);
iv.symbol = symbol; // adjust the symbol within region
for (auto it{ompContext_.rbegin()}; it != targetIt; ++it) {
AddToContextObjectWithDSA(*symbol, Symbol::Flag::OmpPrivate, *it);
}
}
}
// 2.15.1.1 Data-sharing Attribute Rules - Predetermined
// - A loop iteration variable for a sequential loop in a parallel
// or task generating construct is private in the innermost such
// construct that encloses the loop
bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
if (!ompContext_.empty() && GetContext().withinConstruct) {
if (const auto &iv{GetLoopIndex(x)}; iv.symbol) {
if (!iv.symbol->test(Symbol::Flag::OmpPreDetermined)) {
ResolveSeqLoopIndexInParallelOrTaskConstruct(iv);
} else {
// TODO: conflict checks with explicitly determined DSA
}
}
}
return true;
}
const parser::DoConstruct *OmpAttributeVisitor::GetDoConstructIf(
const parser::ExecutionPartConstruct &x) {
if (auto *y{std::get_if<parser::ExecutableConstruct>(&x.u)}) {
@ -6137,11 +6196,9 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndex(
auto &outer{std::get<std::optional<parser::DoConstruct>>(x.t)};
for (const parser::DoConstruct *loop{&*outer}; loop && level > 0; --level) {
// go through all the nested do-loops and resolve index variables
auto &loopControl{loop->GetLoopControl().value()};
using Bounds = parser::LoopControl::Bounds;
const Bounds &bounds{std::get<Bounds>(loopControl.u)};
const parser::Name &iv{bounds.name.thing};
if (auto *symbol{ResolveOmp(iv, ivDSA)}) {
const parser::Name &iv{GetLoopIndex(*loop)};
if (auto *symbol{ResolveOmp(iv, ivDSA, currScope())}) {
symbol->set(Symbol::Flag::OmpPreDetermined);
iv.symbol = symbol; // adjust the symbol within region
AddToContextObjectWithDSA(*symbol, ivDSA);
}
@ -6207,7 +6264,7 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
// predetermined, explicitly determined, and implicitly
// determined data-sharing attributes (2.15.1.1).
if (Symbol * found{currScope().FindSymbol(name.source)}) {
if (IsObjectWithDSA(*found)) {
if (symbol != found) {
name.symbol = found; // adjust the symbol within region
} else if (GetContext().defaultDSA == Symbol::Flag::OmpNone) {
context_.Say(name.source,
@ -6250,7 +6307,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
common::visitors{
[&](const parser::Designator &designator) {
if (const auto *name{GetDesignatorNameIfDataRef(designator)}) {
if (auto *symbol{ResolveOmp(*name, ompFlag)}) {
if (auto *symbol{ResolveOmp(*name, ompFlag, currScope())}) {
AddToContextObjectWithDSA(*symbol, ompFlag);
if (dataSharingAttributeFlags.test(ompFlag)) {
CheckMultipleAppearances(*name, *symbol, ompFlag);
@ -6288,7 +6345,8 @@ void OmpAttributeVisitor::ResolveOmpObject(
for (const Symbol &object :
symbol->get<CommonBlockDetails>().objects()) {
Symbol &mutableObject{const_cast<Symbol &>(object)};
if (auto *resolvedObject{ResolveOmp(mutableObject, ompFlag)}) {
if (auto *resolvedObject{
ResolveOmp(mutableObject, ompFlag, currScope())}) {
AddToContextObjectWithDSA(*resolvedObject, ompFlag);
}
}
@ -6303,35 +6361,36 @@ void OmpAttributeVisitor::ResolveOmpObject(
}
Symbol *OmpAttributeVisitor::ResolveOmp(
const parser::Name &name, Symbol::Flag ompFlag) {
const parser::Name &name, Symbol::Flag ompFlag, Scope &scope) {
if (ompFlagsRequireNewSymbol.test(ompFlag)) {
return DeclarePrivateAccessEntity(name, ompFlag);
return DeclarePrivateAccessEntity(name, ompFlag, scope);
} else {
return DeclareOrMarkOtherAccessEntity(name, ompFlag);
}
}
Symbol *OmpAttributeVisitor::ResolveOmp(Symbol &symbol, Symbol::Flag ompFlag) {
Symbol *OmpAttributeVisitor::ResolveOmp(
Symbol &symbol, Symbol::Flag ompFlag, Scope &scope) {
if (ompFlagsRequireNewSymbol.test(ompFlag)) {
return DeclarePrivateAccessEntity(symbol, ompFlag);
return DeclarePrivateAccessEntity(symbol, ompFlag, scope);
} else {
return DeclareOrMarkOtherAccessEntity(symbol, ompFlag);
}
}
Symbol *OmpAttributeVisitor::DeclarePrivateAccessEntity(
const parser::Name &name, Symbol::Flag ompFlag) {
const parser::Name &name, Symbol::Flag ompFlag, Scope &scope) {
if (!name.symbol) {
return nullptr; // not resolved by Name Resolution step, do nothing
}
name.symbol = DeclarePrivateAccessEntity(*name.symbol, ompFlag);
name.symbol = DeclarePrivateAccessEntity(*name.symbol, ompFlag, scope);
return name.symbol;
}
Symbol *OmpAttributeVisitor::DeclarePrivateAccessEntity(
Symbol &object, Symbol::Flag ompFlag) {
Symbol &object, Symbol::Flag ompFlag, Scope &scope) {
if (object.owner() != currScope()) {
auto &symbol{MakeAssocSymbol(object.name(), object)};
auto &symbol{MakeAssocSymbol(object.name(), object, scope)};
symbol.set(ompFlag);
return &symbol;
} else {
@ -6455,6 +6514,18 @@ void ResolveNamesVisitor::ResolveExecutionParts(const ProgramTree &node) {
}
}
void ResolveNamesVisitor::ResolveOmpParts(const parser::ProgramUnit &node) {
OmpAttributeVisitor{context(), *this}.Walk(node);
if (!context().AnyFatalError()) {
// The data-sharing attribute of the loop iteration variable for a
// sequential loop (2.15.1.1) can only be determined when visiting
// the corresponding DoConstruct, a second walk is to adjust the
// symbols for all the data-refs of that loop iteration variable
// prior to the DoConstruct.
OmpAttributeVisitor{context(), *this}.Walk(node);
}
}
void ResolveNamesVisitor::Post(const parser::Program &) {
// ensure that all temps were deallocated
CHECK(!attrs_);

View File

@ -45,7 +45,7 @@ program mm
!DEF: /mm/c (Implicit) ObjectEntity REAL(4)
c = 2.0
!$omp parallel do private(a,t,/c/) shared(c)
!DEF: /mm/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /mm/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,10
!DEF: /mm/Block1/a (OmpPrivate) HostAssoc REAL(4)
!REF: /mm/b

View File

@ -12,7 +12,7 @@
!DEF: /MainProgram1/Block1/a (OmpPrivate) HostAssoc REAL(8)
a = 2.
!$omp do private(a)
!DEF: /MainProgram1/Block1/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /MainProgram1/Block1/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,10
!DEF: /MainProgram1/Block1/Block1/a (OmpPrivate) HostAssoc REAL(8)
a = 1.

View File

@ -8,7 +8,7 @@
!DEF: /MainProgram1/a (Implicit) ObjectEntity REAL(4)
a = 1.
!$omp parallel do firstprivate(a) lastprivate(a)
!DEF: /MainProgram1/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /MainProgram1/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,10
!DEF: /MainProgram1/Block1/a (OmpFirstPrivate, OmpLastPrivate) HostAssoc REAL(4)
a = 2.

View File

@ -8,6 +8,9 @@
! increment of the associated do-loop.
! c) The loop iteration variables in the associated do-loops of a simd
! construct with multiple associated do-loops are lastprivate.
! d) A loop iteration variable for a sequential loop in a parallel or task
! generating construct is private in the innermost such construct that
! encloses the loop.
! - TBD
! All the tests assume that the do-loops association for collapse/ordered
@ -28,16 +31,16 @@ subroutine test_do
!REF: /test_do/i
i = 99
!$omp do collapse(2)
!DEF: /test_do/Block1/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /test_do/Block1/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
!DEF: /test_do/Block1/Block1/j (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /test_do/Block1/Block1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do j=6,10
!REF: /test_do/a
a(1,1,1) = 0.
!REF: /test_do/k
!DEF: /test_do/Block1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_do/a
!REF: /test_do/k
!REF: /test_do/Block1/k
!REF: /test_do/Block1/Block1/j
!REF: /test_do/Block1/Block1/i
a(k,j,i) = 1.
@ -58,13 +61,13 @@ subroutine test_pardo
!DEF: /test_pardo/k ObjectEntity INTEGER(4)
integer i, j, k
!$omp parallel do collapse(2) private(k) ordered(2)
!DEF: /test_pardo/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /test_pardo/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
!DEF: /test_pardo/Block1/j (OmpPrivate) HostAssoc INTEGER(4)
do j=6,10
!DEF: /test_pardo/Block1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do j=6,10
!REF: /test_pardo/a
a(1,1,1) = 0.
!DEF: /test_pardo/Block1/k (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /test_pardo/Block1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_pardo/a
!REF: /test_pardo/Block1/k
@ -86,9 +89,9 @@ subroutine test_taskloop
!DEF: /test_taskloop/j ObjectEntity INTEGER(4)
integer i, j
!$omp taskloop private(j)
!DEF: /test_taskloop/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /test_taskloop/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
!DEF: /test_taskloop/Block1/j (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /test_taskloop/Block1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
!REF: /test_taskloop/Block1/i
do j=1,i
!REF: /test_taskloop/a
@ -129,13 +132,13 @@ subroutine dotprod (b, c, n, block_size, num_teams, block_threads)
!$omp target map(to:b,c) map(tofrom:sum)
!$omp teams num_teams(num_teams) thread_limit(block_threads) reduction(+:sum)
!$omp distribute
!DEF: /dotprod/Block1/Block1/Block1/i0 (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /dotprod/Block1/Block1/Block1/i0 (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
!REF: /dotprod/n
!REF: /dotprod/block_size
do i0=1,n,block_size
!$omp parallel do reduction(+:sum)
!DEF: /dotprod/Block1/Block1/Block1/Block1/i (OmpPrivate) HostAssoc INTEGER(4)
!REF: /dotprod/i0
!DEF: /dotprod/Block1/Block1/Block1/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
!REF: /dotprod/Block1/Block1/Block1/i0
!DEF: /dotprod/min INTRINSIC (Function) ProcEntity
!REF: /dotprod/block_size
!REF: /dotprod/n
@ -165,15 +168,15 @@ subroutine test_simd
!DEF: /test_simd/k ObjectEntity INTEGER(4)
integer i, j, k
!$omp parallel do simd
!DEF: /test_simd/Block1/i (OmpLinear) HostAssoc INTEGER(4)
!DEF: /test_simd/Block1/i (OmpLinear, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
!REF: /test_simd/j
!DEF: /test_simd/Block1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do j=6,10
!REF: /test_simd/k
!DEF: /test_simd/Block1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_simd/a
!REF: /test_simd/k
!REF: /test_simd/j
!REF: /test_simd/Block1/k
!REF: /test_simd/Block1/j
!REF: /test_simd/Block1/i
a(k,j,i) = 3.14
end do
@ -192,11 +195,11 @@ subroutine test_simd_multi
!DEF: /test_simd_multi/k ObjectEntity INTEGER(4)
integer i, j, k
!$omp parallel do simd collapse(3)
!DEF: /test_simd_multi/Block1/i (OmpLastPrivate) HostAssoc INTEGER(4)
!DEF: /test_simd_multi/Block1/i (OmpLastPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
!DEF: /test_simd_multi/Block1/j (OmpLastPrivate) HostAssoc INTEGER(4)
!DEF: /test_simd_multi/Block1/j (OmpLastPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do j=6,10
!DEF: /test_simd_multi/Block1/k (OmpLastPrivate) HostAssoc INTEGER(4)
!DEF: /test_simd_multi/Block1/k (OmpLastPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_simd_multi/a
!REF: /test_simd_multi/Block1/k
@ -207,3 +210,42 @@ subroutine test_simd_multi
end do
end do
end subroutine test_simd_multi
! Rule d)
!DEF: /test_seq_loop (Subroutine) Subprogram
subroutine test_seq_loop
implicit none
!DEF: /test_seq_loop/i ObjectEntity INTEGER(4)
!DEF: /test_seq_loop/j ObjectEntity INTEGER(4)
integer i, j
!REF: /test_seq_loop/i
i = -1
!REF: /test_seq_loop/j
j = -1
!$omp parallel
!REF: /test_seq_loop/i
!REF: /test_seq_loop/j
print *, i, j
!$omp parallel
!REF: /test_seq_loop/i
!DEF: /test_seq_loop/Block1/Block1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
print *, i, j
!$omp do
!DEF: /test_seq_loop/Block1/Block1/Block1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,10
!REF: /test_seq_loop/Block1/Block1/j
do j=1,10
end do
end do
!REF: /test_seq_loop/i
!REF: /test_seq_loop/Block1/Block1/j
print *, i, j
!$omp end parallel
!REF: /test_seq_loop/i
!REF: /test_seq_loop/j
print *, i, j
!$omp end parallel
!REF: /test_seq_loop/i
!REF: /test_seq_loop/j
print *, i, j
end subroutine test_seq_loop