-
Notifications
You must be signed in to change notification settings - Fork 14k
[Flang][OpenMP] Update relevant warnings to emit when OMP >= v5.2 #144492
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
base: main
Are you sure you want to change the base?
Conversation
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.
Maybe IssueNonConformanceWarning
should have an argument specifying the minimum version to warn on? That way this can not be forgotten in the future
Thanks for the suggestion @tblah, I had avoided doing the version check inside the function to avoid a potentially needed function call, but this approach would be more robust so I will rework the implementation to do this. |
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.
LGTM with nits fixed. Previous discussion was here: #142378 (review)
Edit: see failing test - this does look relevant.
void IssueNonConformanceWarning( | ||
llvm::omp::Directive D, parser::CharBlock source); | ||
void IssueNonConformanceWarning(llvm::omp::Directive D, | ||
parser::CharBlock source, unsigned emit_from_version); |
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.
parser::CharBlock source, unsigned emit_from_version); | |
parser::CharBlock source, unsigned emitFromVersion); |
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.
Fixed
void OmpAttributeVisitor::IssueNonConformanceWarning( | ||
llvm::omp::Directive D, parser::CharBlock source) { | ||
void OmpAttributeVisitor::IssueNonConformanceWarning(llvm::omp::Directive D, | ||
parser::CharBlock source, unsigned emit_from_version) { |
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.
parser::CharBlock source, unsigned emit_from_version) { | |
parser::CharBlock source, unsigned emitFromVersion) { |
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.
Fixed
There has been a number of deprecation warnings that have been added to Flang, however these features are only deprecated when the OpenMP Version being used is 5.2 or later. Previously, flang did not consider the version with the warnings so would always be emitted. Flang now ensures warnings are emitted for the appropriate version of OpenMP, and tests are updated to reflect this change.
23ab4b3
to
2846d90
Compare
The test failure is related yes, new test that was added while I was away so this has been updated. |
There has been a number of deprecation warnings that have been added to Flang, however these features are only deprecated when the OpenMP Version being used is 5.2 or later. Previously, flang did not consider the version with the warnings so would always be emitted.
Flang now ensures warnings are emitted for the appropriate version of OpenMP, and tests are updated to reflect this change.