[Flang][OpenMP] Make implicitly captured scalars fully firstprivatized #147442
+88
−9
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
Currently, we indicate to the runtime that implicit scalar captures are firstprivate (via map and
capture types), enough for the runtime trace to treat it as such, but we do not CodeGen the IR
in such a way that we can take full advantage of this aspect of the OpenMP specification.
This patch seeks to change that by applying the correct symbol flags (firstprivate/implicit) to the
implicitly captured scalars within target regions, which then triggers the delayed privitization code
generation for these symbols, bringing the code generation in-line with the explicit firstpriviate
clause. Currently, similarly to the delayed privitization I have sheltered this segment of code
behind the EnabledDelayedPrivitization flag, as without it, we'll trigger an compiler error for
firstprivate not being supported any time we implicitly capture a scalar and try to firstprivitize
it, in future when this flag is removed it can also be removed here. So, for now, you need to
enable this via providing the compiler the flag on compilation of any programs.
agozillon
requested review from
skatrak,
tblah,
mjklemm,
dpalermo,
mrkajetanp,
TIFitis and
bhandarkar-pranav
llvmbot
added
flang
agozillon
changed the title
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-semantics Author: None (agozillon) ChangesCurrently, we indicate to the runtime that implicit scalar captures are firstprivate (via map and This patch seeks to change that by applying the correct symbol flags (firstprivate/implicit) to the Full diff: https://github.com/llvm/llvm-project/pull/147442.diff 3 Files Affected:
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 299bb6ff876e7..f7aa2df4c25b1 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -23,6 +23,7 @@
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "flang/Support/Flags.h"
#include "llvm/Frontend/OpenMP/OMP.h.inc"
#include "llvm/Support/Debug.h"
#include <list>
@@ -2297,6 +2298,34 @@ static bool IsSymbolStaticStorageDuration(const Symbol &symbol) {
(ultSym.flags().test(Symbol::Flag::InCommonBlock));
}
+static bool IsTargetCaptureImplicitlyFirstPrivatizeable(
+ const Symbol &symbol, const Symbol::Flags &dsa) {
+ // if we're associated with any other flags we skip implicit privitization
+ // for now. If we're an allocatable, pointer or declare target, we're not
+ // implicitly firstprivitizeable under OpenMP restrictions.
+ // TODO: Relax restriction as we progress privitization and further
+ // investigate the flags we can intermix with.
+ if (!dsa.none() || !symbol.flags().none() ||
+ semantics::IsAllocatableOrPointer(symbol)) {
+ return false;
+ }
+
+ // It is default firstprivatizeable as far as the OpenMP specification is
+ // concerned if it is a non-array scalar type that has been implicitly
+ // captured in a target region
+ const auto *type{symbol.GetType()};
+ if ((!symbol.GetShape() || symbol.GetShape()->empty()) &&
+ (type->category() ==
+ Fortran::semantics::DeclTypeSpec::Category::Numeric ||
+ type->category() ==
+ Fortran::semantics::DeclTypeSpec::Category::Logical ||
+ type->category() ==
+ Fortran::semantics::DeclTypeSpec::Category::Character)) {
+ return true;
+ }
+ return false;
+}
+
void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
if (!IsPrivatizable(symbol)) {
return;
@@ -2444,7 +2473,15 @@ void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
useLastDeclSymbol();
PRINT_IMPLICIT_RULE("3) enclosing context");
} else if (targetDir) {
- // TODO 4) not mapped target variable -> firstprivate
+ // 4) not mapped target variable -> firstprivate
+ // - i.e. implicit, but meets OpenMP specification rules for
+ // firstprivate "promotion"
+ if (enableDelayedPrivatizationStaging && symbol &&
+ IsTargetCaptureImplicitlyFirstPrivatizeable(*symbol, prevDSA)) {
+ prevDSA.set(Symbol::Flag::OmpImplicit);
+ prevDSA.set(Symbol::Flag::OmpFirstPrivate);
+ makeSymbol(prevDSA);
+ }
dsa = prevDSA;
} else if (taskGenDir) {
// TODO 5) dummy arg in orphaned taskgen construct -> firstprivate
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90
new file mode 100644
index 0000000000000..6cd14e7d3a2b4
--- /dev/null
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90
@@ -0,0 +1,42 @@
+! Tests delayed privatization works for implicit capture of scalars similarly to
+! the way it works for explicitly firstprivitized scalars.
+
+! RUN: %flang_fc1 -emit-mlir -fopenmp -mmlir --enable-delayed-privatization-staging \
+! RUN: -o - %s 2>&1 | FileCheck %s
+
+! CHECK-LABEL: omp.private {type = firstprivate} @_QFExdgfx_firstprivate_i32 : i32 copy {
+! CHECK: ^bb0(%{{.*}}: !fir.ref<i32>, %{{.*}}: !fir.ref<i32>):
+! CHECK: %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+! CHECK: fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+! CHECK: omp.yield(%{{.*}} : !fir.ref<i32>)
+! CHECK: }
+
+! CHECK-LABEL: omp.private {type = firstprivate} @_QFExfpvx_firstprivate_i32 : i32 copy {
+! CHECK: ^bb0(%{{.*}}: !fir.ref<i32>, %{{.*}}: !fir.ref<i32>):
+! CHECK: %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+! CHECK: fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+! CHECK: omp.yield(%{{.*}} : !fir.ref<i32>)
+! CHECK: }
+
+! CHECK: %[[VAL_0:.*]] = fir.alloca i32 {bindc_name = "xdgfx", uniq_name = "_QFExdgfx"}
+! CHECK: %[[VAL_1:.*]] = fir.declare %[[VAL_0]] {uniq_name = "_QFExdgfx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+! CHECK: %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "xfpvx", uniq_name = "_QFExfpvx"}
+! CHECK: %[[VAL_3:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFExfpvx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+! CHECK: %[[VAL_4:.*]] = omp.map.info var_ptr(%[[VAL_3]] : !fir.ref<i32>, i32) map_clauses(to) capture(ByRef) -> !fir.ref<i32>
+! CHECK: %[[VAL_5:.*]] = omp.map.info var_ptr(%[[VAL_1]] : !fir.ref<i32>, i32) map_clauses(to) capture(ByRef) -> !fir.ref<i32>
+
+! CHECK: omp.target map_entries(%[[VAL_4]] -> %{{.*}}, %[[VAL_5]] -> %{{.*}} : !fir.ref<i32>, !fir.ref<i32>) private(@_QFExfpvx_firstprivate_i32 %[[VAL_3]] -> %[[VAL_6:.*]] [map_idx=0], @_QFExdgfx_firstprivate_i32 %[[VAL_1]] -> %[[VAL_7:.*]] [map_idx=1] : !fir.ref<i32>, !fir.ref<i32>) {
+! CHECK: %{{.*}} = fir.declare %[[VAL_6]] {uniq_name = "_QFExfpvx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+! CHECK: %{{.*}} = fir.declare %[[VAL_7]] {uniq_name = "_QFExdgfx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+
+program test_default_implicit_firstprivate
+ implicit none
+ integer :: xdgfx, xfpvx
+ xdgfx = 1
+ xfpvx = 2
+ !$omp target firstprivate(xfpvx)
+ xdgfx = 42
+ xfpvx = 43
+ !$omp end target
+ write(*,*) xdgfx, xfpvx
+end program
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 217ac5638a3ea..e7e3fb2097308 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
@@ -139,28 +139,28 @@ end subroutine target_allocatable
! CHECK: %[[REAL_ARR_ALLOC:.*]] = fir.alloca !fir.array<?xf32>, {{.*}} {bindc_name = "real_arr", {{.*}}}
! CHECK: %[[REAL_ARR_DECL:.*]]:2 = hlfir.declare %[[REAL_ARR_ALLOC]]({{.*}})
! CHECK: fir.store %[[REAL_ARR_DECL]]#0 to %[[REAL_ARR_DESC_ALLOCA]] : !fir.ref<!fir.box<!fir.array<?xf32>>>
-! CHECK: %[[MAPPED_MI0:.*]] = omp.map.info var_ptr(%[[MAPPED_DECL]]#1 : !fir.ref<i32>, i32) {{.*}}
! CHECK: %[[ALLOC_VAR_MEMBER:.*]] = omp.map.info var_ptr(%[[ALLOC_VAR_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<i32>>>, i32)
! CHECK: %[[ALLOC_VAR_MAP:.*]] = omp.map.info var_ptr(%[[ALLOC_VAR_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.box<!fir.heap<i32>>) {{.*}} members(%[[ALLOC_VAR_MEMBER]] :
! CHECK: %[[REAL_ARR_MEMBER:.*]] = omp.map.info var_ptr(%[[REAL_ARR_DESC_ALLOCA]] : !fir.ref<!fir.box<!fir.array<?xf32>>>, f32)
! CHECK: %[[REAL_ARR_DESC_MAP:.*]] = omp.map.info var_ptr(%[[REAL_ARR_DESC_ALLOCA]] : !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.box<!fir.array<?xf32>>) {{.*}} members(%[[REAL_ARR_MEMBER]] :
! CHECK: fir.store %[[CHAR_VAR_DECL]]#0 to %[[CHAR_VAR_DESC_ALLOCA]] : !fir.ref<!fir.boxchar<1>>
! CHECK: %[[CHAR_VAR_DESC_MAP:.*]] = omp.map.info var_ptr(%[[CHAR_VAR_DESC_ALLOCA]] : !fir.ref<!fir.boxchar<1>>, !fir.boxchar<1>)
+! CHECK: %[[MAPPED_MI0:.*]] = omp.map.info var_ptr(%[[MAPPED_DECL]]#0 : !fir.ref<i32>, i32) {{.*}}
! CHECK: omp.target
! CHECK-SAME: map_entries(
-! CHECK-SAME: %[[MAPPED_MI0]] -> %[[MAPPED_ARG0:[^,]+]],
-! CHECK-SAME: %[[ALLOC_VAR_MAP]] -> %[[MAPPED_ARG1:[^,]+]]
+! CHECK-SAME: %[[ALLOC_VAR_MAP]] -> %[[MAPPED_ARG1:[^,]+]],
! CHECK-SAME: %[[REAL_ARR_DESC_MAP]] -> %[[MAPPED_ARG2:[^,]+]]
! CHECK-SAME: %[[CHAR_VAR_DESC_MAP]] -> %[[MAPPED_ARG3:.[^,]+]]
-! CHECK-SAME: !fir.ref<i32>, !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<!fir.boxchar<1>>, !fir.llvm_ptr<!fir.ref<i32>>, !fir.llvm_ptr<!fir.ref<!fir.array<?xf32>>>
+! CHECK-SAME: %[[MAPPED_MI0]] -> %[[MAPPED_ARG0:[^,]+]]
+! CHECK-SAME: !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<!fir.boxchar<1>>, !fir.ref<i32>, !fir.llvm_ptr<!fir.ref<i32>>
! CHECK-SAME: private(
-! CHECK-SAME: @[[ALLOC_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[ALLOC_ARG:[^,]+]] [map_idx=1],
+! CHECK-SAME: @[[ALLOC_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[ALLOC_ARG:[^,]+]] [map_idx=0],
! CHECK-SAME: @[[REAL_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[REAL_ARG:[^,]+]],
! CHECK-SAME: @[[LB_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[LB_ARG:[^,]+]],
-! CHECK-SAME: @[[ARR_PRIVATIZER_SYM]] %{{[^[:space:]]+}} -> %[[ARR_ARG:[^,]+]] [map_idx=2],
+! CHECK-SAME: @[[ARR_PRIVATIZER_SYM]] %{{[^[:space:]]+}} -> %[[ARR_ARG:[^,]+]] [map_idx=1],
! CHECK-SAME: @[[COMP_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[COMP_ARG:[^,]+]],
-! CHECK-SAME: @[[CHAR_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[CHAR_ARG:[^,]+]] [map_idx=3] :
-! CHECK-SAME: !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<f32>, !fir.ref<i64>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<complex<f32>>, !fir.boxchar<1>) {
+! CHECK-SAME: @[[CHAR_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[CHAR_ARG:[^,]+]] [map_idx=2],
+! CHECK-SAME: !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<f32>, !fir.ref<i64>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<complex<f32>>, !fir.boxchar<1>, !fir.ref<i32>) {
! CHECK-NOT: fir.alloca
! CHECK: hlfir.declare %[[ALLOC_ARG]]
! CHECK: hlfir.declare %[[REAL_ARG]]
|
|
@llvm/pr-subscribers-flang-fir-hlfir Author: None (agozillon) ChangesCurrently, we indicate to the runtime that implicit scalar captures are firstprivate (via map and This patch seeks to change that by applying the correct symbol flags (firstprivate/implicit) to the Full diff: https://github.com/llvm/llvm-project/pull/147442.diff 3 Files Affected:
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 299bb6ff876e7..f7aa2df4c25b1 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -23,6 +23,7 @@
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "flang/Support/Flags.h"
#include "llvm/Frontend/OpenMP/OMP.h.inc"
#include "llvm/Support/Debug.h"
#include <list>
@@ -2297,6 +2298,34 @@ static bool IsSymbolStaticStorageDuration(const Symbol &symbol) {
(ultSym.flags().test(Symbol::Flag::InCommonBlock));
}
+static bool IsTargetCaptureImplicitlyFirstPrivatizeable(
+ const Symbol &symbol, const Symbol::Flags &dsa) {
+ // if we're associated with any other flags we skip implicit privitization
+ // for now. If we're an allocatable, pointer or declare target, we're not
+ // implicitly firstprivitizeable under OpenMP restrictions.
+ // TODO: Relax restriction as we progress privitization and further
+ // investigate the flags we can intermix with.
+ if (!dsa.none() || !symbol.flags().none() ||
+ semantics::IsAllocatableOrPointer(symbol)) {
+ return false;
+ }
+
+ // It is default firstprivatizeable as far as the OpenMP specification is
+ // concerned if it is a non-array scalar type that has been implicitly
+ // captured in a target region
+ const auto *type{symbol.GetType()};
+ if ((!symbol.GetShape() || symbol.GetShape()->empty()) &&
+ (type->category() ==
+ Fortran::semantics::DeclTypeSpec::Category::Numeric ||
+ type->category() ==
+ Fortran::semantics::DeclTypeSpec::Category::Logical ||
+ type->category() ==
+ Fortran::semantics::DeclTypeSpec::Category::Character)) {
+ return true;
+ }
+ return false;
+}
+
void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
if (!IsPrivatizable(symbol)) {
return;
@@ -2444,7 +2473,15 @@ void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
useLastDeclSymbol();
PRINT_IMPLICIT_RULE("3) enclosing context");
} else if (targetDir) {
- // TODO 4) not mapped target variable -> firstprivate
+ // 4) not mapped target variable -> firstprivate
+ // - i.e. implicit, but meets OpenMP specification rules for
+ // firstprivate "promotion"
+ if (enableDelayedPrivatizationStaging && symbol &&
+ IsTargetCaptureImplicitlyFirstPrivatizeable(*symbol, prevDSA)) {
+ prevDSA.set(Symbol::Flag::OmpImplicit);
+ prevDSA.set(Symbol::Flag::OmpFirstPrivate);
+ makeSymbol(prevDSA);
+ }
dsa = prevDSA;
} else if (taskGenDir) {
// TODO 5) dummy arg in orphaned taskgen construct -> firstprivate
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90
new file mode 100644
index 0000000000000..6cd14e7d3a2b4
--- /dev/null
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90
@@ -0,0 +1,42 @@
+! Tests delayed privatization works for implicit capture of scalars similarly to
+! the way it works for explicitly firstprivitized scalars.
+
+! RUN: %flang_fc1 -emit-mlir -fopenmp -mmlir --enable-delayed-privatization-staging \
+! RUN: -o - %s 2>&1 | FileCheck %s
+
+! CHECK-LABEL: omp.private {type = firstprivate} @_QFExdgfx_firstprivate_i32 : i32 copy {
+! CHECK: ^bb0(%{{.*}}: !fir.ref<i32>, %{{.*}}: !fir.ref<i32>):
+! CHECK: %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+! CHECK: fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+! CHECK: omp.yield(%{{.*}} : !fir.ref<i32>)
+! CHECK: }
+
+! CHECK-LABEL: omp.private {type = firstprivate} @_QFExfpvx_firstprivate_i32 : i32 copy {
+! CHECK: ^bb0(%{{.*}}: !fir.ref<i32>, %{{.*}}: !fir.ref<i32>):
+! CHECK: %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+! CHECK: fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+! CHECK: omp.yield(%{{.*}} : !fir.ref<i32>)
+! CHECK: }
+
+! CHECK: %[[VAL_0:.*]] = fir.alloca i32 {bindc_name = "xdgfx", uniq_name = "_QFExdgfx"}
+! CHECK: %[[VAL_1:.*]] = fir.declare %[[VAL_0]] {uniq_name = "_QFExdgfx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+! CHECK: %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "xfpvx", uniq_name = "_QFExfpvx"}
+! CHECK: %[[VAL_3:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFExfpvx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+! CHECK: %[[VAL_4:.*]] = omp.map.info var_ptr(%[[VAL_3]] : !fir.ref<i32>, i32) map_clauses(to) capture(ByRef) -> !fir.ref<i32>
+! CHECK: %[[VAL_5:.*]] = omp.map.info var_ptr(%[[VAL_1]] : !fir.ref<i32>, i32) map_clauses(to) capture(ByRef) -> !fir.ref<i32>
+
+! CHECK: omp.target map_entries(%[[VAL_4]] -> %{{.*}}, %[[VAL_5]] -> %{{.*}} : !fir.ref<i32>, !fir.ref<i32>) private(@_QFExfpvx_firstprivate_i32 %[[VAL_3]] -> %[[VAL_6:.*]] [map_idx=0], @_QFExdgfx_firstprivate_i32 %[[VAL_1]] -> %[[VAL_7:.*]] [map_idx=1] : !fir.ref<i32>, !fir.ref<i32>) {
+! CHECK: %{{.*}} = fir.declare %[[VAL_6]] {uniq_name = "_QFExfpvx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+! CHECK: %{{.*}} = fir.declare %[[VAL_7]] {uniq_name = "_QFExdgfx"} : (!fir.ref<i32>) -> !fir.ref<i32>
+
+program test_default_implicit_firstprivate
+ implicit none
+ integer :: xdgfx, xfpvx
+ xdgfx = 1
+ xfpvx = 2
+ !$omp target firstprivate(xfpvx)
+ xdgfx = 42
+ xfpvx = 43
+ !$omp end target
+ write(*,*) xdgfx, xfpvx
+end program
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 217ac5638a3ea..e7e3fb2097308 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90
@@ -139,28 +139,28 @@ end subroutine target_allocatable
! CHECK: %[[REAL_ARR_ALLOC:.*]] = fir.alloca !fir.array<?xf32>, {{.*}} {bindc_name = "real_arr", {{.*}}}
! CHECK: %[[REAL_ARR_DECL:.*]]:2 = hlfir.declare %[[REAL_ARR_ALLOC]]({{.*}})
! CHECK: fir.store %[[REAL_ARR_DECL]]#0 to %[[REAL_ARR_DESC_ALLOCA]] : !fir.ref<!fir.box<!fir.array<?xf32>>>
-! CHECK: %[[MAPPED_MI0:.*]] = omp.map.info var_ptr(%[[MAPPED_DECL]]#1 : !fir.ref<i32>, i32) {{.*}}
! CHECK: %[[ALLOC_VAR_MEMBER:.*]] = omp.map.info var_ptr(%[[ALLOC_VAR_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<i32>>>, i32)
! CHECK: %[[ALLOC_VAR_MAP:.*]] = omp.map.info var_ptr(%[[ALLOC_VAR_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.box<!fir.heap<i32>>) {{.*}} members(%[[ALLOC_VAR_MEMBER]] :
! CHECK: %[[REAL_ARR_MEMBER:.*]] = omp.map.info var_ptr(%[[REAL_ARR_DESC_ALLOCA]] : !fir.ref<!fir.box<!fir.array<?xf32>>>, f32)
! CHECK: %[[REAL_ARR_DESC_MAP:.*]] = omp.map.info var_ptr(%[[REAL_ARR_DESC_ALLOCA]] : !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.box<!fir.array<?xf32>>) {{.*}} members(%[[REAL_ARR_MEMBER]] :
! CHECK: fir.store %[[CHAR_VAR_DECL]]#0 to %[[CHAR_VAR_DESC_ALLOCA]] : !fir.ref<!fir.boxchar<1>>
! CHECK: %[[CHAR_VAR_DESC_MAP:.*]] = omp.map.info var_ptr(%[[CHAR_VAR_DESC_ALLOCA]] : !fir.ref<!fir.boxchar<1>>, !fir.boxchar<1>)
+! CHECK: %[[MAPPED_MI0:.*]] = omp.map.info var_ptr(%[[MAPPED_DECL]]#0 : !fir.ref<i32>, i32) {{.*}}
! CHECK: omp.target
! CHECK-SAME: map_entries(
-! CHECK-SAME: %[[MAPPED_MI0]] -> %[[MAPPED_ARG0:[^,]+]],
-! CHECK-SAME: %[[ALLOC_VAR_MAP]] -> %[[MAPPED_ARG1:[^,]+]]
+! CHECK-SAME: %[[ALLOC_VAR_MAP]] -> %[[MAPPED_ARG1:[^,]+]],
! CHECK-SAME: %[[REAL_ARR_DESC_MAP]] -> %[[MAPPED_ARG2:[^,]+]]
! CHECK-SAME: %[[CHAR_VAR_DESC_MAP]] -> %[[MAPPED_ARG3:.[^,]+]]
-! CHECK-SAME: !fir.ref<i32>, !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<!fir.boxchar<1>>, !fir.llvm_ptr<!fir.ref<i32>>, !fir.llvm_ptr<!fir.ref<!fir.array<?xf32>>>
+! CHECK-SAME: %[[MAPPED_MI0]] -> %[[MAPPED_ARG0:[^,]+]]
+! CHECK-SAME: !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<!fir.boxchar<1>>, !fir.ref<i32>, !fir.llvm_ptr<!fir.ref<i32>>
! CHECK-SAME: private(
-! CHECK-SAME: @[[ALLOC_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[ALLOC_ARG:[^,]+]] [map_idx=1],
+! CHECK-SAME: @[[ALLOC_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[ALLOC_ARG:[^,]+]] [map_idx=0],
! CHECK-SAME: @[[REAL_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[REAL_ARG:[^,]+]],
! CHECK-SAME: @[[LB_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[LB_ARG:[^,]+]],
-! CHECK-SAME: @[[ARR_PRIVATIZER_SYM]] %{{[^[:space:]]+}} -> %[[ARR_ARG:[^,]+]] [map_idx=2],
+! CHECK-SAME: @[[ARR_PRIVATIZER_SYM]] %{{[^[:space:]]+}} -> %[[ARR_ARG:[^,]+]] [map_idx=1],
! CHECK-SAME: @[[COMP_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[COMP_ARG:[^,]+]],
-! CHECK-SAME: @[[CHAR_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[CHAR_ARG:[^,]+]] [map_idx=3] :
-! CHECK-SAME: !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<f32>, !fir.ref<i64>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<complex<f32>>, !fir.boxchar<1>) {
+! CHECK-SAME: @[[CHAR_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[CHAR_ARG:[^,]+]] [map_idx=2],
+! CHECK-SAME: !fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<f32>, !fir.ref<i64>, !fir.ref<!fir.box<!fir.array<?xf32>>>, !fir.ref<complex<f32>>, !fir.boxchar<1>, !fir.ref<i32>) {
! CHECK-NOT: fir.alloca
! CHECK: hlfir.declare %[[ALLOC_ARG]]
! CHECK: hlfir.declare %[[REAL_ARG]]
|
|
Please feel free to add more people to the review list (or add/remove yourself if you wish), wasn't sure who were the best people to review this as it's not my usual little segment! |
luporl
reviewed
Jul 8, 2025
| // implicitly firstprivitizeable under OpenMP restrictions. | ||
| // TODO: Relax restriction as we progress privitization and further | ||
| // investigate the flags we can intermix with. | ||
| if (!dsa.none() || !symbol.flags().none() || |
There was a problem hiding this comment.
It's possible that other flags not related to DSA may be set.
If you want to consider only DSA and data-mapping related flags, you can use:
(dsa & (dataSharingAttributeFlags | dataMappingAttributeFlags)).none()
Note that dataMappingAttributeFlags would need to be moved from OmpAttributeVisitor::ResolveOmpObject() to OmpAttributeVisitor.
| // 4) not mapped target variable -> firstprivate | ||
| // - i.e. implicit, but meets OpenMP specification rules for | ||
| // firstprivate "promotion" | ||
| if (enableDelayedPrivatizationStaging && symbol && |
There was a problem hiding this comment.
symbol can't be NULL, as that is one of the conditions checked before calling CreateImplicitSymbols.
In the future, it would be good to change its symbol parameter to a reference.
| prevDSA.set(Symbol::Flag::OmpImplicit); | ||
| prevDSA.set(Symbol::Flag::OmpFirstPrivate); | ||
| makeSymbol(prevDSA); | ||
| } |
There was a problem hiding this comment.
Please add this line, to keep debug messages consistent:
PRINT_IMPLICIT_RULE("4) not mapped target variable -> firstprivate");
|
Thank you very much for the quick review @luporl ! Ill update this PR in the next week or so with the review points. |
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.
Currently, we indicate to the runtime that implicit scalar captures are firstprivate (via map and
capture types), enough for the runtime trace to treat it as such, but we do not CodeGen the IR
in such a way that we can take full advantage of this aspect of the OpenMP specification.
This patch seeks to change that by applying the correct symbol flags (firstprivate/implicit) to the
implicitly captured scalars within target regions, which then triggers the delayed privitization code
generation for these symbols, bringing the code generation in-line with the explicit firstpriviate
clause. Currently, similarly to the delayed privitization I have sheltered this segment of code
behind the EnabledDelayedPrivitization flag, as without it, we'll trigger an compiler error for
firstprivate not being supported any time we implicitly capture a scalar and try to firstprivitize
it, in future when this flag is removed it can also be removed here. So, for now, you need to
enable this via providing the compiler the flag on compilation of any programs.