Skip to content

Commit

Permalink
[flang][openacc] Avoid privatizing symbols during semantics (#69506)
Browse files Browse the repository at this point in the history
During flang handling of semantics of OpenACC private/firstprivate/
reduction clauses (including the implicitly private loop IV), a new
scoped symbol was being created. This could lead to ambiguity in the
lowered FIR - aka having multiple fir.declare for the same symbol.
Because lowering of OpenACC does not materialize the meaning of the
private clauses (by actually creating a scoped local symbol), it does
not make sense to create a new symbol in semantics either.

I updated the acc-symbols01.f90 test to reflect this updated approach.
Technically, the test could be removed, but it made sense to keep in
place to highlight this intentional decision.
  • Loading branch information
razvanlupusoru authored Oct 18, 2023
1 parent 077d89f commit 362b115
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 37 deletions.
41 changes: 8 additions & 33 deletions flang/lib/Semantics/resolve-directives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,6 @@ class AccAttributeVisitor : DirectiveAttributeVisitor<llvm::acc::Directive> {
Symbol::Flag::AccCopyIn, Symbol::Flag::AccCopyOut,
Symbol::Flag::AccDelete, Symbol::Flag::AccPresent};

Symbol::Flags accFlagsRequireNewSymbol{Symbol::Flag::AccPrivate,
Symbol::Flag::AccFirstPrivate, Symbol::Flag::AccReduction};

Symbol::Flags accDataMvtFlags{
Symbol::Flag::AccDevice, Symbol::Flag::AccHost, Symbol::Flag::AccSelf};

Expand All @@ -266,7 +263,7 @@ class AccAttributeVisitor : DirectiveAttributeVisitor<llvm::acc::Directive> {
Symbol::Flag::AccDevicePtr, Symbol::Flag::AccDeviceResident,
Symbol::Flag::AccLink, Symbol::Flag::AccPresent};

void PrivatizeAssociatedLoopIndex(const parser::OpenACCLoopConstruct &);
void CheckAssociatedLoopIndex(const parser::OpenACCLoopConstruct &);
void ResolveAccObjectList(const parser::AccObjectList &, Symbol::Flag);
void ResolveAccObject(const parser::AccObject &, Symbol::Flag);
Symbol *ResolveAcc(const parser::Name &, Symbol::Flag, Scope &);
Expand Down Expand Up @@ -877,7 +874,7 @@ bool AccAttributeVisitor::Pre(const parser::OpenACCLoopConstruct &x) {
}
ClearDataSharingAttributeObjects();
SetContextAssociatedLoopLevel(GetAssociatedLoopLevelFromClauses(clauseList));
PrivatizeAssociatedLoopIndex(x);
CheckAssociatedLoopIndex(x);
return true;
}

Expand Down Expand Up @@ -1141,13 +1138,12 @@ std::int64_t AccAttributeVisitor::GetAssociatedLoopLevelFromClauses(
return 1; // default is outermost loop
}

void AccAttributeVisitor::PrivatizeAssociatedLoopIndex(
void AccAttributeVisitor::CheckAssociatedLoopIndex(
const parser::OpenACCLoopConstruct &x) {
std::int64_t level{GetContext().associatedLoopLevel};
if (level <= 0) { // collpase value was negative or 0
if (level <= 0) { // collapse value was negative or 0
return;
}
Symbol::Flag ivDSA{Symbol::Flag::AccPrivate};

const auto getNextDoConstruct =
[this](const parser::Block &block) -> const parser::DoConstruct * {
Expand All @@ -1166,16 +1162,8 @@ void AccAttributeVisitor::PrivatizeAssociatedLoopIndex(

const 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
const parser::Name *iv{GetLoopIndex(*loop)};
if (iv) {
if (auto *symbol{ResolveAcc(*iv, ivDSA, currScope())}) {
symbol->set(Symbol::Flag::AccPreDetermined);
iv->symbol = symbol; // adjust the symbol within region
AddToContextObjectWithDSA(*symbol, ivDSA);
}
}

// Go through all nested loops to ensure index variable exists.
GetLoopIndex(*loop);
const auto &block{std::get<parser::Block>(loop->t)};
loop = getNextDoConstruct(block);
}
Expand Down Expand Up @@ -1328,20 +1316,12 @@ void AccAttributeVisitor::ResolveAccObject(

Symbol *AccAttributeVisitor::ResolveAcc(
const parser::Name &name, Symbol::Flag accFlag, Scope &scope) {
if (accFlagsRequireNewSymbol.test(accFlag)) {
return DeclarePrivateAccessEntity(name, accFlag, scope);
} else {
return DeclareOrMarkOtherAccessEntity(name, accFlag);
}
return DeclareOrMarkOtherAccessEntity(name, accFlag);
}

Symbol *AccAttributeVisitor::ResolveAcc(
Symbol &symbol, Symbol::Flag accFlag, Scope &scope) {
if (accFlagsRequireNewSymbol.test(accFlag)) {
return DeclarePrivateAccessEntity(symbol, accFlag, scope);
} else {
return DeclareOrMarkOtherAccessEntity(symbol, accFlag);
}
return DeclareOrMarkOtherAccessEntity(symbol, accFlag);
}

Symbol *AccAttributeVisitor::DeclareOrMarkOtherAccessEntity(
Expand Down Expand Up @@ -1374,11 +1354,6 @@ static bool WithMultipleAppearancesAccException(
void AccAttributeVisitor::CheckMultipleAppearances(
const parser::Name &name, const Symbol &symbol, Symbol::Flag accFlag) {
const auto *target{&symbol};
if (accFlagsRequireNewSymbol.test(accFlag)) {
if (const auto *details{symbol.detailsIf<HostAssocDetails>()}) {
target = &details->symbol();
}
}
if (HasDataSharingAttributeObject(*target) &&
!WithMultipleAppearancesAccException(symbol, accFlag)) {
context_.Say(name.source,
Expand Down
8 changes: 4 additions & 4 deletions flang/test/Semantics/OpenACC/acc-symbols01.f90
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ program mm
b = 2
!$acc parallel present(c) firstprivate(b) private(a)
!$acc loop
!DEF: /mm/OtherConstruct1/i (AccPrivate, AccPreDetermined) HostAssoc INTEGER(4)
!REF: /mm/i
do i=1,10
!DEF: /mm/OtherConstruct1/a (AccPrivate) HostAssoc INTEGER(4)
!REF: /mm/OtherConstruct1/i
!DEF: /mm/OtherConstruct1/b (AccFirstPrivate) HostAssoc INTEGER(4)
!REF: /mm/a
!REF: /mm/i
!REF: /mm/b
a(i) = b(i)
end do
!$acc end parallel
Expand Down

0 comments on commit 362b115

Please sign in to comment.