-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-flang-fir-hlfir Author: Kareem Ergawy (ergawy) ChangesFull diff: https://github.com/llvm/llvm-project/pull/142567.diff 6 Files Affected:
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
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
47ad6cc
to
345406e
Compare
8f5a70a
to
8c8d15a
Compare
345406e
to
5647d02
Compare
tblah
approved these changes
Jun 4, 2025
8c8d15a
to
1632048
Compare
5647d02
to
fd943c2
Compare
ba6ac8f
to
148fdcf
Compare
Base automatically changed from
users/ergawy/convert_locality_specs_to_clauses_1
to
main
June 4, 2025 23:01
fd943c2
to
445db39
Compare
445db39
to
baaf8b2
Compare
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
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.