Skip to content

[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

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

luporl
Copy link
Contributor

@luporl luporl commented Jan 6, 2025

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

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
@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Jan 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 6, 2025

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-runtime

Author: Leandro Lupori (luporl)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/121824.diff

3 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+2-1)
  • (modified) flang/runtime/derived.cpp (+4)
  • (modified) flang/test/Lower/OpenMP/derived-type-allocatable.f90 (+11)
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

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Thanks!

@luporl luporl merged commit 5130a4e into llvm:main Jan 7, 2025
13 checks passed
@luporl luporl deleted the luporl-fix-regressions branch January 7, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang:runtime flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants