Skip to content
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

[flang][openacc] Use the same iv privatized value in the loop region #81821

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

clementval
Copy link
Contributor

IV variable are privatized during acc loop lowering. An hlfir.declare operation is added when mapping the symbol to the new private value. In order to avoid using multiple value in the acc.loop region, we map the symbol to the result of the hlfir.declare operation inserted.

IV variable are privatized during acc loop lowering. An hlfir.declare
operation is added when mapping the symbol to the new private value.
In order to avoid using multiple value in the acc.loop region, we map
the symbol to the result of the hlfir.declare operation inserted.
@clementval
Copy link
Contributor Author

@rjnw

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir openacc labels Feb 15, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 15, 2024

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

@llvm/pr-subscribers-openacc

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

IV variable are privatized during acc loop lowering. An hlfir.declare operation is added when mapping the symbol to the new private value. In order to avoid using multiple value in the acc.loop region, we map the symbol to the result of the hlfir.declare operation inserted.


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

2 Files Affected:

  • (modified) flang/lib/Lower/OpenACC.cpp (+9-1)
  • (modified) flang/test/Lower/OpenACC/acc-private.f90 (+19)
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 6ae270f63f5cf4..446b1529ca0088 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -1658,10 +1658,18 @@ static void privatizeIv(Fortran::lower::AbstractConverter &converter,
       mlir::acc::DataClause::acc_private, ivValue.getType());
 
   privateOperands.push_back(op.getAccPtr());
-  ivPrivate.push_back(op.getAccPtr());
   privatizations.push_back(mlir::SymbolRefAttr::get(builder.getContext(),
                                                     recipe.getSymName().str()));
+
+  // Map the new private iv to its symbol for the scope of the loop. bindSymbol
+  // might create a hlfir.declare op, if so, we map its result in order to
+  // use the sym value in the scope.
   converter.bindSymbol(sym, op.getAccPtr());
+  auto privateValue = converter.getSymbolAddress(sym);
+  if (auto declareOp =
+          mlir::dyn_cast<hlfir::DeclareOp>(privateValue.getDefiningOp()))
+    privateValue = declareOp.getResults()[0];
+  ivPrivate.push_back(privateValue);
 }
 
 static mlir::acc::LoopOp
diff --git a/flang/test/Lower/OpenACC/acc-private.f90 b/flang/test/Lower/OpenACC/acc-private.f90
index 29074cbaedc910..ba582d40f0f512 100644
--- a/flang/test/Lower/OpenACC/acc-private.f90
+++ b/flang/test/Lower/OpenACC/acc-private.f90
@@ -382,3 +382,22 @@ subroutine sub1()
 end module
 
 ! CHECK: acc.parallel private(@privatization_ref_10xf32 -> %{{.*}} : !fir.ref<!fir.array<10xf32>>)
+
+subroutine acc_private_use()
+  integer :: i, j
+
+  !$acc parallel loop
+  do i = 1, 10
+    j = i
+  end do
+end
+
+! CHECK-LABEL: func.func @_QPacc_private_use()
+! CHECK: %[[I:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFacc_private_useEi"}
+! CHECK: %[[DECL_I:.*]]:2 = hlfir.declare %0 {uniq_name = "_QFacc_private_useEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK: acc.parallel
+! CHECK: %[[PRIV_I:.*]] = acc.private varPtr(%[[DECL_I]]#1 : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = ""}
+! CHECK: %[[DECL_PRIV_I:.*]]:2 = hlfir.declare %[[PRIV_I]] {uniq_name = "_QFacc_private_useEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK: acc.loop private(@privatization_ref_i32 -> %[[PRIV_I]] : !fir.ref<i32>) control(%[[IV0:.*]] : i32) = (%c1{{.*}} : i32) to (%c10{{.*}} : i32) step (%c1{{.*}} : i32)
+! CHECK:   fir.store %[[IV0]] to %[[DECL_PRIV_I]]#0 : !fir.ref<i32>
+! CHECK:   %{{.*}} = fir.load %[[DECL_PRIV_I]]#0 : !fir.ref<i32>

@clementval clementval changed the title [flang][openacc] Use the same iv privatize value in the loop region [flang][openacc] Use the same iv privatized value in the loop region Feb 15, 2024
Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

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

Thank you!

@rjnw
Copy link
Contributor

rjnw commented Feb 15, 2024

This is perfect, thank you!

@clementval clementval merged commit 135529a into llvm:main Feb 20, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants