Skip to content

[flang] Enable delayed localization by default for do concurrent #142567

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 11, 2025

Conversation

ergawy
Copy link
Member

@ergawy ergawy commented Jun 3, 2025

This PR aims to make it easier and more self-contained to revert the switch/flag if we discover any problems with enabling it by default.

@ergawy ergawy requested a review from tblah June 3, 2025 09:40
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jun 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 3, 2025

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

Author: Kareem Ergawy (ergawy)

Changes

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

6 Files Affected:

  • (modified) flang/lib/Lower/Bridge.cpp (+1-4)
  • (modified) flang/test/Lower/do_concurrent_delayed_locality.f90 (+1-1)
  • (modified) flang/test/Lower/do_concurrent_local_assoc_entity.f90 (+1-1)
  • (modified) flang/test/Lower/do_concurrent_local_default_init.f90 (+1-1)
  • (modified) flang/test/Lower/loops.f90 (+1-1)
  • (modified) flang/test/Lower/loops3.f90 (+1-1)
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 2ea838673dd21..7844278ed80e3 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2031,11 +2031,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     fir::LocalitySpecifierOperands privateClauseOps;
     auto doConcurrentLoopOp =
         mlir::dyn_cast_if_present<fir::DoConcurrentLoopOp>(info.loopOp);
-    // TODO Promote to using `enableDelayedPrivatization` (which is enabled by
-    // default unlike the staging flag) once the implementation of this is more
-    // complete.
     bool useDelayedPriv =
-        enableDelayedPrivatizationStaging && doConcurrentLoopOp;
+        enableDelayedPrivatization && doConcurrentLoopOp;
     llvm::SetVector<const Fortran::semantics::Symbol *> allPrivatizedSymbols;
     llvm::SmallSet<const Fortran::semantics::Symbol *, 16> mightHaveReadHostSym;
 
diff --git a/flang/test/Lower/do_concurrent_delayed_locality.f90 b/flang/test/Lower/do_concurrent_delayed_locality.f90
index 6cae0eb46db13..039b17808d19e 100644
--- a/flang/test/Lower/do_concurrent_delayed_locality.f90
+++ b/flang/test/Lower/do_concurrent_delayed_locality.f90
@@ -1,4 +1,4 @@
-! RUN: %flang_fc1 -emit-hlfir -mmlir --enable-delayed-privatization-staging=true -o - %s | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
 
 subroutine do_concurrent_with_locality_specs
   implicit none
diff --git a/flang/test/Lower/do_concurrent_local_assoc_entity.f90 b/flang/test/Lower/do_concurrent_local_assoc_entity.f90
index 6c85ef0ec7595..7748b63d1f383 100644
--- a/flang/test/Lower/do_concurrent_local_assoc_entity.f90
+++ b/flang/test/Lower/do_concurrent_local_assoc_entity.f90
@@ -1,4 +1,4 @@
-! RUN: %flang_fc1 -emit-hlfir -mmlir --enable-delayed-privatization-staging=true -o - %s | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
 
 subroutine local_assoc
   implicit none
diff --git a/flang/test/Lower/do_concurrent_local_default_init.f90 b/flang/test/Lower/do_concurrent_local_default_init.f90
index d643213854744..798cbb335c8c0 100644
--- a/flang/test/Lower/do_concurrent_local_default_init.f90
+++ b/flang/test/Lower/do_concurrent_local_default_init.f90
@@ -1,5 +1,5 @@
 ! Test default initialization of DO CONCURRENT LOCAL() entities.
-! RUN: bbc -emit-hlfir --enable-delayed-privatization-staging=true -I nowhere -o - %s | FileCheck %s
+! RUN: bbc -emit-hlfir -I nowhere -o - %s | FileCheck %s
 
 subroutine test_ptr(p)
   interface
diff --git a/flang/test/Lower/loops.f90 b/flang/test/Lower/loops.f90
index 60df27a591dc3..64f14ff972272 100644
--- a/flang/test/Lower/loops.f90
+++ b/flang/test/Lower/loops.f90
@@ -1,4 +1,4 @@
-! RUN: bbc -emit-fir -hlfir=false -o - %s | FileCheck %s
+! RUN: bbc -emit-fir -hlfir=false --enable-delayed-privatization=false -o - %s | FileCheck %s
 
 ! CHECK-LABEL: loop_test
 subroutine loop_test
diff --git a/flang/test/Lower/loops3.f90 b/flang/test/Lower/loops3.f90
index 84db1972cca16..34d7bcfb7d7ad 100644
--- a/flang/test/Lower/loops3.f90
+++ b/flang/test/Lower/loops3.f90
@@ -1,5 +1,5 @@
 ! Test do concurrent reduction
-! RUN: bbc -emit-fir -hlfir=false -o - %s | FileCheck %s
+! RUN: bbc -emit-fir -hlfir=false --enable-delayed-privatization=false -o - %s | FileCheck %s
 
 ! CHECK-LABEL: loop_test
 subroutine loop_test

Copy link

github-actions bot commented Jun 3, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_2 branch from 47ad6cc to 345406e Compare June 3, 2025 09:42
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_1 branch from 8f5a70a to 8c8d15a Compare June 3, 2025 12:06
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_2 branch from 345406e to 5647d02 Compare June 3, 2025 12:07
@ergawy ergawy requested a review from bhandarkar-pranav June 4, 2025 07:44
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_1 branch from 8c8d15a to 1632048 Compare June 4, 2025 14:53
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_2 branch from 5647d02 to fd943c2 Compare June 4, 2025 14:57
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_1 branch from ba6ac8f to 148fdcf Compare June 4, 2025 18:31
Base automatically changed from users/ergawy/convert_locality_specs_to_clauses_1 to main June 4, 2025 23:01
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_2 branch from fd943c2 to 445db39 Compare June 5, 2025 13:27
@ergawy ergawy force-pushed the users/ergawy/convert_locality_specs_to_clauses_2 branch from 445db39 to baaf8b2 Compare June 11, 2025 07:18
@ergawy ergawy merged commit 937be17 into main Jun 11, 2025
7 checks passed
@ergawy ergawy deleted the users/ergawy/convert_locality_specs_to_clauses_2 branch June 11, 2025 08:10
@ergawy
Copy link
Member Author

ergawy commented Jun 11, 2025

Looks like this broke the gfortran test suite. I am looking into it ...

@ergawy
Copy link
Member Author

ergawy commented Jun 11, 2025

Looks like this broke the gfortran test suite. I am looking into it ...

#143687 hopefully resolves the build issue.

rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
…lvm#142567)

This PR aims to make it easier and more self-contained to revert the
switch/flag if we discover any problems with enabling it by default.
ergawy added a commit that referenced this pull request Jun 12, 2025
`fir.local` ops are not supposed to have any uses at this point (i.e.
during lowering to LLVM). In case of serialization, the
`fir.do_concurrent` users are expected to have been lowered to
`fir.do_loop` nests. In case of parallelization, the `fir.do_concurrent`
users are expected to have been lowered to the target parallel model
(e.g. OpenMP).

This hopefully resolved a build issue introduced by
#142567 (see for example:
https://lab.llvm.org/buildbot/#/builders/199/builds/4009).
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 12, 2025
…vm` (#143687)

`fir.local` ops are not supposed to have any uses at this point (i.e.
during lowering to LLVM). In case of serialization, the
`fir.do_concurrent` users are expected to have been lowered to
`fir.do_loop` nests. In case of parallelization, the `fir.do_concurrent`
users are expected to have been lowered to the target parallel model
(e.g. OpenMP).

This hopefully resolved a build issue introduced by
llvm/llvm-project#142567 (see for example:
https://lab.llvm.org/buildbot/#/builders/199/builds/4009).
ergawy added a commit that referenced this pull request Jun 12, 2025
…rent` (#142567)"

This reverts commit 937be17.

Resolves #143897 until the
todo is properly handled.
ergawy added a commit that referenced this pull request Jun 12, 2025
…rent` (#142567)" (#143905)

This reverts commit 937be17.

Resolves #143897 until the
todo is properly handled.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…lvm#142567)

This PR aims to make it easier and more self-contained to revert the
switch/flag if we discover any problems with enabling it by default.
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…43687)

`fir.local` ops are not supposed to have any uses at this point (i.e.
during lowering to LLVM). In case of serialization, the
`fir.do_concurrent` users are expected to have been lowered to
`fir.do_loop` nests. In case of parallelization, the `fir.do_concurrent`
users are expected to have been lowered to the target parallel model
(e.g. OpenMP).

This hopefully resolved a build issue introduced by
llvm#142567 (see for example:
https://lab.llvm.org/buildbot/#/builders/199/builds/4009).
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
…rent` (llvm#142567)" (llvm#143905)

This reverts commit 937be17.

Resolves llvm#143897 until the
todo is properly handled.
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
…lvm#142567)

This PR aims to make it easier and more self-contained to revert the
switch/flag if we discover any problems with enabling it by default.
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
…43687)

`fir.local` ops are not supposed to have any uses at this point (i.e.
during lowering to LLVM). In case of serialization, the
`fir.do_concurrent` users are expected to have been lowered to
`fir.do_loop` nests. In case of parallelization, the `fir.do_concurrent`
users are expected to have been lowered to the target parallel model
(e.g. OpenMP).

This hopefully resolved a build issue introduced by
llvm#142567 (see for example:
https://lab.llvm.org/buildbot/#/builders/199/builds/4009).
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
…rent` (llvm#142567)" (llvm#143905)

This reverts commit 937be17.

Resolves llvm#143897 until the
todo is properly handled.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants