Skip to content

Commit 615a759

Browse files
anchurajanchuraj
committed
R2: Addressing review comments
1 parent ee14b03 commit 615a759

File tree

6 files changed

+130
-78
lines changed

6 files changed

+130
-78
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ static const OmpDirectiveSet allDistributeParallelDoSimdSet{
195195
static const OmpDirectiveSet allDistributeSimdSet{
196196
allDistributeSet & allSimdSet};
197197
static const OmpDirectiveSet allDoSimdSet{allDoSet & allSimdSet};
198+
static const OmpDirectiveSet scanAllowedSet{allDoSet | allSimdSet};
198199
static const OmpDirectiveSet allTaskloopSimdSet{allTaskloopSet & allSimdSet};
199200

200201
static const OmpDirectiveSet compositeConstructSet{

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

Lines changed: 57 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "flang/Parser/parse-tree.h"
1313
#include "flang/Semantics/expression.h"
1414
#include "flang/Semantics/tools.h"
15+
#include <variant>
1516

1617
namespace Fortran::semantics {
1718

@@ -747,63 +748,61 @@ void OmpStructureChecker::CheckSIMDNest(const parser::OpenMPConstruct &c) {
747748
// current context yet.
748749
// TODO: Check for declare simd regions.
749750
bool eligibleSIMD{false};
750-
common::visit(
751-
Fortran::common::visitors{
752-
// Allow `!$OMP ORDERED SIMD`
753-
[&](const parser::OpenMPBlockConstruct &c) {
754-
const auto &beginBlockDir{
755-
std::get<parser::OmpBeginBlockDirective>(c.t)};
756-
const auto &beginDir{
757-
std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
758-
if (beginDir.v == llvm::omp::Directive::OMPD_ordered) {
759-
const auto &clauses{
760-
std::get<parser::OmpClauseList>(beginBlockDir.t)};
761-
for (const auto &clause : clauses.v) {
762-
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
763-
eligibleSIMD = true;
764-
break;
765-
}
766-
}
767-
}
768-
},
769-
[&](const parser::OpenMPStandaloneConstruct &c) {
770-
if (const auto &simpleConstruct =
771-
std::get_if<parser::OpenMPSimpleStandaloneConstruct>(
772-
&c.u)) {
773-
const auto &dir{std::get<parser::OmpSimpleStandaloneDirective>(
774-
simpleConstruct->t)};
775-
if (dir.v == llvm::omp::Directive::OMPD_ordered) {
776-
const auto &clauses{
777-
std::get<parser::OmpClauseList>(simpleConstruct->t)};
778-
for (const auto &clause : clauses.v) {
779-
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
780-
eligibleSIMD = true;
781-
break;
782-
}
783-
}
784-
} else if (dir.v == llvm::omp::Directive::OMPD_scan) {
785-
eligibleSIMD = true;
786-
}
787-
}
788-
},
789-
// Allowing SIMD construct
790-
[&](const parser::OpenMPLoopConstruct &c) {
791-
const auto &beginLoopDir{
792-
std::get<parser::OmpBeginLoopDirective>(c.t)};
793-
const auto &beginDir{
794-
std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
795-
if ((beginDir.v == llvm::omp::Directive::OMPD_simd) ||
796-
(beginDir.v == llvm::omp::Directive::OMPD_parallel_do_simd) ||
797-
(beginDir.v == llvm::omp::Directive::OMPD_do_simd)) {
798-
eligibleSIMD = true;
799-
}
800-
},
801-
[&](const parser::OpenMPAtomicConstruct &c) {
802-
// Allow `!$OMP ATOMIC`
803-
eligibleSIMD = true;
804-
},
805-
[&](const auto &c) {},
806-
},
751+
common::visit(Fortran::common::visitors{
752+
// Allow `!$OMP ORDERED SIMD`
753+
[&](const parser::OpenMPBlockConstruct &c) {
754+
const auto &beginBlockDir{
755+
std::get<parser::OmpBeginBlockDirective>(c.t)};
756+
const auto &beginDir{
757+
std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
758+
if (beginDir.v == llvm::omp::Directive::OMPD_ordered) {
759+
const auto &clauses{
760+
std::get<parser::OmpClauseList>(beginBlockDir.t)};
761+
for (const auto &clause : clauses.v) {
762+
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
763+
eligibleSIMD = true;
764+
break;
765+
}
766+
}
767+
}
768+
},
769+
[&](const parser::OpenMPStandaloneConstruct &c) {
770+
if (const auto &simpleConstruct =
771+
std::get_if<parser::OpenMPSimpleStandaloneConstruct>(
772+
&c.u)) {
773+
const auto &dir{std::get<parser::OmpSimpleStandaloneDirective>(
774+
simpleConstruct->t)};
775+
if (dir.v == llvm::omp::Directive::OMPD_ordered) {
776+
const auto &clauses{
777+
std::get<parser::OmpClauseList>(simpleConstruct->t)};
778+
for (const auto &clause : clauses.v) {
779+
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
780+
eligibleSIMD = true;
781+
break;
782+
}
783+
}
784+
} else if (dir.v == llvm::omp::Directive::OMPD_scan) {
785+
eligibleSIMD = true;
786+
}
787+
}
788+
},
789+
// Allowing SIMD construct
790+
[&](const parser::OpenMPLoopConstruct &c) {
791+
const auto &beginLoopDir{
792+
std::get<parser::OmpBeginLoopDirective>(c.t)};
793+
const auto &beginDir{
794+
std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
795+
if ((beginDir.v == llvm::omp::Directive::OMPD_simd) ||
796+
(beginDir.v == llvm::omp::Directive::OMPD_do_simd)) {
797+
eligibleSIMD = true;
798+
}
799+
},
800+
[&](const parser::OpenMPAtomicConstruct &c) {
801+
// Allow `!$OMP ATOMIC`
802+
eligibleSIMD = true;
803+
},
804+
[&](const auto &c) {},
805+
},
807806
c.u);
808807
if (!eligibleSIMD) {
809808
context_.Say(parser::FindSourceLocation(c),
@@ -2990,15 +2989,7 @@ void OmpStructureChecker::CheckReductionModifier(
29902989
// or "simd" directive.
29912990
// The worksharing-loop directives are OMPD_do and OMPD_for. Only the
29922991
// former is allowed in Fortran.
2993-
switch (dirCtx.directive) {
2994-
case llvm::omp::Directive::OMPD_do: // worksharing-loop
2995-
case llvm::omp::Directive::OMPD_do_simd: // worksharing-loop simd
2996-
case llvm::omp::Directive::
2997-
OMPD_parallel_do_simd: // worksharing-parallel-loop
2998-
// simd
2999-
case llvm::omp::Directive::OMPD_simd: // "simd"
3000-
break;
3001-
default:
2992+
if (!llvm::omp::scanAllowedSet.test(dirCtx.directive)) {
30022993
context_.Say(GetContext().clauseSource,
30032994
"Modifier 'INSCAN' on REDUCTION clause is only allowed with "
30042995
"worksharing-loop, worksharing-loop simd, "

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,8 +1644,7 @@ bool OmpAttributeVisitor::Pre(
16441644
"Exactly one of `exclusive` or `inclusive` clause is expected"_err_en_US);
16451645
}
16461646
if (!parentContext ||
1647-
(llvm::omp::getDirectiveAssociation(parentContext->directive) !=
1648-
llvm::omp::Association::Loop)) {
1647+
(!llvm::omp::scanAllowedSet.test(parentContext->directive))) {
16491648
context_.Say(standaloneDir.source,
16501649
"Orphaned `omp scan` directives are prohibited; perhaps you forgot "
16511650
"to enclose the directive in to a worksharing loop, a worksharing "

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,11 @@
22
! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
33

44
! Check for parsing scan directive
5-
subroutine test_scan_inclusive()
5+
subroutine test_scan(n, a, b)
66
implicit none
7-
integer, parameter :: n = 100
7+
integer n
88
integer a(n), b(n)
9-
integer x, k
10-
11-
! initialization
12-
x = 0
13-
do k = 1, n
14-
a(k) = k
15-
end do
9+
integer x,y,k
1610

1711
! a(k) is included in the computation of producing results in b(k)
1812
!$omp parallel do simd reduction(inscan,+: x)
@@ -37,4 +31,28 @@ subroutine test_scan_inclusive()
3731
!$omp scan exclusive(x)
3832
x = x + a(k)
3933
end do
34+
35+
!$omp parallel do simd reduction(inscan,+: x)
36+
do k = 1, n
37+
x = x + a(k)
38+
!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct
39+
!PARSE-TREE-NEXT: OmpSimpleStandaloneDirective -> llvm::omp::Directive = scan
40+
!PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Inclusive -> OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
41+
!PARSE-TREE-NEXT: OmpObject -> Designator -> DataRef -> Name = 'y'
42+
!CHECK: !$omp scan inclusive(x,y)
43+
!$omp scan inclusive(x, y)
44+
b(k) = x
45+
end do
46+
47+
!$omp parallel do simd reduction(inscan,+: x)
48+
do k = 1, n
49+
x = x + a(k)
50+
!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct
51+
!PARSE-TREE-NEXT: OmpSimpleStandaloneDirective -> llvm::omp::Directive = scan
52+
!PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Exclusive -> OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
53+
!PARSE-TREE-NEXT: OmpObject -> Designator -> DataRef -> Name = 'y'
54+
!CHECK: !$omp scan exclusive(x,y)
55+
!$omp scan exclusive(x, y)
56+
b(k) = x
57+
end do
4058
end subroutine

‎flang/test/Semantics/OpenMP/out

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
error: Semantic errors in nested-simd.f90
2+
./nested-simd.f90:45:15: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
3+
!$omp teams
4+
^
5+
./nested-simd.f90:45:15: error: TEAMS region can only be strictly nested within the implicit parallel region or TARGET region
6+
!$omp teams
7+
^
8+
./nested-simd.f90:62:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
9+
!$omp task
10+
^
11+
./nested-simd.f90:68:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
12+
!$omp target
13+
^
14+
./nested-simd.f90:95:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
15+
!$omp task
16+
^
17+
./nested-simd.f90:101:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
18+
!$omp target
19+
^
20+
./nested-simd.f90:108:13: error: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
21+
!$OMP DO
22+
^^
23+
./nested-simd.f90:129:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
24+
!$omp task
25+
^
26+
./nested-simd.f90:135:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
27+
!$omp target
28+
^
29+
./nested-simd.f90:142:13: error: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
30+
!$OMP DO
31+
^^
32+
./nested-simd.f90:163:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
33+
!$omp task
34+
^
35+
./nested-simd.f90:169:13: error: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct, the `SCAN` construct and the `ORDERED` construct with the `SIMD` clause.
36+
!$omp target
37+
^

‎flang/test/Semantics/OpenMP/scan.f90

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
22

33
subroutine test_scan()
4-
integer x, k
4+
integer x, y, k
55

66
!ERROR: Orphaned `omp scan` directives are prohibited; perhaps you forgot to enclose the directive in to a worksharing loop, a worksharing loop simd or a simd directive.
77
!$omp scan inclusive(x)
@@ -16,4 +16,10 @@ subroutine test_scan()
1616
!ERROR: Exactly one of `exclusive` or `inclusive` clause is expected
1717
!$omp scan
1818
end do
19+
20+
!$omp parallel do simd reduction(inscan,+: x)
21+
do k = 1, n
22+
!ERROR: Exactly one of `exclusive` or `inclusive` clause is expected
23+
!$omp scan inclusive(x) exclusive(y)
24+
end do
1925
end subroutine

0 commit comments

Comments
 (0)