Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

default clause replaced by otherwise clause for metadirective in OpenMP 5.2 #128640

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6ebd599
default clause replaced by otherwise clause for metadirective
ravurvi20 Feb 25, 2025
5efa687
Merge branch 'llvm:main' into otherwise-clause
ravurvi20 Mar 24, 2025
a3a8a41
Merge branch 'llvm:main' into otherwise-clause
ravurvi20 Mar 25, 2025
185a9ad
Merge branch 'llvm:main' into otherwise-clause
ravurvi20 Mar 26, 2025
24fe1cf
Update ParseOpenMP.cpp and error messages
Apr 2, 2025
44a2539
Update LIT test cases
Apr 2, 2025
f1b974c
Updated ParseOpenMP.cpp
Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

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

Copy link
Contributor Author

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_clause for the OMPC_otherwise case in OpenMP versions prior to 5.2, as suggested earlier. For OMPC_unknown, I'm using err_omp_unknown_clause, which aligns with the guidance to treat OMPC_unknown and OMPC_otherwise separately.

Copy link
Member

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

: Error<"unknown clause '%0' in %1">;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a message for a wrong clause for a directive: err_omp_unexpected_clause. This does not apply when we have some unknown string where a clause is expected.

def warn_omp_default_deprecated : Warning<"'default' clause for"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use err_omp_deprecate_old_syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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).

" '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">;
Expand Down
20 changes: 20 additions & 0 deletions clang/lib/Parse/ParseOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error message does not accept any parameters

}
if (getLangOpts().OpenMP >= 52 && CKind == OMPC_unknown) {
Diag(Tok, diag::err_omp_unknown_clause)
<< PP.getSpelling(Tok) << "metadirective";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please treat OMPC_unknown and OMPC_otherwise separately: in the former case the message should be something like "expected an OpenMP clause" (see err_omp_unknown_directive for reference). The latter case should use err_omp_unexpected_clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kparzysz I have made the requested modifications to the code based on your suggestions. Please review it and provide your feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @kparzysz

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @kparzysz

if (CKind == OMPC_default && getLangOpts().OpenMP >= 52) {
Diag(Tok, diag::warn_omp_default_deprecated);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop braces

SourceLocation Loc = ConsumeToken();

// Parse '('.
Expand All @@ -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") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why need to double check here?

ConsumeToken(); // Consume 'otherwise'
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop braces

}
// Skip Directive for now. We will parse directive in the second iteration
int paren = 0;
while (Tok.isNot(tok::r_paren) || paren != 0) {
Expand Down
61 changes: 44 additions & 17 deletions clang/test/OpenMP/metadirective_messages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading