Skip to content

Commit 2691fc1

Browse files
author
Ferran Toda
committed
Address reviews, add more tests and fix an issue
1 parent a898938 commit 2691fc1

13 files changed

+302
-53
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1054,7 +1054,7 @@ Link make(const parser::OmpClause::Link &inp,
10541054

10551055
LoopRange make(const parser::OmpClause::Looprange &inp,
10561056
semantics::SemanticsContext &semaCtx) {
1057-
TODO_NOLOC("looprange clause");
1057+
llvm_unreachable("Unimplemented: looprange");
10581058
}
10591059

10601060
Map make(const parser::OmpClause::Map &inp,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3470,7 +3470,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
34703470
break;
34713471
case llvm::omp::Directive::OMPD_tile:
34723472
genTileOp(converter, symTable, stmtCtx, semaCtx, eval, loc, queue, item);
3473-
case llvm::omp::Directive::OMPD_fuse:
3473+
case llvm::omp::Directive::OMPD_fuse: {
34743474
unsigned version = semaCtx.langOptions().OpenMPVersion;
34753475
if (!semaCtx.langOptions().OpenMPSimd)
34763476
TODO(loc, "Unhandled loop directive (" +

‎flang/lib/Semantics/canonicalize-omp.cpp‎

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -172,81 +172,85 @@ class CanonicalizationOfOmp {
172172
if (auto *endDir{
173173
GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
174174
auto &endDirName = endDir->DirName();
175-
if (endDirName.v == llvm::omp::Directive::OMPD_fuse)
176-
endFuseNeeded = false;
177-
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
178-
std::move(*endDir);
179-
nextIt = block.erase(nextIt);
175+
if (endDirName.v != llvm::omp::Directive::OMPD_fuse) {
176+
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
177+
std::move(*endDir);
178+
nextIt = block.erase(nextIt);
179+
}
180180
}
181181
}
182182
} else {
183183
messages_.Say(beginName.source,
184184
"DO loop after the %s directive must have loop control"_err_en_US,
185185
parser::ToUpperCaseLetters(beginName.source.ToString()));
186-
endFuseNeeded = false;
187186
}
188187
} else if (auto *ompLoopCons{
189188
GetOmpIf<parser::OpenMPLoopConstruct>(*nextIt)}) {
190-
// We should allow UNROLL and TILE constructs to be inserted between an
191-
// OpenMP Loop Construct and the DO loop itself
189+
// We should allow loop transformation constructs to be inserted between
190+
// an OpenMP Loop Construct and the DO loop itself
192191
auto &nestedBeginDirective = ompLoopCons->BeginDir();
193192
auto &nestedBeginName = nestedBeginDirective.DirName();
194193
if (llvm::omp::loopTransformationSet.test(nestedBeginName.v)) {
195194
if (nestedBeginName.v == llvm::omp::Directive::OMPD_unroll &&
196195
llvm::omp::loopTransformationSet.test(beginName.v)) {
197-
// if a loop has been unrolled, the user can not then tile that loop
198-
// as it has been unrolled
196+
// if a loop has been unrolled, the user can not then transform that
197+
// loop as it has been unrolled
199198
const parser::OmpClauseList &unrollClauseList{
200199
nestedBeginDirective.Clauses()};
201200
if (unrollClauseList.v.empty()) {
202201
// if the clause list is empty for an unroll construct, we assume
203202
// the loop is being fully unrolled
204203
transformUnrollError(beginName, messages_);
205-
endFuseNeeded = false;
206204
} else {
207205
// parse the clauses for the unroll directive to find the full
208206
// clause
209207
for (auto &clause : unrollClauseList.v) {
210208
if (clause.Id() == llvm::omp::OMPC_full) {
211209
transformUnrollError(beginName, messages_);
212-
endFuseNeeded = false;
213210
}
214211
}
215212
}
216213
}
217-
// iterate through the remaining block items to find the end directive
218-
// for the unroll/tile directive.
219-
parser::Block::iterator endIt;
220-
endIt = nextIt;
221-
while (endIt != block.end()) {
214+
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
215+
auto &loopConsList =
216+
std::get<std::list<parser::NestedConstruct>>(x.t);
217+
loopConsList.push_back(parser::NestedConstruct{
218+
common::Indirection{std::move(*ompLoopCons)}});
219+
nextIt = block.erase(nextIt);
220+
// check the following block item to find the end directive
221+
// for the loop transform directive.
222+
if (nextIt != block.end()) {
222223
if (auto *endDir{
223-
GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
224+
GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
224225
auto &endDirName = endDir->DirName();
225-
if (endDirName.v == beginName.v) {
226-
if (endDirName.v == llvm::omp::Directive::OMPD_fuse)
227-
endFuseNeeded = false;
226+
if (endDirName.v == beginName.v &&
227+
endDirName.v != llvm::omp::Directive::OMPD_fuse) {
228228
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
229229
std::move(*endDir);
230-
endIt = block.erase(endIt);
231-
continue;
230+
nextIt = block.erase(nextIt);
232231
}
233232
}
234-
++endIt;
235233
}
236-
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
237-
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
238-
loopConsList.push_back(parser::NestedConstruct{
239-
common::Indirection{std::move(*ompLoopCons)}});
240-
nextIt = block.erase(nextIt);
241234
} else {
242235
messages_.Say(nestedBeginName.source,
243236
"Only Loop Transformation Constructs or Loop Nests can be nested within Loop Constructs"_err_en_US,
244237
parser::ToUpperCaseLetters(nestedBeginName.source.ToString()));
245-
endFuseNeeded = false;
246238
}
247239
} else {
248240
missingDoConstruct(beginName, messages_);
249-
endFuseNeeded = false;
241+
}
242+
243+
if (endFuseNeeded && nextIt != block.end()) {
244+
if (auto *endDir{
245+
GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
246+
auto &endDirName = endDir->DirName();
247+
if (endDirName.v == llvm::omp::Directive::OMPD_fuse) {
248+
endFuseNeeded = false;
249+
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
250+
std::move(*endDir);
251+
nextIt = block.erase(nextIt);
252+
}
253+
}
250254
}
251255
if (endFuseNeeded)
252256
continue;

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

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
306306
}
307307
if (beginName.v == llvm::omp::Directive::OMPD_fuse) {
308308
CheckLooprangeBounds(x);
309-
}
309+
} else
310+
CheckNestedFuse(x);
310311
}
311312

312313
const parser::Name OmpStructureChecker::GetLoopIndex(
@@ -421,7 +422,7 @@ void OmpStructureChecker::CheckDistLinear(
421422
// clauses.
422423
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
423424
for (auto &loopCons : loopConsList) {
424-
std::int64_t collapseVal_ = collapseVal;
425+
std::int64_t curCollapseVal{collapseVal};
425426
if (const auto &loopConstruct{
426427
std::get_if<parser::DoConstruct>(&loopCons)}) {
427428
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
@@ -431,8 +432,8 @@ void OmpStructureChecker::CheckDistLinear(
431432
// Remove the symbol from the collected set
432433
indexVars.erase(&itrVal.symbol->GetUltimate());
433434
}
434-
collapseVal_--;
435-
if (collapseVal_ == 0) {
435+
curCollapseVal--;
436+
if (curCollapseVal == 0) {
436437
break;
437438
}
438439
}
@@ -466,7 +467,8 @@ void OmpStructureChecker::CheckLooprangeBounds(
466467
if (const auto count{GetIntValue(std::get<1>((lrClause->v).t))}) {
467468
auto &loopConsList =
468469
std::get<std::list<parser::NestedConstruct>>(x.t);
469-
if (loopConsList.size() < (unsigned)(*first + *count - 1)) {
470+
if (*first > 0 && *count > 0 &&
471+
loopConsList.size() < (unsigned)(*first + *count - 1)) {
470472
context_.Say(clause.source,
471473
"The loop range indicated in the %s clause"
472474
" must not be out of the bounds of the Loop Sequence"
@@ -481,6 +483,38 @@ void OmpStructureChecker::CheckLooprangeBounds(
481483
}
482484
}
483485

486+
void OmpStructureChecker::CheckNestedFuse(
487+
const parser::OpenMPLoopConstruct &x) {
488+
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
489+
for (auto &loopCons : loopConsList) {
490+
if (const auto &ompConstruct{
491+
std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
492+
&loopCons)}) {
493+
const parser::OmpClauseList &clauseList =
494+
ompConstruct->value().BeginDir().Clauses();
495+
if (!clauseList.v.empty()) {
496+
for (auto &clause : clauseList.v) {
497+
if (const auto *lrClause{
498+
std::get_if<parser::OmpClause::Looprange>(&clause.u)}) {
499+
if (const auto count{GetIntValue(std::get<1>((lrClause->v).t))}) {
500+
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(
501+
ompConstruct->value().t);
502+
if (loopConsList.size() > (unsigned)(*count)) {
503+
context_.Say(x.BeginDir().DirName().source,
504+
"The loop sequence following the %s construct"
505+
" must be fully fused first."_err_en_US,
506+
parser::ToUpperCaseLetters(
507+
x.BeginDir().DirName().source.ToString()));
508+
}
509+
}
510+
return;
511+
}
512+
}
513+
}
514+
}
515+
}
516+
}
517+
484518
void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
485519
const parser::OmpClauseList &clauseList{x.BeginDir().Clauses()};
486520

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
314314
void CheckAtomicUpdate(const parser::OpenMPAtomicConstruct &x);
315315

316316
void CheckLooprangeBounds(const parser::OpenMPLoopConstruct &x);
317+
void CheckNestedFuse(const parser::OpenMPLoopConstruct &x);
317318
void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
318319
void CheckSIMDNest(const parser::OpenMPConstruct &x);
319320
void CheckTargetNest(const parser::OpenMPConstruct &x);

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,12 +2019,12 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
20192019

20202020
if (beginName.v == llvm::omp::Directive::OMPD_do) {
20212021
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
2022-
if (loopConsList.size() == 1) { //a OMPD_do cant be a loop sequence
2023-
if (const auto &doConstruct{
2024-
std::get_if<parser::DoConstruct>(&loopConsList.front())}) {
2025-
if (doConstruct->IsDoWhile()) {
2026-
return true;
2027-
}
2022+
assert(loopConsList.size() == 1 &&
2023+
"Expected a single DoConstruct or OpenMPLoopConstruct");
2024+
if (const auto &doConstruct{
2025+
std::get_if<parser::DoConstruct>(&loopConsList.front())}) {
2026+
if (doConstruct->IsDoWhile()) {
2027+
return true;
20282028
}
20292029
}
20302030
}
@@ -2375,7 +2375,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
23752375
auto &loopConsList =
23762376
std::get<std::list<parser::NestedConstruct>>(innerMostLoop->t);
23772377
for (auto &loopCons : loopConsList) {
2378-
std::int64_t level_{level};
2378+
std::int64_t curLevel{level};
23792379
const parser::NestedConstruct *innerMostNest = nullptr;
23802380
if (const auto &innerloop{std::get_if<parser::DoConstruct>(&loopCons)}) {
23812381
innerMostNest = &loopCons;
@@ -2387,8 +2387,8 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
23872387

23882388
if (innerMostNest) {
23892389
if (const auto &outer{std::get_if<parser::DoConstruct>(innerMostNest)}) {
2390-
for (const parser::DoConstruct *loop{&*outer}; loop && level_ > 0;
2391-
--level_) {
2390+
for (const parser::DoConstruct *loop{&*outer}; loop && curLevel > 0;
2391+
--curLevel) {
23922392
if (loop->IsDoConcurrent()) {
23932393
// DO CONCURRENT is explicitly allowed for the LOOP construct so long
23942394
// as there isn't a COLLAPSE clause
@@ -2419,7 +2419,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
24192419
loop = it != block.end() ? GetDoConstructIf(*it) : nullptr;
24202420
}
24212421
}
2422-
CheckAssocLoopLevel(level_, GetAssociatedClause());
2422+
CheckAssocLoopLevel(curLevel, GetAssociatedClause());
24232423
} else if (const auto *loop{std::get_if<
24242424
common::Indirection<parser::OpenMPLoopConstruct>>(
24252425
innerMostNest)}) {

‎flang/test/Parser/OpenMP/fail-looprange.f90‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
! RUN: not %flang_fc1 -fsyntax-only -fopenmp %s 2>&1 | FileCheck %s
2+
3+
! CHECK: error: expected end of line
4+
!$omp fuse looprange
5+
6+
! CHECK: error: expected end of line
7+
!$omp fuse looprange(1)
8+
9+
! CHECK: error: expected end of line
10+
!$omp fuse looprange(1,2,3)
11+
end

‎flang/test/Parser/OpenMP/fuse.f90‎ renamed to ‎flang/test/Parser/OpenMP/fuse01.f90‎

File renamed without changes.

‎flang/test/Parser/OpenMP/fuse02.f90‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
! Test the Parse Tree to ensure the OpenMP Loop Transformation Construct Fuse can be constructed on another Fuse
2+
3+
! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-PARSE
4+
! RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=51 %s | FileCheck %s --check-prefix=CHECK-UNPARSE
5+
6+
subroutine fuse_on_fuse
7+
implicit none
8+
integer :: I = 10
9+
integer :: j
10+
11+
!$omp fuse
12+
!$omp fuse
13+
do i = 1, I
14+
continue
15+
end do
16+
do j = 1, I
17+
continue
18+
end do
19+
!$omp end fuse
20+
do j = 1, I
21+
continue
22+
end do
23+
!$omp end fuse
24+
end subroutine
25+
26+
!CHECK-PARSE: | ExecutionPart -> Block
27+
!CHECK-PARSE-NEXT: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
28+
!CHECK-PARSE-NEXT: | | | OmpBeginLoopDirective
29+
!CHECK-PARSE-NEXT: | | | | OmpDirectiveName -> llvm::omp::Directive = fuse
30+
!CHECK-PARSE-NEXT: | | | | OmpClauseList ->
31+
!CHECK-PARSE-NEXT: | | | | Flags = None
32+
!CHECK-PARSE-NEXT: | | | OpenMPLoopConstruct
33+
!CHECK-PARSE-NEXT: | | | | OmpBeginLoopDirective
34+
!CHECK-PARSE-NEXT: | | | | | OmpDirectiveName -> llvm::omp::Directive = fuse
35+
!CHECK-PARSE-NEXT: | | | | | OmpClauseList ->
36+
!CHECK-PARSE-NEXT: | | | | | Flags = None
37+
!CHECK-PARSE-NEXT: | | | | DoConstruct
38+
!CHECK-PARSE-NEXT: | | | | | NonLabelDoStmt
39+
!CHECK-PARSE-NEXT: | | | | | | LoopControl -> LoopBounds
40+
!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Name = 'i'
41+
!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Expr = '1_4'
42+
!CHECK-PARSE-NEXT: | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
43+
!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Expr = 'i'
44+
!CHECK-PARSE-NEXT: | | | | | | | | Designator -> DataRef -> Name = 'i'
45+
!CHECK-PARSE-NEXT: | | | | | Block
46+
!CHECK-PARSE-NEXT: | | | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> ContinueStmt
47+
!CHECK-PARSE-NEXT: | | | | | EndDoStmt ->
48+
!CHECK-PARSE-NEXT: | | | | DoConstruct
49+
!CHECK-PARSE-NEXT: | | | | | NonLabelDoStmt
50+
!CHECK-PARSE-NEXT: | | | | | | LoopControl -> LoopBounds
51+
!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Name = 'j'
52+
!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Expr = '1_4'
53+
!CHECK-PARSE-NEXT: | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
54+
!CHECK-PARSE-NEXT: | | | | | | | Scalar -> Expr = 'i'
55+
!CHECK-PARSE-NEXT: | | | | | | | | Designator -> DataRef -> Name = 'i'
56+
!CHECK-PARSE-NEXT: | | | | | Block
57+
!CHECK-PARSE-NEXT: | | | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> ContinueStmt
58+
!CHECK-PARSE-NEXT: | | | | | EndDoStmt ->
59+
!CHECK-PARSE-NEXT: | | | | OmpEndLoopDirective
60+
!CHECK-PARSE-NEXT: | | | | | OmpDirectiveName -> llvm::omp::Directive = fuse
61+
!CHECK-PARSE-NEXT: | | | | | OmpClauseList ->
62+
!CHECK-PARSE-NEXT: | | | | | Flags = None
63+
!CHECK-PARSE-NEXT: | | | DoConstruct
64+
!CHECK-PARSE-NEXT: | | | | NonLabelDoStmt
65+
!CHECK-PARSE-NEXT: | | | | | LoopControl -> LoopBounds
66+
!CHECK-PARSE-NEXT: | | | | | | Scalar -> Name = 'j'
67+
!CHECK-PARSE-NEXT: | | | | | | Scalar -> Expr = '1_4'
68+
!CHECK-PARSE-NEXT: | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
69+
!CHECK-PARSE-NEXT: | | | | | | Scalar -> Expr = 'i'
70+
!CHECK-PARSE-NEXT: | | | | | | | Designator -> DataRef -> Name = 'i'
71+
!CHECK-PARSE-NEXT: | | | | Block
72+
!CHECK-PARSE-NEXT: | | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> ContinueStmt
73+
!CHECK-PARSE-NEXT: | | | | EndDoStmt ->
74+
!CHECK-PARSE-NEXT: | | | OmpEndLoopDirective
75+
!CHECK-PARSE-NEXT: | | | | OmpDirectiveName -> llvm::omp::Directive = fuse
76+
!CHECK-PARSE-NEXT: | | | | OmpClauseList ->
77+
!CHECK-PARSE-NEXT: | | | | Flags = None
78+
79+
!CHECK-UNPARSE: SUBROUTINE fuse_on_fuse
80+
!CHECK-UNPARSE-NEXT: IMPLICIT NONE
81+
!CHECK-UNPARSE-NEXT: INTEGER :: i = 10_4
82+
!CHECK-UNPARSE-NEXT: INTEGER j
83+
!CHECK-UNPARSE-NEXT: !$OMP FUSE
84+
!CHECK-UNPARSE-NEXT: !$OMP FUSE
85+
!CHECK-UNPARSE-NEXT: DO i=1_4,i
86+
!CHECK-UNPARSE-NEXT: CONTINUE
87+
!CHECK-UNPARSE-NEXT: END DO
88+
!CHECK-UNPARSE-NEXT: DO j=1_4,i
89+
!CHECK-UNPARSE-NEXT: CONTINUE
90+
!CHECK-UNPARSE-NEXT: END DO
91+
!CHECK-UNPARSE-NEXT: !$OMP END FUSE
92+
!CHECK-UNPARSE-NEXT: DO j=1_4,i
93+
!CHECK-UNPARSE-NEXT: CONTINUE
94+
!CHECK-UNPARSE-NEXT: END DO
95+
!CHECK-UNPARSE-NEXT: !$OMP END FUSE

0 commit comments

Comments
 (0)