-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[flang][OpenMP] Add semantics test: named COMMON + member with firstprivate+lastprivate is valid #162234
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
Conversation
@tblah didn't knew who to tag so tagged you(as related to flang just like the one you merged earlier), this PR is waiting to be reviewed and CI is all green and good to merge!! |
@@ -0,0 +1,13 @@ | |||
! RUN: %flang_fc1 -fopenmp -fsyntax-only %s 2>&1 | FileCheck %s --allow-empty |
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.
Please could you add -fopenmp-version=60 (or whichever version was the first to allow this) so that this test does not fail once the required check is added for older openmp versions.
a6d387e
to
36e9310
Compare
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Krish Gupta (KrxGu) ChangesThis adds a positive semantics test showing that:
The reporter example in #162033 therefore conforms to OpenMP and Flang is correct to accept it. This test documents and locks in that behavior to avoid regressions. Full diff: https://github.com/llvm/llvm-project/pull/162234.diff 1 Files Affected:
diff --git a/flang/test/Semantics/OpenMP/omp-common-fp-lp.f90 b/flang/test/Semantics/OpenMP/omp-common-fp-lp.f90
new file mode 100644
index 0000000000000..ed52e76ca62ce
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/omp-common-fp-lp.f90
@@ -0,0 +1,21 @@
+! RUN: %flang_fc1 -fopenmp -fopenmp-version=51 -fsyntax-only %s 2>&1 | FileCheck %s --allow-empty
+! CHECK-NOT: error:
+! CHECK-NOT: warning:
+
+! Regression test for issue #162033.
+! Verify that a named COMMON block can appear in a data-sharing clause together
+! with one of its members in another clause on the same construct. This is valid
+! in OpenMP >= 5.1 because:
+! - A named COMMON in a clause is equivalent to listing all its explicit members
+! - A list item may appear in both FIRSTPRIVATE and LASTPRIVATE on the same directive
+! OpenMP 5.0 explicitly forbade this combination.
+
+subroutine sub1()
+ common /com/ j
+ j = 10
+!$omp parallel do firstprivate(j) lastprivate(/com/)
+ do i = 1, 10
+ j = j + 1
+ end do
+!$omp end parallel do
+end
|
36e9310
to
346c4c0
Compare
…tprivate is valid Gate test to OpenMP 5.1+ and add explanatory comment per review feedback.
346c4c0
to
7d0ee79
Compare
@tblah Thanks for the review! I've addressed both comments:
Note on CI fix: I also removed the CI is all green, good to merge!! |
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! Would you like help merging?
Good to me
@tblah Yes, please! That would be great. Thank you for the review and approval! 🙏 |
This adds a positive semantics test showing that:
The reporter example in #162033 therefore conforms to OpenMP and Flang is correct to accept it. This test documents and locks in that behavior to avoid regressions.
