[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
This revision was automatically updated to reflect the committed changes.
Closed by commit rL295808: [OpenMP] Generate better diagnostics for cancel and
cancellation point (authored by Hahnfeld).
Changed prior to commit:
https://reviews.llvm.org/D30135?vs=89106=89326#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30135
Files:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/cancel_messages.cpp
cfe/trunk/test/OpenMP/cancellation_point_messages.cpp
Index: cfe/trunk/test/OpenMP/cancel_messages.cpp
===
--- cfe/trunk/test/OpenMP/cancel_messages.cpp
+++ cfe/trunk/test/OpenMP/cancel_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancel'}}
#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel sections( // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
#pragma omp cancel for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
#pragma omp cancel taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
Index: cfe/trunk/test/OpenMP/cancellation_point_messages.cpp
===
--- cfe/trunk/test/OpenMP/cancellation_point_messages.cpp
+++ cfe/trunk/test/OpenMP/cancellation_point_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancellation point'}}
#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point sections( // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
Index: cfe/trunk/lib/Sema/SemaOpenMP.cpp
===
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp
@@ -1956,7 +1956,23 @@
return SR;
}
-static bool CheckNestingOfRegions(Sema , DSAStackTy *Stack,
+static bool checkCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+ OpenMPDirectiveKind CancelRegion,
+ SourceLocation StartLoc) {
+ // CancelRegion is only needed for cancel and cancellation_point.
+ if (CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_cancellation_point)
+return false;
+
+ if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
+ CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
+return false;
+
+ SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+ << getOpenMPDirectiveName(CancelRegion);
+ return true;
+}
+
+static bool checkNestingOfRegions(Sema , DSAStackTy *Stack,
OpenMPDirectiveKind CurrentRegion,
const DeclarationNameInfo ,
OpenMPDirectiveKind CancelRegion,
@@ -2256,7 +2272,9 @@
OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc) {
StmtResult Res = StmtError();
- if (CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
+ // First check CancelRegion which is then used in checkNestingOfRegions.
+ if (checkCancelRegion(*this, Kind, CancelRegion, StartLoc) ||
+ checkNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D30135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
Hahnfeld updated this revision to Diff 89106.
Hahnfeld marked 3 inline comments as done.
Hahnfeld edited the summary of this revision.
Hahnfeld added a comment.
Address review comment's and apply new naming style to checkNestingOfRegions
https://reviews.llvm.org/D30135
Files:
lib/Sema/SemaOpenMP.cpp
test/OpenMP/cancel_messages.cpp
test/OpenMP/cancellation_point_messages.cpp
Index: test/OpenMP/cancellation_point_messages.cpp
===
--- test/OpenMP/cancellation_point_messages.cpp
+++ test/OpenMP/cancellation_point_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancellation point'}}
#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point sections( // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
Index: test/OpenMP/cancel_messages.cpp
===
--- test/OpenMP/cancel_messages.cpp
+++ test/OpenMP/cancel_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancel'}}
#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel sections( // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
#pragma omp cancel for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
#pragma omp cancel taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -1956,7 +1956,23 @@
return SR;
}
-static bool CheckNestingOfRegions(Sema , DSAStackTy *Stack,
+static bool checkCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+ OpenMPDirectiveKind CancelRegion,
+ SourceLocation StartLoc) {
+ // CancelRegion is only needed for cancel and cancellation_point.
+ if (CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_cancellation_point)
+return false;
+
+ if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
+ CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
+return false;
+
+ SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+ << getOpenMPDirectiveName(CancelRegion);
+ return true;
+}
+
+static bool checkNestingOfRegions(Sema , DSAStackTy *Stack,
OpenMPDirectiveKind CurrentRegion,
const DeclarationNameInfo ,
OpenMPDirectiveKind CancelRegion,
@@ -2256,7 +2272,9 @@
OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc) {
StmtResult Res = StmtError();
- if (CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
+ // First check CancelRegion which is then used in checkNestingOfRegions.
+ if (checkCancelRegion(*this, Kind, CancelRegion, StartLoc) ||
+ checkNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
StartLoc))
return StmtError();
@@ -5860,12 +5878,6 @@
Sema::ActOnOpenMPCancellationPointDirective(SourceLocation StartLoc,
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
ABataev added inline comments.
Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+ if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+ CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+ }
Hahnfeld wrote:
> ABataev wrote:
> > Hahnfeld wrote:
> > > ABataev wrote:
> > > > It is better to convert this to return `false` and make error message
> > > > and `return true` statement unconditional
> > > I wanted to keep the style in `CheckNestingOfRegions`: That way it's
> > > easier to add more checks later on if needed like additional restrictions
> > > on `CancelRegion`
> > I just meant that it's better to make it look like this:
> > ```
> > if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
> > CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
> > return false;
> >
> > SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
> > << getOpenMPDirectiveName(CancelRegion);
> > return true;
> > ```
> I understood what you want it to look like. However, that style makes it
> impossible to add additional diagnostics to this function
Let's think about it later, if(!) some changes will be required
https://reviews.llvm.org/D30135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
Hahnfeld added inline comments.
Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+ if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+ CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+ }
ABataev wrote:
> Hahnfeld wrote:
> > ABataev wrote:
> > > It is better to convert this to return `false` and make error message and
> > > `return true` statement unconditional
> > I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier
> > to add more checks later on if needed like additional restrictions on
> > `CancelRegion`
> I just meant that it's better to make it look like this:
> ```
> if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
> CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
> return false;
>
> SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
> << getOpenMPDirectiveName(CancelRegion);
> return true;
> ```
I understood what you want it to look like. However, that style makes it
impossible to add additional diagnostics to this function
https://reviews.llvm.org/D30135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
ABataev added inline comments.
Comment at: lib/Sema/SemaOpenMP.cpp:1959
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+ OpenMPDirectiveKind CancelRegion,
Hahnfeld wrote:
> ABataev wrote:
> > Should be `checkCancelRegion`
> It's also `CheckNestingOfRegions`, but can do
Old formatting, this should be fixed
Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+ if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+ CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+ }
Hahnfeld wrote:
> ABataev wrote:
> > It is better to convert this to return `false` and make error message and
> > `return true` statement unconditional
> I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier
> to add more checks later on if needed like additional restrictions on
> `CancelRegion`
I just meant that it's better to make it look like this:
```
if (CancelRegion == OMPD_parallel || CancelRegion == OMPD_for ||
CancelRegion == OMPD_sections || CancelRegion == OMPD_taskgroup)
return false;
SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
<< getOpenMPDirectiveName(CancelRegion);
return true;
```
https://reviews.llvm.org/D30135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
Hahnfeld added inline comments.
Comment at: lib/Sema/SemaOpenMP.cpp:1959
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+ OpenMPDirectiveKind CancelRegion,
ABataev wrote:
> Should be `checkCancelRegion`
It's also `CheckNestingOfRegions`, but can do
Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+ if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+ CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+ }
ABataev wrote:
> It is better to convert this to return `false` and make error message and
> `return true` statement unconditional
I wanted to keep the style in `CheckNestingOfRegions`: That way it's easier to
add more checks later on if needed like additional restrictions on
`CancelRegion`
https://reviews.llvm.org/D30135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
ABataev added inline comments.
Comment at: lib/Sema/SemaOpenMP.cpp:1959
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+ OpenMPDirectiveKind CancelRegion,
Should be `checkCancelRegion`
Comment at: lib/Sema/SemaOpenMP.cpp:1964-1966
+ CurrentRegion != OMPD_cancellation_point) {
+return false;
+ }
No need for braces
Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973
+ if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+ CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+ }
It is better to convert this to return `false` and make error message and
`return true` statement unconditional
https://reviews.llvm.org/D30135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
Hahnfeld updated this revision to Diff 89099.
Hahnfeld edited the summary of this revision.
Hahnfeld added a comment.
new static function `CheckCancelRegion`
https://reviews.llvm.org/D30135
Files:
lib/Sema/SemaOpenMP.cpp
test/OpenMP/cancel_messages.cpp
test/OpenMP/cancellation_point_messages.cpp
Index: test/OpenMP/cancellation_point_messages.cpp
===
--- test/OpenMP/cancellation_point_messages.cpp
+++ test/OpenMP/cancellation_point_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancellation point'}}
#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point sections( // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
Index: test/OpenMP/cancel_messages.cpp
===
--- test/OpenMP/cancel_messages.cpp
+++ test/OpenMP/cancel_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancel'}}
#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel sections( // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
#pragma omp cancel for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
#pragma omp cancel taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancel' are ignored}}
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -1956,6 +1956,25 @@
return SR;
}
+static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion,
+ OpenMPDirectiveKind CancelRegion,
+ SourceLocation StartLoc) {
+ // CancelRegion is only needed for cancel and cancellation_point.
+ if (CurrentRegion != OMPD_cancel &&
+ CurrentRegion != OMPD_cancellation_point) {
+return false;
+ }
+
+ if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
+ CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) {
+SemaRef.Diag(StartLoc, diag::err_omp_wrong_cancel_region)
+<< getOpenMPDirectiveName(CancelRegion);
+return true;
+ }
+
+ return false;
+}
+
static bool CheckNestingOfRegions(Sema , DSAStackTy *Stack,
OpenMPDirectiveKind CurrentRegion,
const DeclarationNameInfo ,
@@ -2256,7 +2275,9 @@
OpenMPDirectiveKind CancelRegion, ArrayRef Clauses,
Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc) {
StmtResult Res = StmtError();
- if (CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
+ // First CheckCancelRegion which is then used in CheckNestingOfRegions.
+ if (CheckCancelRegion(*this, Kind, CancelRegion, StartLoc) ||
+ CheckNestingOfRegions(*this, DSAStack, Kind, DirName, CancelRegion,
StartLoc))
return StmtError();
@@ -5860,12 +5881,6 @@
Sema::ActOnOpenMPCancellationPointDirective(SourceLocation StartLoc,
SourceLocation EndLoc,
OpenMPDirectiveKind CancelRegion) {
- if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for &&
-
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
ABataev added a comment. I see. I think it is better to check the `CancelRegion` just before call of `CheckNestingOfRegions()` function. You need to extract checks for `CancelRegion` from `ActOnOpenMPCancellationPointDirective()` and `ActOnOpenMPCancelDirective()` functions into a standalone function and call it before `CheckNestingOfRegions()`. https://reviews.llvm.org/D30135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
Hahnfeld added a comment.
In https://reviews.llvm.org/D30135#681354, @ABataev wrote:
> Not sure that this is better because at first, we need to be sure that this
> nesting is allowed. Why do we need to perform some additional analysis if
> nesting is not allowed at all?
`CheckNestingOfRegions` uses `CancelRegion` to determine whether cancel and
cancellation point may be nested inside the parent construct. However with the
current code, `CancelRegion` would only be checked afterwards.
#pragma omp parallel
{
#pragma omp cancellation point unknown
}
therefore produces `region cannot be closely nested inside 'parallel' region`.
After this change, it says `one of 'for', 'parallel', 'sections' or 'taskgroup'
is expected` as in the test case which is better IMO.
Should I try to improve the summary to explain that?
https://reviews.llvm.org/D30135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
ABataev added a comment. Not sure that this is better because at first, we need to be sure that this nesting is allowed. Why do we need to perform some additional analysis if nesting is not allowed at all? https://reviews.llvm.org/D30135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point
Hahnfeld created this revision.
Handle errors related to a specific directive before checking the nesting:
The specific checks may validate required arguments and give more helpful
messages, especially when the nesting depends on those arguments.
This change requires some minor adaptions to the tests to maintain the
expected diagnostics.
https://reviews.llvm.org/D30135
Files:
lib/Sema/SemaOpenMP.cpp
test/OpenMP/cancel_messages.cpp
test/OpenMP/cancellation_point_messages.cpp
test/OpenMP/nesting_of_regions.cpp
test/OpenMP/task_messages.cpp
Index: test/OpenMP/task_messages.cpp
===
--- test/OpenMP/task_messages.cpp
+++ test/OpenMP/task_messages.cpp
@@ -101,7 +101,8 @@
// expected-error@+2 {{reduction variable must be shared}}
// expected-error@+1 {{region cannot be closely nested inside 'task' region; perhaps you forget to enclose 'omp for' directive into a parallel region?}}
#pragma omp for reduction(+ : r)
- ++r;
+ for (int i = 0; i < 10; ++i)
+++r;
// expected-error@+1 {{directive '#pragma omp task' cannot contain more than one 'untied' clause}}
#pragma omp task untied untied
++r;
@@ -257,7 +258,8 @@
// expected-error@+2 {{reduction variable must be shared}}
// expected-error@+1 {{region cannot be closely nested inside 'task' region; perhaps you forget to enclose 'omp for' directive into a parallel region?}}
#pragma omp for reduction(+ : r)
- ++r;
+ for (int i = 0; i < 10; ++i)
+++r;
// expected-error@+1 {{directive '#pragma omp task' cannot contain more than one 'untied' clause}}
#pragma omp task untied untied
++r;
Index: test/OpenMP/nesting_of_regions.cpp
===
--- test/OpenMP/nesting_of_regions.cpp
+++ test/OpenMP/nesting_of_regions.cpp
@@ -12467,7 +12467,7 @@
// expected-error@+2 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an l-value expression with scalar type}}
// expected-note@+1 {{expected an expression statement}}
{
-#pragma omp target update // expected-error {{OpenMP constructs may not be nested inside an atomic region}}
+#pragma omp target update to(a) // expected-error {{OpenMP constructs may not be nested inside an atomic region}}
}
#pragma omp atomic
// expected-error@+2 {{the statement for 'atomic' must be an expression statement of form '++x;', '--x;', 'x++;', 'x--;', 'x binop= expr;', 'x = x binop expr' or 'x = expr binop x', where x is an l-value expression with scalar type}}
Index: test/OpenMP/cancellation_point_messages.cpp
===
--- test/OpenMP/cancellation_point_messages.cpp
+++ test/OpenMP/cancellation_point_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancellation point // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancellation point'}}
#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancellation point unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancellation point sections( // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point for, ) // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
#pragma omp cancellation point taskgroup() // expected-warning {{extra tokens at the end of '#pragma omp cancellation point' are ignored}}
Index: test/OpenMP/cancel_messages.cpp
===
--- test/OpenMP/cancel_messages.cpp
+++ test/OpenMP/cancel_messages.cpp
@@ -4,8 +4,16 @@
#pragma omp cancellation // expected-error {{expected an OpenMP directive}}
#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
;
+#pragma omp parallel
+ {
+#pragma omp cancel // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+ }
#pragma omp cancel parallel untied // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp cancel'}}
#pragma omp cancel unknown // expected-error {{one of 'for', 'parallel', 'sections' or 'taskgroup' is expected}}
+#pragma omp parallel
+ {
+#pragma omp cancel unknown // expected-error {{one of 'for',
