Skip to content

Commit ead87b9

Browse files
author
Ferran Toda
committed
Address reviews, add more tests and fix an issue
1 parent de8f50c commit ead87b9

13 files changed

+307
-53
lines changed

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

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

10391039
LoopRange make(const parser::OmpClause::Looprange &inp,
10401040
semantics::SemanticsContext &semaCtx) {
1041-
TODO_NOLOC("looprange clause");
1041+
llvm_unreachable("Unimplemented: looprange");
10421042
}
10431043

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

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3360,7 +3360,12 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
33603360
newOp = genTeamsOp(converter, symTable, stmtCtx, semaCtx, eval, loc, queue,
33613361
item);
33623362
break;
3363-
case llvm::omp::Directive::OMPD_fuse:
3363+
case llvm::omp::Directive::OMPD_fuse: {
3364+
unsigned version = semaCtx.langOptions().OpenMPVersion;
3365+
TODO(loc, "Unhandled loop directive (" +
3366+
llvm::omp::getOpenMPDirectiveName(dir, version) + ")");
3367+
break;
3368+
}
33643369
case llvm::omp::Directive::OMPD_tile: {
33653370
unsigned version = semaCtx.langOptions().OpenMPVersion;
33663371
if (!semaCtx.langOptions().OpenMPSimd)

‎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
@@ -307,7 +307,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
307307
}
308308
if (beginName.v == llvm::omp::Directive::OMPD_fuse) {
309309
CheckLooprangeBounds(x);
310-
}
310+
} else
311+
CheckNestedFuse(x);
311312
}
312313

313314
const parser::Name OmpStructureChecker::GetLoopIndex(
@@ -422,7 +423,7 @@ void OmpStructureChecker::CheckDistLinear(
422423
// clauses.
423424
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
424425
for (auto &loopCons : loopConsList) {
425-
std::int64_t collapseVal_ = collapseVal;
426+
std::int64_t curCollapseVal{collapseVal};
426427
if (const auto &loopConstruct{
427428
std::get_if<parser::DoConstruct>(&loopCons)}) {
428429
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
@@ -432,8 +433,8 @@ void OmpStructureChecker::CheckDistLinear(
432433
// Remove the symbol from the collected set
433434
indexVars.erase(&itrVal.symbol->GetUltimate());
434435
}
435-
collapseVal_--;
436-
if (collapseVal_ == 0) {
436+
curCollapseVal--;
437+
if (curCollapseVal == 0) {
437438
break;
438439
}
439440
}
@@ -467,7 +468,8 @@ void OmpStructureChecker::CheckLooprangeBounds(
467468
if (const auto count{GetIntValue(std::get<1>((lrClause->v).t))}) {
468469
auto &loopConsList =
469470
std::get<std::list<parser::NestedConstruct>>(x.t);
470-
if (loopConsList.size() < (unsigned)(*first + *count - 1)) {
471+
if (*first > 0 && *count > 0 &&
472+
loopConsList.size() < (unsigned)(*first + *count - 1)) {
471473
context_.Say(clause.source,
472474
"The loop range indicated in the %s clause"
473475
" must not be out of the bounds of the Loop Sequence"
@@ -482,6 +484,38 @@ void OmpStructureChecker::CheckLooprangeBounds(
482484
}
483485
}
484486

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ class OmpStructureChecker
298298
void CheckAtomicUpdate(const parser::OpenMPAtomicConstruct &x);
299299

300300
void CheckLooprangeBounds(const parser::OpenMPLoopConstruct &x);
301+
void CheckNestedFuse(const parser::OpenMPLoopConstruct &x);
301302
void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
302303
void CheckSIMDNest(const parser::OpenMPConstruct &x);
303304
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
@@ -1985,12 +1985,12 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
19851985

19861986
if (beginName.v == llvm::omp::Directive::OMPD_do) {
19871987
auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
1988-
if (loopConsList.size() == 1) { //a OMPD_do cant be a loop sequence
1989-
if (const auto &doConstruct{
1990-
std::get_if<parser::DoConstruct>(&loopConsList.front())}) {
1991-
if (doConstruct->IsDoWhile()) {
1992-
return true;
1993-
}
1988+
assert(loopConsList.size() == 1 &&
1989+
"Expected a single DoConstruct or OpenMPLoopConstruct");
1990+
if (const auto &doConstruct{
1991+
std::get_if<parser::DoConstruct>(&loopConsList.front())}) {
1992+
if (doConstruct->IsDoWhile()) {
1993+
return true;
19941994
}
19951995
}
19961996
}
@@ -2224,7 +2224,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
22242224
auto &loopConsList =
22252225
std::get<std::list<parser::NestedConstruct>>(innerMostLoop->t);
22262226
for (auto &loopCons : loopConsList) {
2227-
std::int64_t level_{level};
2227+
std::int64_t curLevel{level};
22282228
const parser::NestedConstruct *innerMostNest = nullptr;
22292229
if (const auto &innerloop{std::get_if<parser::DoConstruct>(&loopCons)}) {
22302230
innerMostNest = &loopCons;
@@ -2236,8 +2236,8 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
22362236

22372237
if (innerMostNest) {
22382238
if (const auto &outer{std::get_if<parser::DoConstruct>(innerMostNest)}) {
2239-
for (const parser::DoConstruct *loop{&*outer}; loop && level_ > 0;
2240-
--level_) {
2239+
for (const parser::DoConstruct *loop{&*outer}; loop && curLevel > 0;
2240+
--curLevel) {
22412241
if (loop->IsDoConcurrent()) {
22422242
// DO CONCURRENT is explicitly allowed for the LOOP construct so long
22432243
// as there isn't a COLLAPSE clause
@@ -2268,7 +2268,7 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
22682268
loop = it != block.end() ? GetDoConstructIf(*it) : nullptr;
22692269
}
22702270
}
2271-
CheckAssocLoopLevel(level_, GetAssociatedClause());
2271+
CheckAssocLoopLevel(curLevel, GetAssociatedClause());
22722272
} else if (const auto *loop{std::get_if<
22732273
common::Indirection<parser::OpenMPLoopConstruct>>(
22742274
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)