[flang-commits] [flang] [llvm] [mlir] [WIP][flang][OpenMP] Enable delayed privatization by default (PR #90945)
Kareem Ergawy via flang-commits
flang-commits at lists.llvm.org
Tue Jun 4 20:51:23 PDT 2024
https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/90945
>From 6ad9565006e43f2916cc41fa6dfa7f77d33a653d Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Sat, 4 May 2024 00:02:21 -0500
Subject: [PATCH 1/9] [flang][OpenMP] Try to unify induction var privatization
This PR tries to unify the paths taken by flang to privatize the
induction variable of loops. With the changes introduced here, both
cases below are now privatized in the same way:
```fortran
subroutine nested_constructs
implicit none
integer :: i
!$omp parallel default(private)
do i = 1, 10
end do
!$omp end parallel
end subroutine
```
```fortran
subroutine nested_constructs
implicit none
integer :: i
!$omp parallel private(i)
do i = 1, 10
end do
!$omp end parallel
end subroutine
```
---
flang/lib/Semantics/resolve-directives.cpp | 1 -
flang/test/Lower/OpenMP/default-clause.f90 | 13 +++++++++----
flang/test/Semantics/OpenMP/do05-positivecase.f90 | 2 +-
flang/test/Semantics/OpenMP/symbol08.f90 | 8 ++++----
4 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index c99b1c413970e..ba013dcd5d283 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1674,7 +1674,6 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
// parallel or task
if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
targetIt++;
- symbol->set(Symbol::Flag::OmpPreDetermined);
iv.symbol = symbol; // adjust the symbol within region
for (auto it{dirContext_.rbegin()}; it != targetIt; ++it) {
AddToContextObjectWithDSA(*symbol, Symbol::Flag::OmpPrivate, *it);
diff --git a/flang/test/Lower/OpenMP/default-clause.f90 b/flang/test/Lower/OpenMP/default-clause.f90
index c9e76780de5de..f09da04d24494 100644
--- a/flang/test/Lower/OpenMP/default-clause.f90
+++ b/flang/test/Lower/OpenMP/default-clause.f90
@@ -537,16 +537,21 @@ subroutine nested_constructs
integer :: y, z
!CHECK: omp.parallel {
-!CHECK: %[[INNER_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned}
-!CHECK: %[[INNER_J_DECL:.*]]:2 = hlfir.declare %[[INNER_J]] {{.*}}
-!CHECK: %[[INNER_I:.*]] = fir.alloca i32 {bindc_name = "i", pinned}
-!CHECK: %[[INNER_I_DECL:.*]]:2 = hlfir.declare %[[INNER_I]] {{.*}}
+
!CHECK: %[[INNER_Y:.*]] = fir.alloca i32 {bindc_name = "y", pinned, uniq_name = "_QFnested_constructsEy"}
!CHECK: %[[INNER_Y_DECL:.*]]:2 = hlfir.declare %[[INNER_Y]] {{.*}}
!CHECK: %[[TEMP:.*]] = fir.load %[[Y_DECL]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP]] to %[[INNER_Y_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
+
+!CHECK: %[[INNER_I:.*]] = fir.alloca i32 {bindc_name = "i", pinned, uniq_name
+!CHECK: %[[INNER_I_DECL:.*]]:2 = hlfir.declare %[[INNER_I]] {{.*}}
+
+!CHECK: %[[INNER_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned, uniq_name
+!CHECK: %[[INNER_J_DECL:.*]]:2 = hlfir.declare %[[INNER_J]] {{.*}}
+
!CHECK: %[[INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_constructsEz"}
!CHECK: %[[INNER_Z_DECL:.*]]:2 = hlfir.declare %[[INNER_Z]] {{.*}}
+
!$omp parallel default(private) firstprivate(y)
!CHECK: {{.*}} = fir.do_loop {{.*}} {
do i = 1, 10
diff --git a/flang/test/Semantics/OpenMP/do05-positivecase.f90 b/flang/test/Semantics/OpenMP/do05-positivecase.f90
index 4e02235f58a1a..7fa6f00447052 100644
--- a/flang/test/Semantics/OpenMP/do05-positivecase.f90
+++ b/flang/test/Semantics/OpenMP/do05-positivecase.f90
@@ -9,7 +9,7 @@ program omp_do
!DEF: /omp_do/n ObjectEntity INTEGER(4)
integer i,n
!$omp parallel
- !DEF: /omp_do/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+ !DEF: /omp_do/OtherConstruct1/i (OmpPrivate) HostAssoc INTEGER(4)
do i=1,10
!$omp single
print *, "hello"
diff --git a/flang/test/Semantics/OpenMP/symbol08.f90 b/flang/test/Semantics/OpenMP/symbol08.f90
index 50f34b736cdb7..64643fe76c39d 100644
--- a/flang/test/Semantics/OpenMP/symbol08.f90
+++ b/flang/test/Semantics/OpenMP/symbol08.f90
@@ -37,7 +37,7 @@ subroutine test_do
do j=6,10
!REF: /test_do/a
a(1,1,1) = 0.
- !DEF: /test_do/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+ !DEF: /test_do/OtherConstruct1/k (OmpPrivate) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_do/a
!REF: /test_do/OtherConstruct1/k
@@ -170,9 +170,9 @@ subroutine test_simd
!$omp parallel do simd
!DEF: /test_simd/OtherConstruct1/i (OmpLinear, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
- !DEF: /test_simd/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+ !DEF: /test_simd/OtherConstruct1/j (OmpPrivate) HostAssoc INTEGER(4)
do j=6,10
- !DEF: /test_simd/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+ !DEF: /test_simd/OtherConstruct1/k (OmpPrivate) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_simd/a
!REF: /test_simd/OtherConstruct1/k
@@ -228,7 +228,7 @@ subroutine test_seq_loop
print *, i, j
!$omp parallel
!REF: /test_seq_loop/i
- !DEF: /test_seq_loop/OtherConstruct1/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
+ !DEF: /test_seq_loop/OtherConstruct1/OtherConstruct1/j (OmpPrivate) HostAssoc INTEGER(4)
print *, i, j
!$omp do
!DEF: /test_seq_loop/OtherConstruct1/OtherConstruct1/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
>From 833b6004bea556c3cc968339b33cead99a700e55 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Mon, 6 May 2024 05:20:03 -0500
Subject: [PATCH 2/9] Add `assert(!OmpPreDetermined)`
---
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 3569a0f070beb..f53f62d6d17d5 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -87,10 +87,8 @@ void DataSharingProcessor::insertDeallocs() {
}
void DataSharingProcessor::cloneSymbol(const Fortran::semantics::Symbol *sym) {
- // Privatization for symbols which are pre-determined (like loop index
- // variables) happen separately, for everything else privatize here.
- if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
- return;
+ assert(!sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined));
+
bool success = converter.createHostAssociateVarClone(*sym);
(void)success;
assert(success && "Privatization failed due to existing binding");
>From e29c7d43530540193edc6e2cf8ed7a8eeb26cff1 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Mon, 6 May 2024 05:54:19 -0500
Subject: [PATCH 3/9] Undo assert.
---
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index f53f62d6d17d5..3569a0f070beb 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -87,8 +87,10 @@ void DataSharingProcessor::insertDeallocs() {
}
void DataSharingProcessor::cloneSymbol(const Fortran::semantics::Symbol *sym) {
- assert(!sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined));
-
+ // Privatization for symbols which are pre-determined (like loop index
+ // variables) happen separately, for everything else privatize here.
+ if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
+ return;
bool success = converter.createHostAssociateVarClone(*sym);
(void)success;
assert(success && "Privatization failed due to existing binding");
>From fe3562054af3e3b28fd5a23249841fa0310a09a8 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Tue, 7 May 2024 07:28:13 -0500
Subject: [PATCH 4/9] retain flag setting, check parallel
---
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 12 ++++++++++--
flang/lib/Semantics/resolve-directives.cpp | 1 +
flang/test/Semantics/OpenMP/do05-positivecase.f90 | 2 +-
flang/test/Semantics/OpenMP/symbol08.f90 | 8 ++++----
4 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 82d8d8dd98ea2..f2986ad0fab03 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -94,8 +94,17 @@ void DataSharingProcessor::insertDeallocs() {
void DataSharingProcessor::cloneSymbol(const Fortran::semantics::Symbol *sym) {
// Privatization for symbols which are pre-determined (like loop index
// variables) happen separately, for everything else privatize here.
- if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
+ auto isOMPParallelConstruct = [](Fortran::lower::pft::Evaluation &eval) {
+ if (const auto *ompConstruct = eval.getIf<parser::OpenMPConstruct>())
+ if (std::holds_alternative<parser::OpenMPBlockConstruct>(ompConstruct->u))
+ return true;
+ return false;
+ };
+
+ if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined) &&
+ !isOMPParallelConstruct(eval))
return;
+
bool success = converter.createHostAssociateVarClone(*sym);
(void)success;
assert(success && "Privatization failed due to existing binding");
@@ -344,7 +353,6 @@ void DataSharingProcessor::collectSymbols(
assert(curScope && "couldn't find current scope");
if (isPrivatizable(*sym) && !symbolsInNestedRegions.contains(sym) &&
!privatizedSymbols.contains(sym) &&
- !sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined) &&
(collectImplicit ||
!sym->test(Fortran::semantics::Symbol::Flag::OmpImplicit)) &&
clauseScopes.contains(&sym->owner()))
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index b82d300a2864c..2add2056f658d 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1674,6 +1674,7 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
// parallel or task
if (auto *symbol{ResolveOmp(iv, Symbol::Flag::OmpPrivate, targetIt->scope)}) {
targetIt++;
+ symbol->set(Symbol::Flag::OmpPreDetermined);
iv.symbol = symbol; // adjust the symbol within region
for (auto it{dirContext_.rbegin()}; it != targetIt; ++it) {
AddToContextObjectWithDSA(*symbol, Symbol::Flag::OmpPrivate, *it);
diff --git a/flang/test/Semantics/OpenMP/do05-positivecase.f90 b/flang/test/Semantics/OpenMP/do05-positivecase.f90
index 7fa6f00447052..4e02235f58a1a 100644
--- a/flang/test/Semantics/OpenMP/do05-positivecase.f90
+++ b/flang/test/Semantics/OpenMP/do05-positivecase.f90
@@ -9,7 +9,7 @@ program omp_do
!DEF: /omp_do/n ObjectEntity INTEGER(4)
integer i,n
!$omp parallel
- !DEF: /omp_do/OtherConstruct1/i (OmpPrivate) HostAssoc INTEGER(4)
+ !DEF: /omp_do/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,10
!$omp single
print *, "hello"
diff --git a/flang/test/Semantics/OpenMP/symbol08.f90 b/flang/test/Semantics/OpenMP/symbol08.f90
index 52b7567b945e0..3af85af74ee97 100644
--- a/flang/test/Semantics/OpenMP/symbol08.f90
+++ b/flang/test/Semantics/OpenMP/symbol08.f90
@@ -37,7 +37,7 @@ subroutine test_do
do j=6,10
!REF: /test_do/a
a(1,1,1) = 0.
- !DEF: /test_do/OtherConstruct1/k (OmpPrivate) HostAssoc INTEGER(4)
+ !DEF: /test_do/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_do/a
!REF: /test_do/OtherConstruct1/k
@@ -170,9 +170,9 @@ subroutine test_simd
!$omp parallel do simd
!DEF: /test_simd/OtherConstruct1/i (OmpLinear, OmpPreDetermined) HostAssoc INTEGER(4)
do i=1,5
- !DEF: /test_simd/OtherConstruct1/j (OmpPrivate) HostAssoc INTEGER(4)
+ !DEF: /test_simd/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do j=6,10
- !DEF: /test_simd/OtherConstruct1/k (OmpPrivate) HostAssoc INTEGER(4)
+ !DEF: /test_simd/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
do k=11,15
!REF: /test_simd/a
!REF: /test_simd/OtherConstruct1/k
@@ -228,7 +228,7 @@ subroutine test_seq_loop
print *, i, j
!$omp parallel
!REF: /test_seq_loop/i
- !DEF: /test_seq_loop/OtherConstruct1/OtherConstruct1/j (OmpPrivate) HostAssoc INTEGER(4)
+ !DEF: /test_seq_loop/OtherConstruct1/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
print *, i, j
!$omp do
!DEF: /test_seq_loop/OtherConstruct1/OtherConstruct1/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
>From e2bd438eebf2ec2b99cd7eccd82e32b5ee065cdc Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Fri, 3 May 2024 02:30:33 -0500
Subject: [PATCH 5/9] [WIP][flang][OpenMP] Enable delayed privatization by
default
Flips the delayed privatization switch to be on by default. For now,
this PR is only used to smoke out issues resulting from enabling dealyed
privatization. It is not meant to be merged (yet).
---
flang/lib/Lower/OpenMP/Utils.cpp | 2 +-
flang/test/Lower/OpenMP/copyprivate.f90 | 4 +++-
flang/test/Lower/OpenMP/default-clause-byref.f90 | 9 +++++++--
flang/test/Lower/OpenMP/default-clause.f90 | 9 +++++++--
flang/test/Lower/OpenMP/firstprivate-commonblock.f90 | 4 +++-
flang/test/Lower/OpenMP/implicit-dsa.f90 | 4 +++-
.../Lower/OpenMP/parallel-firstprivate-clause-scalar.f90 | 3 ++-
.../test/Lower/OpenMP/parallel-private-clause-fixes.f90 | 5 ++++-
flang/test/Lower/OpenMP/parallel-private-clause-str.f90 | 8 ++++++--
flang/test/Lower/OpenMP/parallel-private-clause.f90 | 3 ++-
flang/test/Lower/OpenMP/parallel-wsloop.f90 | 3 ++-
flang/test/Lower/OpenMP/private-commonblock.f90 | 4 +++-
flang/test/Lower/OpenMP/unstructured.f90 | 3 ++-
13 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index c38e0c18cac88..9e980661a51f3 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -31,7 +31,7 @@ llvm::cl::opt<bool> enableDelayedPrivatization(
"openmp-enable-delayed-privatization",
llvm::cl::desc(
"Emit `[first]private` variables as clauses on the MLIR ops."),
- llvm::cl::init(false));
+ llvm::cl::init(true));
namespace Fortran {
namespace lower {
diff --git a/flang/test/Lower/OpenMP/copyprivate.f90 b/flang/test/Lower/OpenMP/copyprivate.f90
index 9b76a996ef3e1..d1efb14de027f 100644
--- a/flang/test/Lower/OpenMP/copyprivate.f90
+++ b/flang/test/Lower/OpenMP/copyprivate.f90
@@ -1,5 +1,7 @@
! Test COPYPRIVATE.
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false -o - %s 2>&1 \
+! RUN: | FileCheck %s
!CHECK-DAG: func private @_copy_i64(%{{.*}}: !fir.ref<i64>, %{{.*}}: !fir.ref<i64>)
!CHECK-DAG: func private @_copy_f32(%{{.*}}: !fir.ref<f32>, %{{.*}}: !fir.ref<f32>)
diff --git a/flang/test/Lower/OpenMP/default-clause-byref.f90 b/flang/test/Lower/OpenMP/default-clause-byref.f90
index 62ba67e5962f4..71ea16aa06233 100644
--- a/flang/test/Lower/OpenMP/default-clause-byref.f90
+++ b/flang/test/Lower/OpenMP/default-clause-byref.f90
@@ -1,8 +1,13 @@
! This test checks lowering of OpenMP parallel directive
! with `DEFAULT` clause present.
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --force-byref-reduction %s -o - | FileCheck %s
-! RUN: bbc -fopenmp -emit-hlfir --force-byref-reduction %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --force-byref-reduction \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
+
+! RUN: bbc -fopenmp -emit-hlfir --force-byref-reduction \
+! RUN: --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
!CHECK: func @_QQmain() attributes {fir.bindc_name = "default_clause_lowering"} {
diff --git a/flang/test/Lower/OpenMP/default-clause.f90 b/flang/test/Lower/OpenMP/default-clause.f90
index 018fefce374a3..c23da31e519c9 100644
--- a/flang/test/Lower/OpenMP/default-clause.f90
+++ b/flang/test/Lower/OpenMP/default-clause.f90
@@ -1,8 +1,13 @@
! This test checks lowering of OpenMP parallel directive
! with `DEFAULT` clause present.
-! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
-! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
+
+! RUN: bbc -fopenmp -emit-hlfir \
+! RUN: --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
!CHECK: func @_QQmain() attributes {fir.bindc_name = "default_clause_lowering"} {
diff --git a/flang/test/Lower/OpenMP/firstprivate-commonblock.f90 b/flang/test/Lower/OpenMP/firstprivate-commonblock.f90
index d0fcdac76ad79..9b30492a91ba4 100644
--- a/flang/test/Lower/OpenMP/firstprivate-commonblock.f90
+++ b/flang/test/Lower/OpenMP/firstprivate-commonblock.f90
@@ -1,4 +1,6 @@
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false -o - %s 2>&1 \
+! RUN: | FileCheck %s
!CHECK: func.func @_QPfirstprivate_common() {
!CHECK: %[[val_0:.*]] = fir.address_of(@c_) : !fir.ref<!fir.array<8xi8>>
diff --git a/flang/test/Lower/OpenMP/implicit-dsa.f90 b/flang/test/Lower/OpenMP/implicit-dsa.f90
index 0f67d5bfd194f..e9472f1351e75 100644
--- a/flang/test/Lower/OpenMP/implicit-dsa.f90
+++ b/flang/test/Lower/OpenMP/implicit-dsa.f90
@@ -1,4 +1,6 @@
-!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false -o - %s 2>&1 \
+! RUN: | FileCheck %s
! Checks lowering of OpenMP variables with implicitly determined DSAs.
diff --git a/flang/test/Lower/OpenMP/parallel-firstprivate-clause-scalar.f90 b/flang/test/Lower/OpenMP/parallel-firstprivate-clause-scalar.f90
index 6402f98a2addc..399ef7fbff121 100644
--- a/flang/test/Lower/OpenMP/parallel-firstprivate-clause-scalar.f90
+++ b/flang/test/Lower/OpenMP/parallel-firstprivate-clause-scalar.f90
@@ -1,7 +1,8 @@
! This test checks lowering of `FIRSTPRIVATE` clause for scalar types.
! REQUIRES: shell
-! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck %s --check-prefix=CHECK
+! RUN: bbc -fopenmp -emit-hlfir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s --check-prefix=CHECK
!CHECK-DAG: func @_QPfirstprivate_complex(%[[ARG1:.*]]: !fir.ref<!fir.complex<4>>{{.*}}, %[[ARG2:.*]]: !fir.ref<!fir.complex<8>>{{.*}}) {
!CHECK: %[[ARG1_DECL:.*]]:2 = hlfir.declare %[[ARG1]] {uniq_name = "_QFfirstprivate_complexEarg1"} : (!fir.ref<!fir.complex<4>>) -> (!fir.ref<!fir.complex<4>>, !fir.ref<!fir.complex<4>>)
diff --git a/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90 b/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
index f8343338112c9..215d607534d00 100644
--- a/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
+++ b/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
@@ -1,6 +1,9 @@
! This test checks a few bug fixes in the PRIVATE clause lowering
-! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-hlfir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
+
+
! CHECK-LABEL: multiple_private_fix
! CHECK-SAME: %[[GAMA:.*]]: !fir.ref<i32> {fir.bindc_name = "gama"}
diff --git a/flang/test/Lower/OpenMP/parallel-private-clause-str.f90 b/flang/test/Lower/OpenMP/parallel-private-clause-str.f90
index 025e51e066176..473abf324e13b 100644
--- a/flang/test/Lower/OpenMP/parallel-private-clause-str.f90
+++ b/flang/test/Lower/OpenMP/parallel-private-clause-str.f90
@@ -2,8 +2,12 @@
! `PRIVATE` clause present for strings
! REQUIRES: shell
-! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck %s
-!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-hlfir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false -o - %s 2>&1 \
+! RUN: | FileCheck %s
!CHECK: func.func @_QPtest_allocatable_string(%{{.*}}: !fir.ref<i32> {fir.bindc_name = "n"}) {
!CHECK: %[[C_BOX_REF:.*]] = fir.alloca !fir.box<!fir.heap<!fir.char<1,?>>> {bindc_name = "c", uniq_name = "_QFtest_allocatable_stringEc"}
diff --git a/flang/test/Lower/OpenMP/parallel-private-clause.f90 b/flang/test/Lower/OpenMP/parallel-private-clause.f90
index b9b58a135aaa2..76fc8098ae2a3 100644
--- a/flang/test/Lower/OpenMP/parallel-private-clause.f90
+++ b/flang/test/Lower/OpenMP/parallel-private-clause.f90
@@ -2,7 +2,8 @@
! `PRIVATE` clause present.
! REQUIRES: shell
-! RUN: bbc --use-desc-for-alloc=false -fopenmp -emit-hlfir %s -o - | \
+! RUN: bbc --use-desc-for-alloc=false -fopenmp -emit-hlfir \
+! RUN: --openmp-enable-delayed-privatization=false %s -o - | \
! RUN: FileCheck %s --check-prefix=FIRDialect
!FIRDialect: func @_QPprivate_clause(%[[ARG1:.*]]: !fir.ref<i32> {fir.bindc_name = "arg1"}, %[[ARG2:.*]]: !fir.ref<!fir.array<10xi32>> {fir.bindc_name = "arg2"}, %[[ARG3:.*]]: !fir.boxchar<1> {fir.bindc_name = "arg3"}, %[[ARG4:.*]]: !fir.boxchar<1> {fir.bindc_name = "arg4"}) {
diff --git a/flang/test/Lower/OpenMP/parallel-wsloop.f90 b/flang/test/Lower/OpenMP/parallel-wsloop.f90
index 602b3d1c05f0d..d03159b0957f4 100644
--- a/flang/test/Lower/OpenMP/parallel-wsloop.f90
+++ b/flang/test/Lower/OpenMP/parallel-wsloop.f90
@@ -1,6 +1,7 @@
! This test checks lowering of OpenMP DO Directive (Worksharing).
-! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-hlfir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
! CHECK-LABEL: func @_QPsimple_parallel_do()
subroutine simple_parallel_do
diff --git a/flang/test/Lower/OpenMP/private-commonblock.f90 b/flang/test/Lower/OpenMP/private-commonblock.f90
index ee580594f7c3f..b8f459bac201e 100644
--- a/flang/test/Lower/OpenMP/private-commonblock.f90
+++ b/flang/test/Lower/OpenMP/private-commonblock.f90
@@ -1,4 +1,6 @@
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false -o - %s 2>&1 \
+! RUN: | FileCheck %s
!CHECK: func.func @_QPprivate_common() {
!CHECK: omp.parallel {
diff --git a/flang/test/Lower/OpenMP/unstructured.f90 b/flang/test/Lower/OpenMP/unstructured.f90
index 6a1331799d547..5b874a3c6867d 100644
--- a/flang/test/Lower/OpenMP/unstructured.f90
+++ b/flang/test/Lower/OpenMP/unstructured.f90
@@ -1,6 +1,7 @@
! Test unstructured code adjacent to and inside OpenMP constructs.
-! RUN: bbc %s -fopenmp -emit-hlfir -o "-" | FileCheck %s
+! RUN: bbc %s -fopenmp -emit-hlfir --openmp-enable-delayed-privatization=false -o "-" \
+! RUN: | FileCheck %s
! CHECK-LABEL: func @_QPss1{{.*}} {
! CHECK: br ^bb1
>From dfcd932ec50246582be8f99aecdd58ff53c13847 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Mon, 27 May 2024 07:49:21 -0500
Subject: [PATCH 6/9] fix more expectations
---
.../Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90 | 9 +++++++--
flang/test/Lower/OpenMP/hlfir-seqloop-parallel.f90 | 9 +++++++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90 b/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90
index 773452206993f..c59c10d1c7b76 100644
--- a/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90
+++ b/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90
@@ -1,7 +1,12 @@
! This test checks the lowering of parallel do
-! RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | FileCheck %s
-! RUN: bbc -fopenmp -emit-fir -hlfir=false %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
+
+! RUN: bbc -fopenmp -emit-fir -hlfir=false \
+! RUN: --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
! The string "EXPECTED" denotes the expected FIR
diff --git a/flang/test/Lower/OpenMP/hlfir-seqloop-parallel.f90 b/flang/test/Lower/OpenMP/hlfir-seqloop-parallel.f90
index 271b97819e606..084f2de6360a3 100644
--- a/flang/test/Lower/OpenMP/hlfir-seqloop-parallel.f90
+++ b/flang/test/Lower/OpenMP/hlfir-seqloop-parallel.f90
@@ -1,8 +1,13 @@
! This test checks lowering of sequential loops in OpenMP parallel.
! The loop indices of these loops should be privatised.
-! RUN: bbc -hlfir -fopenmp -emit-hlfir %s -o - | FileCheck %s
-! RUN: %flang_fc1 -emit-hlfir -flang-experimental-hlfir -fopenmp %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-hlfir \
+! RUN: --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
+
+! RUN: %flang_fc1 -emit-hlfir -flang-experimental-hlfir -fopenmp \
+! RUN: -mmlir --openmp-enable-delayed-privatization=false %s -o - \
+! RUN: | FileCheck %s
subroutine sb1
>From bef3b906e09f12a8c2c7c1b7fb5fc37b3acc9203 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Tue, 28 May 2024 04:46:18 -0500
Subject: [PATCH 7/9] xxx
---
.../lib/Lower/OpenMP/DataSharingProcessor.cpp | 26 ++++++++++++-------
flang/lib/Lower/OpenMP/OpenMP.cpp | 5 ++--
flang/lib/Optimizer/Builder/FIRBuilder.cpp | 6 ++++-
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 3 +++
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 1 +
5 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index b722e19272ca1..68b63c5753f60 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -82,11 +82,11 @@ void DataSharingProcessor::processStep2(mlir::Operation *op, bool isLoop) {
}
void DataSharingProcessor::insertDeallocs() {
- for (const semantics::Symbol *sym : allPrivatizedSymbols)
+ for (const semantics::Symbol *sym : allPrivatizedSymbols) {
if (semantics::IsAllocatable(sym->GetUltimate())) {
if (!useDelayedPrivatization) {
converter.createHostAssociateVarCloneDealloc(*sym);
- return;
+ continue;
}
lower::SymbolBox hsb = converter.lookupOneLevelUpSymbol(*sym);
@@ -102,8 +102,9 @@ void DataSharingProcessor::insertDeallocs() {
mlir::Region &deallocRegion = privatizer.getDeallocRegion();
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- mlir::Block *deallocEntryBlock = firOpBuilder.createBlock(
- &deallocRegion, /*insertPt=*/{}, symType, symLoc);
+ mlir::Block *deallocEntryBlock =
+ firOpBuilder.createBlock(&deallocRegion, /*insertPt=*/{}, symType,
+ firOpBuilder.getUnknownLoc());
firOpBuilder.setInsertionPointToEnd(deallocEntryBlock);
symTable->addSymbol(*sym,
@@ -114,6 +115,7 @@ void DataSharingProcessor::insertDeallocs() {
symTable->popScope();
}
+ }
}
void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) {
@@ -138,8 +140,9 @@ void DataSharingProcessor::copyLastPrivateSymbol(
void DataSharingProcessor::collectOmpObjectListSymbol(
const omp::ObjectList &objects,
llvm::SetVector<const semantics::Symbol *> &symbolSet) {
- for (const omp::Object &object : objects)
+ for (const omp::Object &object : objects) {
symbolSet.insert(object.id());
+ }
}
void DataSharingProcessor::collectSymbolsForPrivatization() {
@@ -162,8 +165,9 @@ void DataSharingProcessor::collectSymbolsForPrivatization() {
}
}
- for (auto *sym : explicitlyPrivatizedSymbols)
+ for (auto *sym : explicitlyPrivatizedSymbols) {
allPrivatizedSymbols.insert(sym);
+ }
if (hasCollapse && hasLastPrivateOp)
TODO(converter.getCurrentLocation(), "Collapse clause with lastprivate");
@@ -474,7 +478,7 @@ void DataSharingProcessor::doPrivatize(
firOpBuilder.setInsertionPoint(&moduleOp.getBodyRegion().front(),
moduleOp.getBodyRegion().front().begin());
auto result = firOpBuilder.create<mlir::omp::PrivateClauseOp>(
- symLoc, uniquePrivatizerName, symType,
+ firOpBuilder.getUnknownLoc(), uniquePrivatizerName, symType,
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
: mlir::omp::DataSharingClauseType::Private);
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
@@ -485,13 +489,14 @@ void DataSharingProcessor::doPrivatize(
{
mlir::Region &allocRegion = result.getAllocRegion();
mlir::Block *allocEntryBlock = firOpBuilder.createBlock(
- &allocRegion, /*insertPt=*/{}, symType, symLoc);
+ &allocRegion, /*insertPt=*/{}, symType, firOpBuilder.getUnknownLoc());
firOpBuilder.setInsertionPointToEnd(allocEntryBlock);
fir::ExtendedValue localExV =
hlfir::translateToExtendedValue(
- symLoc, firOpBuilder, hlfir::Entity{allocRegion.getArgument(0)},
+ firOpBuilder.getUnknownLoc(), firOpBuilder,
+ hlfir::Entity{allocRegion.getArgument(0)},
/*contiguousHint=*/
evaluate::IsSimplyContiguous(*sym, converter.getFoldingContext()))
.first;
@@ -511,7 +516,8 @@ void DataSharingProcessor::doPrivatize(
// First block argument corresponding to the original/host value while
// second block argument corresponding to the privatized value.
mlir::Block *copyEntryBlock = firOpBuilder.createBlock(
- ©Region, /*insertPt=*/{}, {symType, symType}, {symLoc, symLoc});
+ ©Region, /*insertPt=*/{}, {symType, symType},
+ {firOpBuilder.getUnknownLoc(), firOpBuilder.getUnknownLoc()});
firOpBuilder.setInsertionPointToEnd(copyEntryBlock);
auto addSymbol = [&](unsigned argIdx, bool force = false) {
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 9598457d123cf..18b5c9bfe5832 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1337,8 +1337,9 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
llvm::SmallVector<mlir::Location> privateVarLocs = reductionLocs;
privateVarLocs.reserve(privateVarLocs.size() + privateVars.size());
- llvm::transform(privateVars, std::back_inserter(privateVarLocs),
- [](mlir::Value v) { return v.getLoc(); });
+ llvm::transform(
+ privateVars, std::back_inserter(privateVarLocs),
+ [&](mlir::Value v) { return firOpBuilder.getUnknownLoc(); });
firOpBuilder.createBlock(®ion, /*insertPt=*/{}, privateVarTypes,
privateVarLocs);
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 3c3fd02d7c88e..b9114e0541522 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -252,8 +252,12 @@ mlir::Block *fir::FirOpBuilder::getAllocaBlock() {
.getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>()) {
return ompOutlineableIface.getAllocaBlock();
}
- if (getRegion().getParentOfType<mlir::omp::DeclareReductionOp>())
+
+ if (getRegion().getParentOfType<mlir::omp::DeclareReductionOp>() ||
+ getRegion().getParentOfType<mlir::omp::PrivateClauseOp>()) {
return &getRegion().front();
+ }
+
if (auto accRecipeIface =
getRegion().getParentOfType<mlir::acc::RecipeInterface>()) {
return accRecipeIface.getAllocaBlock(getRegion());
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index cb4de9c8876dc..eab41eb8a35b2 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1583,6 +1583,9 @@ IRBuilder<>::InsertPoint OpenMPIRBuilder::createParallel(
} else {
Builder.restoreIP(
PrivCB(InnerAllocaIP, Builder.saveIP(), V, *Inner, ReplacementValue));
+ InnerAllocaIP = {InnerAllocaIP.getPoint()->getParent(),
+ InnerAllocaIP.getPoint()};
+
assert(ReplacementValue &&
"Expected copy/create callback to set replacement value!");
if (ReplacementValue == &V)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 6ec4c120c11ea..8b72dff0f72c0 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1351,6 +1351,7 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
// called for variables which have destructors/finalizers.
auto finiCB = [&](InsertPointTy codeGenIP) {
InsertPointTy oldIP = builder.saveIP();
+ // TODO Should this be updated?
builder.restoreIP(codeGenIP);
// if the reduction has a cleanup region, inline it here to finalize the
>From 249f583d2c4e57a5c2ded45fea47ed90f11d03e6 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Tue, 28 May 2024 07:14:56 -0500
Subject: [PATCH 8/9] yyy
---
.../lib/Lower/OpenMP/DataSharingProcessor.cpp | 24 +++++++------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 68b63c5753f60..557a9685024c5 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -82,7 +82,7 @@ void DataSharingProcessor::processStep2(mlir::Operation *op, bool isLoop) {
}
void DataSharingProcessor::insertDeallocs() {
- for (const semantics::Symbol *sym : allPrivatizedSymbols) {
+ for (const semantics::Symbol *sym : allPrivatizedSymbols)
if (semantics::IsAllocatable(sym->GetUltimate())) {
if (!useDelayedPrivatization) {
converter.createHostAssociateVarCloneDealloc(*sym);
@@ -102,9 +102,8 @@ void DataSharingProcessor::insertDeallocs() {
mlir::Region &deallocRegion = privatizer.getDeallocRegion();
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- mlir::Block *deallocEntryBlock =
- firOpBuilder.createBlock(&deallocRegion, /*insertPt=*/{}, symType,
- firOpBuilder.getUnknownLoc());
+ mlir::Block *deallocEntryBlock = firOpBuilder.createBlock(
+ &deallocRegion, /*insertPt=*/{}, symType, symLoc);
firOpBuilder.setInsertionPointToEnd(deallocEntryBlock);
symTable->addSymbol(*sym,
@@ -115,7 +114,6 @@ void DataSharingProcessor::insertDeallocs() {
symTable->popScope();
}
- }
}
void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) {
@@ -140,9 +138,8 @@ void DataSharingProcessor::copyLastPrivateSymbol(
void DataSharingProcessor::collectOmpObjectListSymbol(
const omp::ObjectList &objects,
llvm::SetVector<const semantics::Symbol *> &symbolSet) {
- for (const omp::Object &object : objects) {
+ for (const omp::Object &object : objects)
symbolSet.insert(object.id());
- }
}
void DataSharingProcessor::collectSymbolsForPrivatization() {
@@ -165,9 +162,8 @@ void DataSharingProcessor::collectSymbolsForPrivatization() {
}
}
- for (auto *sym : explicitlyPrivatizedSymbols) {
+ for (auto *sym : explicitlyPrivatizedSymbols)
allPrivatizedSymbols.insert(sym);
- }
if (hasCollapse && hasLastPrivateOp)
TODO(converter.getCurrentLocation(), "Collapse clause with lastprivate");
@@ -478,7 +474,7 @@ void DataSharingProcessor::doPrivatize(
firOpBuilder.setInsertionPoint(&moduleOp.getBodyRegion().front(),
moduleOp.getBodyRegion().front().begin());
auto result = firOpBuilder.create<mlir::omp::PrivateClauseOp>(
- firOpBuilder.getUnknownLoc(), uniquePrivatizerName, symType,
+ symLoc, uniquePrivatizerName, symType,
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
: mlir::omp::DataSharingClauseType::Private);
fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
@@ -489,14 +485,13 @@ void DataSharingProcessor::doPrivatize(
{
mlir::Region &allocRegion = result.getAllocRegion();
mlir::Block *allocEntryBlock = firOpBuilder.createBlock(
- &allocRegion, /*insertPt=*/{}, symType, firOpBuilder.getUnknownLoc());
+ &allocRegion, /*insertPt=*/{}, symType, symLoc);
firOpBuilder.setInsertionPointToEnd(allocEntryBlock);
fir::ExtendedValue localExV =
hlfir::translateToExtendedValue(
- firOpBuilder.getUnknownLoc(), firOpBuilder,
- hlfir::Entity{allocRegion.getArgument(0)},
+ symLoc, firOpBuilder, hlfir::Entity{allocRegion.getArgument(0)},
/*contiguousHint=*/
evaluate::IsSimplyContiguous(*sym, converter.getFoldingContext()))
.first;
@@ -516,8 +511,7 @@ void DataSharingProcessor::doPrivatize(
// First block argument corresponding to the original/host value while
// second block argument corresponding to the privatized value.
mlir::Block *copyEntryBlock = firOpBuilder.createBlock(
- ©Region, /*insertPt=*/{}, {symType, symType},
- {firOpBuilder.getUnknownLoc(), firOpBuilder.getUnknownLoc()});
+ ©Region, /*insertPt=*/{}, {symType, symType}, {symLoc, symLoc});
firOpBuilder.setInsertionPointToEnd(copyEntryBlock);
auto addSymbol = [&](unsigned argIdx, bool force = false) {
>From ca83608167494f46f936c501e01b161c5ccba1f5 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Tue, 4 Jun 2024 22:50:51 -0500
Subject: [PATCH 9/9] post-merge fixes
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 5 ++---
.../LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp | 1 -
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 18b5c9bfe5832..9598457d123cf 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1337,9 +1337,8 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
llvm::SmallVector<mlir::Location> privateVarLocs = reductionLocs;
privateVarLocs.reserve(privateVarLocs.size() + privateVars.size());
- llvm::transform(
- privateVars, std::back_inserter(privateVarLocs),
- [&](mlir::Value v) { return firOpBuilder.getUnknownLoc(); });
+ llvm::transform(privateVars, std::back_inserter(privateVarLocs),
+ [](mlir::Value v) { return v.getLoc(); });
firOpBuilder.createBlock(®ion, /*insertPt=*/{}, privateVarTypes,
privateVarLocs);
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 8b72dff0f72c0..6ec4c120c11ea 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1351,7 +1351,6 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
// called for variables which have destructors/finalizers.
auto finiCB = [&](InsertPointTy codeGenIP) {
InsertPointTy oldIP = builder.saveIP();
- // TODO Should this be updated?
builder.restoreIP(codeGenIP);
// if the reduction has a cleanup region, inline it here to finalize the
More information about the flang-commits
mailing list