Skip to content

Commit b10a198

Browse files
anchurajanchuraj
committed
R7: Addressed a few review comments
1 parent 6409cd9 commit b10a198

File tree

3 files changed

+52
-46
lines changed

3 files changed

+52
-46
lines changed

‎flang/include/flang/Semantics/openmp-directive-sets.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ static const OmpDirectiveSet allTeamsSet{
187187
// Directive sets for groups of multiple directives
188188
//===----------------------------------------------------------------------===//
189189

190-
// Directive sets that form Composite constructs
190+
// Composite constructs
191191
static const OmpDirectiveSet allDistributeParallelDoSet{
192192
allDistributeSet & allParallelSet & allDoSet};
193193
static const OmpDirectiveSet allDistributeParallelDoSimdSet{
@@ -292,9 +292,10 @@ static const OmpDirectiveSet workShareSet{
292292

293293
//===----------------------------------------------------------------------===//
294294
// Directive sets for parent directives that do allow/not allow a construct
295-
static const OmpDirectiveSet scanParentAllowedSet{allDoSet | allSimdSet};
296295
//===----------------------------------------------------------------------===//
297296

297+
static const OmpDirectiveSet scanParentAllowedSet{allDoSet | allSimdSet};
298+
298299
//===----------------------------------------------------------------------===//
299300
// Directive sets for allowed/not allowed nested directives
300301
//===----------------------------------------------------------------------===//

‎flang/lib/Semantics/check-omp-structure.cpp‎

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -974,36 +974,40 @@ void OmpStructureChecker::CheckDistLinear(
974974
}
975975

976976
void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
977-
const auto &beginLoopDir = std::get<parser::OmpBeginLoopDirective>(x.t);
977+
const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
978978
const auto &clauseList{std::get<parser::OmpClauseList>(beginLoopDir.t)};
979979
for (const auto &clause : clauseList.v) {
980980
if (const auto *reductionClause{
981981
std::get_if<parser::OmpClause::Reduction>(&clause.u)}) {
982-
using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
983982
const auto &maybeModifier{
984983
std::get<std::optional<ReductionModifier>>(reductionClause->v.t)};
985984
if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
986985

987986
const auto &objectList{
988987
std::get<parser::OmpObjectList>(reductionClause->v.t)};
988+
auto checkReductionSymbolInScan = [&](const parser::Name *name) {
989+
if (name->symbol) {
990+
std::string nameStr = name->symbol->name().ToString();
991+
if (GetContext().usedInScanDirective.find(nameStr) ==
992+
GetContext().usedInScanDirective.end()) {
993+
context_.Say(name->source,
994+
"List item %s must appear in 'inclusive' or "
995+
"'exclusive' clause of an "
996+
"enclosed scan directive"_err_en_US,
997+
nameStr);
998+
}
999+
}
1000+
};
9891001
for (const auto &ompObj : objectList.v) {
9901002
common::visit(
9911003
common::visitors{
9921004
[&](const parser::Designator &designator) {
9931005
if (const auto *name{semantics::getDesignatorNameIfDataRef(
9941006
designator)}) {
995-
std::string nameStr = name->symbol->name().ToString();
996-
if (GetContext().usedInScanDirective.find(nameStr) ==
997-
GetContext().usedInScanDirective.end()) {
998-
context_.Say(name->source,
999-
"List item %s must appear in 'inclusive' or "
1000-
"'exclusive' clause of an "
1001-
"enclosed scan directive"_err_en_US,
1002-
nameStr);
1003-
}
1007+
checkReductionSymbolInScan(name);
10041008
}
10051009
},
1006-
[&](const auto &name) {},
1010+
[&](const auto &name) { checkReductionSymbolInScan(&name); },
10071011
},
10081012
ompObj.u);
10091013
}
@@ -2831,24 +2835,23 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
28312835
if (CheckReductionOperators(x)) {
28322836
CheckReductionTypeList(x);
28332837
}
2834-
using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
28352838
if (const auto &maybeModifier{
28362839
std::get<std::optional<ReductionModifier>>(x.v.t)}) {
2837-
ReductionModifier modifier{*maybeModifier};
2840+
const ReductionModifier modifier{*maybeModifier};
28382841
const auto &ompObjectList{std::get<parser::OmpObjectList>(x.v.t)};
2839-
addModifiertoMap(ompObjectList, modifier);
2842+
AddModifierToMap(ompObjectList, modifier);
28402843
CheckReductionModifier(modifier);
28412844
}
28422845
}
28432846

28442847
void OmpStructureChecker::Enter(const parser::OmpClause::Inclusive &x) {
28452848
CheckAllowed(llvm::omp::Clause::OMPC_inclusive);
2846-
checkAndAddSymbolsToUsedInScanList(x.v);
2849+
CheckAndAddSymbolsToUsedInScanList(x.v);
28472850
}
28482851

28492852
void OmpStructureChecker::Enter(const parser::OmpClause::Exclusive &x) {
28502853
CheckAllowed(llvm::omp::Clause::OMPC_exclusive);
2851-
checkAndAddSymbolsToUsedInScanList(x.v);
2854+
CheckAndAddSymbolsToUsedInScanList(x.v);
28522855
}
28532856

28542857
bool OmpStructureChecker::CheckReductionOperators(
@@ -2892,8 +2895,8 @@ bool OmpStructureChecker::CheckReductionOperators(
28922895
return ok;
28932896
}
28942897

2895-
void OmpStructureChecker::addModifiertoMap(const parser::OmpObjectList &x,
2896-
parser::OmpReductionClause::ReductionModifier &modifier) {
2898+
void OmpStructureChecker::AddModifierToMap(
2899+
const parser::OmpObjectList &x, const ReductionModifier &modifier) {
28972900
for (const auto &ompObject : x.v) {
28982901
if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
28992902
if (const auto *symbol{name->symbol}) {
@@ -2903,32 +2906,35 @@ void OmpStructureChecker::addModifiertoMap(const parser::OmpObjectList &x,
29032906
}
29042907
}
29052908

2906-
void OmpStructureChecker::checkAndAddSymbolsToUsedInScanList(
2909+
void OmpStructureChecker::CheckAndAddSymbolsToUsedInScanList(
29072910
const parser::OmpObjectList &x) {
29082911
for (const auto &ompObj : x.v) {
2912+
auto checkScanSymbolInReduction = [&](const parser::Name *name) {
2913+
if (name->symbol) {
2914+
if (CurrentDirectiveIsNested()) {
2915+
std::string nameStr = name->symbol->name().ToString();
2916+
if (GetContextParent().reductionMod.find(nameStr) ==
2917+
GetContextParent().reductionMod.end()) {
2918+
2919+
context_.Say(name->source,
2920+
"List item %s must appear in 'reduction' clause "
2921+
"with the 'inscan' modifier of the parent "
2922+
"directive"_err_en_US,
2923+
nameStr);
2924+
}
2925+
GetContextParent().usedInScanDirective.insert(nameStr);
2926+
}
2927+
}
2928+
};
29092929
common::visit(
29102930
common::visitors{
29112931
[&](const parser::Designator &designator) {
29122932
if (const auto *name{
29132933
semantics::getDesignatorNameIfDataRef(designator)}) {
2914-
if (name->symbol) {
2915-
if (CurrentDirectiveIsNested()) {
2916-
std::string nameStr = name->symbol->name().ToString();
2917-
if (GetContextParent().reductionMod.find(nameStr) ==
2918-
GetContextParent().reductionMod.end()) {
2919-
2920-
context_.Say(name->source,
2921-
"List item %s must appear in 'reduction' clause "
2922-
"with the 'inscan' modifier of the parent "
2923-
"directive"_err_en_US,
2924-
nameStr);
2925-
}
2926-
GetContextParent().usedInScanDirective.insert(nameStr);
2927-
}
2928-
}
2934+
checkScanSymbolInReduction(name);
29292935
}
29302936
},
2931-
[&](const auto &name) {},
2937+
[&](const auto &name) { checkScanSymbolInReduction(&name); },
29322938
},
29332939
ompObj.u);
29342940
}
@@ -3068,10 +3074,9 @@ void OmpStructureChecker::CheckReductionTypeList(
30683074
}
30693075

30703076
void OmpStructureChecker::CheckReductionModifier(
3071-
const parser::OmpReductionClause::ReductionModifier &modifier) {
3072-
using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
3077+
const ReductionModifier &modifier) {
30733078
if (modifier == ReductionModifier::Default) {
3074-
// the default one is always ok.
3079+
// The default one is always ok.
30753080
return;
30763081
}
30773082
const DirectiveContext &dirCtx{GetContext()};

‎flang/lib/Semantics/check-omp-structure.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class OmpStructureChecker
7070
) {
7171
}
7272
using llvmOmpClause = const llvm::omp::Clause;
73+
using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
7374

7475
void Enter(const parser::OpenMPConstruct &);
7576
void Leave(const parser::OpenMPConstruct &);
@@ -227,8 +228,7 @@ class OmpStructureChecker
227228
bool CheckIntrinsicOperator(
228229
const parser::DefinedOperator::IntrinsicOperator &);
229230
void CheckReductionTypeList(const parser::OmpClause::Reduction &);
230-
void CheckReductionModifier(
231-
const parser::OmpReductionClause::ReductionModifier &);
231+
void CheckReductionModifier(const ReductionModifier &);
232232
void CheckMasterNesting(const parser::OpenMPBlockConstruct &x);
233233
void ChecksOnOrderedAsBlock();
234234
void CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &x);
@@ -249,9 +249,9 @@ class OmpStructureChecker
249249
const parser::OmpObjectList &ompObjectList);
250250
void CheckPredefinedAllocatorRestriction(
251251
const parser::CharBlock &source, const parser::Name &name);
252-
void checkAndAddSymbolsToUsedInScanList(const parser::OmpObjectList &x);
253-
void addModifiertoMap(const parser::OmpObjectList &x,
254-
parser::OmpReductionClause::ReductionModifier &modifier);
252+
void CheckAndAddSymbolsToUsedInScanList(const parser::OmpObjectList &x);
253+
void AddModifierToMap(
254+
const parser::OmpObjectList &x, const ReductionModifier &modifier);
255255
bool isPredefinedAllocator{false};
256256

257257
void CheckAllowedRequiresClause(llvmOmpClause clause);

0 commit comments

Comments
 (0)