Skip to content

[flang][OpenMP] Add --openmp-enable-delayed-privatization-staging flag #94749

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
Jun 7, 2024

Conversation

ergawy
Copy link
Member

@ergawy ergawy commented Jun 7, 2024

Adds a new flag for delayed privatization to differentiate between features for which delayed privatization is fully supported vs. features with partial support. This enables use roll out delayed privatization and test it in the wild for completely supported constructs while keeping it switched off for partially supported ones. Which allows us to discover the issues early on rather than wait for all constructs to be supported.

Adds a new flag for delayed privatization to differentiate between
features for which delayed privatization is fully supported vs. features
with partial support. This enables use roll out delayed privatization and
test it in the wild for completely supported constructs while keeping it
switched off for partially supported ones. Which allows us to discover the
issues early on rather than wait for all constructs to be supported.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Jun 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 7, 2024

@llvm/pr-subscribers-flang-openmp

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

Author: Kareem Ergawy (ergawy)

Changes

Adds a new flag for delayed privatization to differentiate between features for which delayed privatization is fully supported vs. features with partial support. This enables use roll out delayed privatization and test it in the wild for completely supported constructs while keeping it switched off for partially supported ones. Which allows us to discover the issues early on rather than wait for all constructs to be supported.


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

6 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+1-1)
  • (modified) flang/lib/Lower/OpenMP/Utils.cpp (+6)
  • (modified) flang/lib/Lower/OpenMP/Utils.h (+1)
  • (modified) flang/test/Lower/OpenMP/DelayedPrivatization/target-private-allocatable.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/DelayedPrivatization/target-private-simple.f90 (+2-2)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 184c43ff9fe91..6b391e11beb48 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1114,7 +1114,7 @@ static void genTargetClauses(
                                          llvm::omp::Directive::OMPD_target);
 
   // `target private(..)` is only supported in delayed privatization mode.
-  if (!enableDelayedPrivatization)
+  if (!enableDelayedPrivatizationStaging)
     cp.processTODO<clause::Private>(loc, llvm::omp::Directive::OMPD_target);
 }
 
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index da94352a84a7c..36d96f37ff36a 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -36,6 +36,12 @@ llvm::cl::opt<bool> enableDelayedPrivatization(
         "Emit `[first]private` variables as clauses on the MLIR ops."),
     llvm::cl::init(false));
 
+llvm::cl::opt<bool> enableDelayedPrivatizationStaging(
+    "openmp-enable-delayed-privatization-staging",
+    llvm::cl::desc("For partially supported constructs, emit `[first]private` "
+                   "variables as clauses on the MLIR ops."),
+    llvm::cl::init(false));
+
 namespace Fortran {
 namespace lower {
 namespace omp {
diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h
index d20f9187640f0..0b4fe9044bfa7 100644
--- a/flang/lib/Lower/OpenMP/Utils.h
+++ b/flang/lib/Lower/OpenMP/Utils.h
@@ -17,6 +17,7 @@
 
 extern llvm::cl::opt<bool> treatIndexAsSection;
 extern llvm::cl::opt<bool> enableDelayedPrivatization;
+extern llvm::cl::opt<bool> enableDelayedPrivatizationStaging;
 
 namespace fir {
 class FirOpBuilder;
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-allocatable.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-allocatable.f90
index 17a28c6a5ab7d..a27de1152ce17 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-allocatable.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-allocatable.f90
@@ -1,8 +1,8 @@
 ! Tests delayed privatization for `targets ... private(..)` for allocatables.
 
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization-staging \
 ! RUN:   -o - %s 2>&1 | FileCheck %s
-! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization-staging -o - %s 2>&1 \
 ! RUN:   | FileCheck %s
 
 subroutine target_allocatable
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
index 8682a420e89d9..6e8282b2af625 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
@@ -1,8 +1,8 @@
 ! Tests delayed privatization for `targets ... private(..)` for allocatables.
 
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization-staging \
 ! RUN:   -o - %s 2>&1 | FileCheck %s
-! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization-staging -o - %s 2>&1 \
 ! RUN:   | FileCheck %s
 
 subroutine target_allocatable(lb, ub, l)
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-simple.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-simple.f90
index 94edeaa5a7ef1..524e973780c49 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-simple.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-simple.f90
@@ -1,8 +1,8 @@
 ! Tests delayed privatization for `targets ... private(..)` for simple variables.
 
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization-staging \
 ! RUN:   -o - %s 2>&1 | FileCheck %s
-! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization-staging -o - %s 2>&1 \
 ! RUN:   | FileCheck %s
 
 subroutine target_simple

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

@ergawy ergawy merged commit 1539da4 into llvm:main Jun 7, 2024
11 checks passed
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
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