-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[flang][OpenMP] Handle pointers and allocatables in clone init #121824
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
InitializeClone(), implemented in llvm#120295, was not handling top level pointers and allocatables correctly. Pointers and unallocated variables must be skipped. This caused some regressions in the Fujitsu testsuite: https://linaro.atlassian.net/browse/LLVM-1488
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-runtime Author: Leandro Lupori (luporl) ChangesInitializeClone(), implemented in #120295, was not handling top This caused some regressions in the Fujitsu testsuite: https://linaro.atlassian.net/browse/LLVM-1488 Full diff: https://github.com/llvm/llvm-project/pull/121824.diff 3 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index cd312537551eab..9dfdbd8337ae91 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -126,7 +126,8 @@ void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) {
assert(sb);
mlir::Value addr = sb.getAddr();
assert(addr);
- return hlfir::mayHaveAllocatableComponent(addr.getType());
+ return !fir::isPointerType(addr.getType()) &&
+ hlfir::mayHaveAllocatableComponent(addr.getType());
};
if (needInitClone()) {
diff --git a/flang/runtime/derived.cpp b/flang/runtime/derived.cpp
index 7c164ff8904520..10813c62e5da1f 100644
--- a/flang/runtime/derived.cpp
+++ b/flang/runtime/derived.cpp
@@ -129,6 +129,10 @@ RT_API_ATTRS int InitializeClone(const Descriptor &clone,
std::size_t elements{orig.Elements()};
int stat{StatOk};
+ // Skip pointers and unallocated variables.
+ if (orig.IsPointer() || !orig.IsAllocated()) {
+ return stat;
+ }
// Initialize each data component.
std::size_t components{componentDesc.Elements()};
for (std::size_t i{0}; i < components; ++i) {
diff --git a/flang/test/Lower/OpenMP/derived-type-allocatable.f90 b/flang/test/Lower/OpenMP/derived-type-allocatable.f90
index d265954ef1ce1e..2dc4e20f27af21 100644
--- a/flang/test/Lower/OpenMP/derived-type-allocatable.f90
+++ b/flang/test/Lower/OpenMP/derived-type-allocatable.f90
@@ -13,6 +13,10 @@ module m1
contains
+!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_pointer
+!CHECK-NOT: fir.call @_FortranAInitializeClone
+!CHECK: omp.yield
+
!CHECK-LABEL: omp.private {type = private} @_QMm1Ftest_nested
!CHECK: fir.call @_FortranAInitializeClone
!CHECK-NEXT: omp.yield
@@ -91,4 +95,11 @@ subroutine test_nested()
!$omp parallel private(d2)
!$omp end parallel
end subroutine
+
+ subroutine test_pointer()
+ type(x), pointer :: ptr
+
+ !$omp parallel private(ptr)
+ !$omp end parallel
+ end subroutine
end module
|
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
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!
InitializeClone(), implemented in #120295, was not handling top
level pointers and allocatables correctly.
Pointers and unallocated variables must be skipped.
This caused some regressions in the Fujitsu testsuite: https://linaro.atlassian.net/browse/LLVM-1488