[Flang][OpenMP] Improve Semantics for Derived Type Array Elements #167296
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flang does not allow the use of Structure Component types inside of certain OpenMP clauses. While this has been introduced, it seemed that Structure Component Array Elements were not being captured as they got embedded in the parse tree. To ensure all structure component types are identified, a new `HasStructureComponent` evaluate function has been introduced to walk a Semantics expression, identified where Components are used within. This replaces the previous implementation of `CheckStructureComponent` which just looked for the StructureComponent inside of a DataRef. Fixes llvm#150830
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-semantics Author: Jack Styles (Stylie777) ChangesFlang does not allow the use of Structure Component types inside of certain OpenMP clauses. While this has been introduced, it seemed that Structure Component Array Elements were not being captured as they got embedded in the parse tree. To ensure all structure component types are identified, a new This replaces the previous implementation of Fixes #150830 Full diff: https://github.com/llvm/llvm-project/pull/167296.diff 7 Files Affected:
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 7f64d230f7348..4248e3a5461f5 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -1110,6 +1110,9 @@ bool IsArraySection(const Expr<SomeType> &expr);
// Predicate: does an expression contain constant?
bool HasConstant(const Expr<SomeType> &);
+// Predicate: Does an expression contain a component
+bool HasStructureComponent(const Expr<SomeType> &expr);
+
// Utilities for attaching the location of the declaration of a symbol
// of interest to a message. Handles the case of USE association gracefully.
parser::Message *AttachDeclaration(parser::Message &, const Symbol &);
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index bd06acc21e47f..a26c862d00dba 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1210,6 +1210,27 @@ bool HasConstant(const Expr<SomeType> &expr) {
return HasConstantHelper{}(expr);
}
+// HasStructureComponent()
+struct HasStructureComponentHelper : public AnyTraverse<HasStructureComponentHelper, bool, false> {
+ using Base = AnyTraverse<HasStructureComponentHelper, bool, false>;
+ HasStructureComponentHelper() : Base(*this) {}
+ using Base::operator();
+
+ bool operator()(const DataRef &dataRef) const {
+ return std::holds_alternative<Component>(dataRef.u);
+ }
+ bool operator()(const NamedEntity &NamedEntity) const {
+ return NamedEntity.UnwrapComponent() != nullptr;
+ }
+ bool operator()(const Component &) const {
+ return true;
+ }
+};
+
+bool HasStructureComponent(const Expr<SomeType> &expr) {
+ return HasStructureComponentHelper{}(expr);
+}
+
parser::Message *AttachDeclaration(
parser::Message &message, const Symbol &symbol) {
const Symbol *unhosted{&symbol};
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index d7db15dd37949..5bc644bd4e0f6 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -4673,12 +4673,13 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) {
void OmpStructureChecker::CheckStructureComponent(
const parser::OmpObjectList &objects, llvm::omp::Clause clauseId) {
auto CheckComponent{[&](const parser::Designator &designator) {
- if (auto *dataRef{std::get_if<parser::DataRef>(&designator.u)}) {
+ if (const parser::DataRef *dataRef{std::get_if<parser::DataRef>(&designator.u)}) {
if (!IsDataRefTypeParamInquiry(dataRef)) {
- if (auto *comp{parser::Unwrap<parser::StructureComponent>(*dataRef)}) {
- context_.Say(comp->component.source,
- "A variable that is part of another variable cannot appear on the %s clause"_err_en_US,
- parser::ToUpperCaseLetters(getClauseName(clauseId).str()));
+ const auto expr = AnalyzeExpr(context_, designator);
+ if (expr.has_value() && evaluate::HasStructureComponent(expr.value())) {
+ context_.Say(designator.source,
+ "A variable that is part of another variable cannot appear on the %s clause"_err_en_US,
+ parser::ToUpperCaseLetters(getClauseName(clauseId).str()));
}
}
}
diff --git a/flang/test/Semantics/OpenMP/in-reduction.f90 b/flang/test/Semantics/OpenMP/in-reduction.f90
index 1b82134b7104b..3f1e735214061 100644
--- a/flang/test/Semantics/OpenMP/in-reduction.f90
+++ b/flang/test/Semantics/OpenMP/in-reduction.f90
@@ -47,6 +47,7 @@ subroutine f06
integer :: a(10)
end type
type(t) :: x
+!ERROR: A variable that is part of another variable cannot appear on the IN_REDUCTION clause
!ERROR: The base expression of an array element or section in IN_REDUCTION clause must be an identifier
!$omp target in_reduction(+: x%a(2))
!$omp end target
@@ -57,6 +58,7 @@ subroutine f07
integer :: a(10)
end type
type(t) :: x
+!ERROR: A variable that is part of another variable cannot appear on the IN_REDUCTION clause
!ERROR: The base expression of an array element or section in IN_REDUCTION clause must be an identifier
!$omp target in_reduction(+: x%a(1:10))
!$omp end target
diff --git a/flang/test/Semantics/OpenMP/reduction15.f90 b/flang/test/Semantics/OpenMP/reduction15.f90
index 1d4de6ff702bb..61fa417f1111c 100644
--- a/flang/test/Semantics/OpenMP/reduction15.f90
+++ b/flang/test/Semantics/OpenMP/reduction15.f90
@@ -13,6 +13,7 @@ module m
subroutine f00
type(t) :: x
+ !ERROR: A variable that is part of another variable cannot appear on the REDUCTION clause
!ERROR: The base expression of an array element or section in REDUCTION clause must be an identifier
!$omp do reduction (+ : x%a(2))
do i = 1, 10
@@ -22,6 +23,7 @@ subroutine f00
subroutine f01
type(t) :: x
+ !ERROR: A variable that is part of another variable cannot appear on the REDUCTION clause
!ERROR: The base expression of an array element or section in REDUCTION clause must be an identifier
!$omp do reduction (+ : x%a(1:10))
do i = 1, 10
diff --git a/flang/test/Semantics/OpenMP/reduction17.f90 b/flang/test/Semantics/OpenMP/reduction17.f90
new file mode 100644
index 0000000000000..5b6e8e977f46c
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/reduction17.f90
@@ -0,0 +1,18 @@
+! Test that Structure Component Array Elements are caught by Semantics and return an error
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=45
+
+type test_type
+ integer :: array(2)
+end type
+
+contains
+ subroutine test
+ type(test_type) :: x
+
+ !ERROR: A variable that is part of another variable cannot appear on the REDUCTION clause
+ !$omp do reduction(+: x%array(2))
+ do i=1, 2
+ end do
+ !$omp end do
+ end subroutine
+end
diff --git a/flang/test/Semantics/OpenMP/task-reduction.f90 b/flang/test/Semantics/OpenMP/task-reduction.f90
index 5a18ee48e7728..f76b07ae568f4 100644
--- a/flang/test/Semantics/OpenMP/task-reduction.f90
+++ b/flang/test/Semantics/OpenMP/task-reduction.f90
@@ -47,6 +47,7 @@ subroutine f06
integer :: a(10)
end type
type(t) :: x
+!ERROR: A variable that is part of another variable cannot appear on the TASK_REDUCTION clause
!ERROR: The base expression of an array element or section in TASK_REDUCTION clause must be an identifier
!$omp taskgroup task_reduction(+: x%a(2))
!$omp end taskgroup
@@ -57,6 +58,7 @@ subroutine f07
integer :: a(10)
end type
type(t) :: x
+!ERROR: A variable that is part of another variable cannot appear on the TASK_REDUCTION clause
!ERROR: The base expression of an array element or section in TASK_REDUCTION clause must be an identifier
!$omp taskgroup task_reduction(+: x%a(1:10))
!$omp end taskgroup
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
tblah
reviewed
Nov 10, 2025
| bool operator()(const NamedEntity &NamedEntity) const { | ||
| return NamedEntity.UnwrapComponent() != nullptr; | ||
| } | ||
| bool operator()(const Component &) const { return true; } |
Contributor
There was a problem hiding this comment.
Do we need the other two visitors or will it still catch them here after one more traversal?
Contributor
Author
There was a problem hiding this comment.
Yes we can remove these, updated.
| if (!IsDataRefTypeParamInquiry(dataRef)) { | ||
| if (auto *comp{parser::Unwrap<parser::StructureComponent>(*dataRef)}) { | ||
| context_.Say(comp->component.source, | ||
| const auto expr = AnalyzeExpr(context_, designator); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flang does not allow the use of Structure Component types inside of certain OpenMP clauses. While this has been introduced, it seemed that Structure Component Array Elements were not being captured as they got embedded in the parse tree. To ensure all structure component types are identified, a new
HasStructureComponentevaluate function has been introduced to walk a Semantics expression, identified where Components are used within.This replaces the previous implementation of
CheckStructureComponentwhich just looked for the StructureComponent inside of a DataRef.Fixes #150830