Skip to content

[flang][OpenMP] Fix bug in emitting dealloc logic #93641

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 2 commits into from
May 29, 2024

Conversation

ergawy
Copy link
Member

@ergawy ergawy commented May 29, 2024

Fixes a bug in emiting deacllocation logic when delayed privatization is disabled. I introduced the bug when implementing delayed privatization for allocatables: when delayed privatization is disabled the deacllocation ops are emitted for only one allocatable variables.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels May 29, 2024
@llvmbot
Copy link
Member

llvmbot commented May 29, 2024

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Kareem Ergawy (ergawy)

Changes

Fixes a bug in emiting deacllocation logic when delayed privatization is disabled. I introduced the bug when implementing delayed privatization for allocatables: when delayed privatization is disabled the deacllocation ops are emitted for only one allocatable variables.


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

2 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+1-1)
  • (added) flang/test/Lower/OpenMP/allocatable-multiple-vars.f90 (+25)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index b722e19272ca1..557a9685024c5 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -86,7 +86,7 @@ void DataSharingProcessor::insertDeallocs() {
     if (semantics::IsAllocatable(sym->GetUltimate())) {
       if (!useDelayedPrivatization) {
         converter.createHostAssociateVarCloneDealloc(*sym);
-        return;
+        continue;
       }
 
       lower::SymbolBox hsb = converter.lookupOneLevelUpSymbol(*sym);
diff --git a/flang/test/Lower/OpenMP/allocatable-multiple-vars.f90 b/flang/test/Lower/OpenMP/allocatable-multiple-vars.f90
new file mode 100644
index 0000000000000..325d3d2c59bc7
--- /dev/null
+++ b/flang/test/Lower/OpenMP/allocatable-multiple-vars.f90
@@ -0,0 +1,25 @@
+! Test delayed privatization for multiple allocatable variables.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization=false \
+! RUN:   -o - %s 2>&1 | FileCheck %s
+
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization=false -o - %s 2>&1 |\
+! RUN:   FileCheck %s
+
+subroutine delayed_privatization_allocatable
+  implicit none
+  integer, allocatable :: var1, var2
+
+!$omp parallel private(var1, var2)
+  var1 = 10
+  var2 = 20
+!$omp end parallel
+end subroutine
+
+! CHECK:      omp.parallel {
+! CHECK:        fir.allocmem
+! CHECK:        fir.allocmem
+! CHECK:        fir.freemem
+! CHECK:        fir.freemem
+! CHECK:        omp.terminator
+! CHECK-NEXT: }

Fixes a bug in emiting deacllocation logic when delayed privatization is
disabled. I introduced the bug when implementing delayed privatization
for allocatables: when delayed privatization is disabled the
deacllocation ops are emitted for only one allocatable variables.
@ergawy ergawy force-pushed the delayed_privatization_24 branch from 91c042e to 344cb00 Compare May 29, 2024 04:14
Copy link
Contributor

@NimishMishra NimishMishra left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. The change looks good.

@kiranchandramohan
Copy link
Contributor

Thanks for the fix. The change looks good.

Please approve if it is fine.

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

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

LG.

Co-authored-by: Kiran Chandramohan <[email protected]>
@ergawy ergawy merged commit e1aa8ad into llvm:main May 29, 2024
7 checks passed
vg0204 pushed a commit to vg0204/llvm-project that referenced this pull request May 29, 2024
Fixes a bug in emiting deacllocation logic when delayed privatization is
disabled. I introduced the bug when implementing delayed privatization
for allocatables: when delayed privatization is disabled the
deacllocation ops are emitted for only one allocatable variables.
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 Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants