-
Notifications
You must be signed in to change notification settings - Fork 15.4k
default clause replaced by otherwise clause for metadirective in OpenMP 5.2 #128640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
6ebd599
5efa687
a3a8a41
185a9ad
24fe1cf
44a2539
f1b974c
09f39f1
7d0536d
cf0ff92
9712b74
64bc712
87ffc1a
8e63fb1
c3f1560
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1660,6 +1660,10 @@ def err_omp_expected_colon : Error<"missing ':' in %0">; | |
| def err_omp_missing_comma : Error< "missing ',' after %0">; | ||
| def err_omp_expected_context_selector | ||
| : Error<"expected valid context selector in %0">; | ||
| def err_omp_unknown_clause | ||
| : Error<"unknown clause '%0' in %1">; | ||
|
||
| def warn_omp_default_deprecated : Warning<"'default' clause for" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use err_omp_deprecate_old_syntax
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since default is being deprecated not disallowed in metadirective I had put a warning instead on using an error (err_omp_deprecate_old_syntax).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think better to add operands here and use them to allow reusing this warning for future changes
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the deprecated clauses have their own warning messages without operands.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be fixed, actually, better to have one message with arguments than many
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that consolidating into a single message with arguments would be a cleaner approach. For now, I’d appreciate it if we could proceed with this version, and I’ll take up the refactoring in a follow-up patch. |
||
| " 'metadirective' is deprecated; use 'otherwise' instead">, InGroup<Deprecated>; | ||
| def err_omp_requires_out_inout_depend_type : Error< | ||
| "reserved locator 'omp_all_memory' requires 'out' or 'inout' " | ||
| "dependency types">; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2759,6 +2759,19 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( | |
| OpenMPClauseKind CKind = Tok.isAnnotation() | ||
| ? OMPC_unknown | ||
| : getOpenMPClauseKind(PP.getSpelling(Tok)); | ||
| // Check if the clause is unrecognized. | ||
| if (getLangOpts().OpenMP < 52 && | ||
| (CKind == OMPC_unknown || CKind == OMPC_otherwise)) { | ||
| Diag(Tok, diag::err_omp_unknown_clause) | ||
| << PP.getSpelling(Tok) << "metadirective"; | ||
|
||
| } | ||
| if (getLangOpts().OpenMP >= 52 && CKind == OMPC_unknown) { | ||
| Diag(Tok, diag::err_omp_unknown_clause) | ||
| << PP.getSpelling(Tok) << "metadirective"; | ||
| } | ||
|
||
| if (CKind == OMPC_default && getLangOpts().OpenMP >= 52) { | ||
| Diag(Tok, diag::warn_omp_default_deprecated); | ||
| } | ||
|
||
| SourceLocation Loc = ConsumeToken(); | ||
|
|
||
| // Parse '('. | ||
|
|
@@ -2785,6 +2798,13 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective( | |
| return Directive; | ||
| } | ||
| } | ||
| if (CKind == OMPC_otherwise) { | ||
| // Check for 'otherwise' keyword. | ||
| if (Tok.is(tok::identifier) && | ||
| Tok.getIdentifierInfo()->getName() == "otherwise") { | ||
|
||
| ConsumeToken(); // Consume 'otherwise' | ||
| } | ||
|
||
| } | ||
| // Skip Directive for now. We will parse directive in the second iteration | ||
| int paren = 0; | ||
| while (Tok.isNot(tok::r_paren) || paren != 0) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,21 +2,48 @@ | |
|
|
||
| // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -verify -fopenmp-simd -x c++ -std=c++14 -fexceptions -fcxx-exceptions %s | ||
|
|
||
| // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -verify=expected,omp52 -fopenmp -fopenmp-version=52 -ferror-limit 100 -o - %s -Wuninitialized | ||
|
|
||
| void foo() { | ||
| #pragma omp metadirective // expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when() // expected-error {{expected valid context selector in when clause}} expected-error {{expected expression}} expected-warning {{expected identifier or string literal describing a context set; set skipped}} expected-note {{context set options are: 'construct' 'device' 'target_device' 'implementation' 'user'}} expected-note {{the ignored set spans until here}} | ||
| ; | ||
| #pragma omp metadirective when(device{}) // expected-warning {{expected '=' after the context set name "device"; '=' assumed}} expected-warning {{expected identifier or string literal describing a context selector; selector skipped}} expected-note {{context selector options are: 'kind' 'arch' 'isa'}} expected-note {{the ignored selector spans until here}} expected-error {{expected valid context selector in when clause}} expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when(device{arch(nvptx)}) // expected-error {{missing ':' in when clause}} expected-error {{expected expression}} expected-warning {{expected '=' after the context set name "device"; '=' assumed}} | ||
| ; | ||
| #pragma omp metadirective when(device{arch(nvptx)}: ) default() // expected-warning {{expected '=' after the context set name "device"; '=' assumed}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : ) default(xyz) // expected-error {{expected an OpenMP directive}} expected-error {{use of undeclared identifier 'xyz'}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : parallel default() // expected-error {{expected ',' or ')' in 'when' clause}} expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when(device = {isa("some-unsupported-feature")} : parallel) default(single) // expected-warning {{isa trait 'some-unsupported-feature' is not known to the current target; verify the spelling or consider restricting the context selector with the 'arch' selector further}} | ||
| ; | ||
| } | ||
| #if _OPENMP >= 202111 | ||
| #pragma omp metadirective // omp52-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when() // omp52-error {{expected valid context selector in when clause}} expected-error {{expected expression}} expected-warning {{expected identifier or string literal describing a context set; set skipped}} expected-note {{context set options are: 'construct' 'device' 'target_device' 'implementation' 'user'}} expected-note {{the ignored set spans until here}} | ||
| ; | ||
| #pragma omp metadirective when(device{}) // omp52-warning {{expected '=' after the context set name "device"; '=' assumed}} expected-warning {{expected identifier or string literal describing a context selector; selector skipped}} expected-note {{context selector options are: 'kind' 'arch' 'isa'}} expected-note {{the ignored selector spans until here}} expected-error {{expected valid context selector in when clause}} expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when(device{arch(nvptx)}) // omp52-error {{missing ':' in when clause}} expected-error {{expected expression}} expected-warning {{expected '=' after the context set name "device"; '=' assumed}} | ||
| ; | ||
| #pragma omp metadirective when(device{arch(nvptx)}: ) otherwise() // omp52-warning {{expected '=' after the context set name "device"; '=' assumed}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : ) otherwise(xyz) // omp52-error {{expected an OpenMP directive}} expected-error {{use of undeclared identifier 'xyz'}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : parallel otherwise() // omp52-error {{expected ',' or ')' in 'when' clause}} expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when(device = {isa("some-unsupported-feature")} : parallel) otherwise(single) // omp52-warning {{isa trait 'some-unsupported-feature' is not known to the current target; verify the spelling or consider restricting the context selector with the 'arch' selector further}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : parallel) default() // omp52-warning {{'default' clause for 'metadirective' is deprecated; use 'otherwise' instead}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : parallel) xyz() //omp52-error {{unknown clause 'xyz' in metadirective}} | ||
| ; | ||
| #else | ||
| #pragma omp metadirective // expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when() // expected-error {{expected valid context selector in when clause}} expected-error {{expected expression}} expected-warning {{expected identifier or string literal describing a context set; set skipped}} expected-note {{context set options are: 'construct' 'device' 'target_device' 'implementation' 'user'}} expected-note {{the ignored set spans until here}} | ||
| ; | ||
| #pragma omp metadirective when(device{}) // expected-warning {{expected '=' after the context set name "device"; '=' assumed}} expected-warning {{expected identifier or string literal describing a context selector; selector skipped}} expected-note {{context selector options are: 'kind' 'arch' 'isa'}} expected-note {{the ignored selector spans until here}} expected-error {{expected valid context selector in when clause}} expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when(device{arch(nvptx)}) // expected-error {{missing ':' in when clause}} expected-error {{expected expression}} expected-warning {{expected '=' after the context set name "device"; '=' assumed}} | ||
| ; | ||
| #pragma omp metadirective when(device{arch(nvptx)}: ) default() // expected-warning {{expected '=' after the context set name "device"; '=' assumed}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : ) default(xyz) // expected-error {{expected an OpenMP directive}} expected-error {{use of undeclared identifier 'xyz'}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : parallel default() // expected-error {{expected ',' or ')' in 'when' clause}} expected-error {{expected expression}} | ||
| ; | ||
| #pragma omp metadirective when(device = {isa("some-unsupported-feature")} : parallel) default(single) // expected-warning {{isa trait 'some-unsupported-feature' is not known to the current target; verify the spelling or consider restricting the context selector with the 'arch' selector further}} | ||
| ; | ||
| #pragma omp metadirective when(device = {arch(nvptx)} : parallel) xyz() //expected-error {{unknown clause 'xyz' in metadirective}} | ||
| ; | ||
| #endif | ||
| } | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use err_omp_expected_clause instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. I've already used
err_omp_unexpected_clausefor theOMPC_otherwisecase in OpenMP versions prior to 5.2, as suggested earlier. ForOMPC_unknown, I'm usingerr_omp_unknown_clause, which aligns with the guidance to treatOMPC_unknownandOMPC_otherwiseseparately.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is wrong. If the wrong clause is used, it should be reported automatically. If none was specified, err_omp_expected_clause should be used