From 9314e8e3a212eccb473c2269b6402f84fbcef0dd Mon Sep 17 00:00:00 2001 From: agozillon Date: Mon, 7 Jul 2025 20:01:28 -0500 Subject: [PATCH] [Flang][OpenMP] Make implicitly captured scalars fully firstprivatized 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. --- flang/lib/Semantics/resolve-directives.cpp | 39 ++++++++++++++++- .../target-private-implicit-scalar-map.f90 | 42 +++++++++++++++++++ .../target-private-multiple-variables.f90 | 16 +++---- 3 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map.f90 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 @@ -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, %{{.*}}: !fir.ref): +! CHECK: %{{.*}} = fir.load %{{.*}} : !fir.ref +! CHECK: fir.store %{{.*}} to %{{.*}} : !fir.ref +! CHECK: omp.yield(%{{.*}} : !fir.ref) +! CHECK: } + +! CHECK-LABEL: omp.private {type = firstprivate} @_QFExfpvx_firstprivate_i32 : i32 copy { +! CHECK: ^bb0(%{{.*}}: !fir.ref, %{{.*}}: !fir.ref): +! CHECK: %{{.*}} = fir.load %{{.*}} : !fir.ref +! CHECK: fir.store %{{.*}} to %{{.*}} : !fir.ref +! CHECK: omp.yield(%{{.*}} : !fir.ref) +! 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) -> !fir.ref +! CHECK: %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "xfpvx", uniq_name = "_QFExfpvx"} +! CHECK: %[[VAL_3:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFExfpvx"} : (!fir.ref) -> !fir.ref +! CHECK: %[[VAL_4:.*]] = omp.map.info var_ptr(%[[VAL_3]] : !fir.ref, i32) map_clauses(to) capture(ByRef) -> !fir.ref +! CHECK: %[[VAL_5:.*]] = omp.map.info var_ptr(%[[VAL_1]] : !fir.ref, i32) map_clauses(to) capture(ByRef) -> !fir.ref + +! CHECK: omp.target map_entries(%[[VAL_4]] -> %{{.*}}, %[[VAL_5]] -> %{{.*}} : !fir.ref, !fir.ref) private(@_QFExfpvx_firstprivate_i32 %[[VAL_3]] -> %[[VAL_6:.*]] [map_idx=0], @_QFExdgfx_firstprivate_i32 %[[VAL_1]] -> %[[VAL_7:.*]] [map_idx=1] : !fir.ref, !fir.ref) { +! CHECK: %{{.*}} = fir.declare %[[VAL_6]] {uniq_name = "_QFExfpvx"} : (!fir.ref) -> !fir.ref +! CHECK: %{{.*}} = fir.declare %[[VAL_7]] {uniq_name = "_QFExdgfx"} : (!fir.ref) -> !fir.ref + +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, {{.*}} {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>> -! CHECK: %[[MAPPED_MI0:.*]] = omp.map.info var_ptr(%[[MAPPED_DECL]]#1 : !fir.ref, i32) {{.*}} ! CHECK: %[[ALLOC_VAR_MEMBER:.*]] = omp.map.info var_ptr(%[[ALLOC_VAR_DECL]]#0 : !fir.ref>>, i32) ! CHECK: %[[ALLOC_VAR_MAP:.*]] = omp.map.info var_ptr(%[[ALLOC_VAR_DECL]]#0 : !fir.ref>>, !fir.box>) {{.*}} members(%[[ALLOC_VAR_MEMBER]] : ! CHECK: %[[REAL_ARR_MEMBER:.*]] = omp.map.info var_ptr(%[[REAL_ARR_DESC_ALLOCA]] : !fir.ref>>, f32) ! CHECK: %[[REAL_ARR_DESC_MAP:.*]] = omp.map.info var_ptr(%[[REAL_ARR_DESC_ALLOCA]] : !fir.ref>>, !fir.box>) {{.*}} members(%[[REAL_ARR_MEMBER]] : ! CHECK: fir.store %[[CHAR_VAR_DECL]]#0 to %[[CHAR_VAR_DESC_ALLOCA]] : !fir.ref> ! CHECK: %[[CHAR_VAR_DESC_MAP:.*]] = omp.map.info var_ptr(%[[CHAR_VAR_DESC_ALLOCA]] : !fir.ref>, !fir.boxchar<1>) +! CHECK: %[[MAPPED_MI0:.*]] = omp.map.info var_ptr(%[[MAPPED_DECL]]#0 : !fir.ref, 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, !fir.ref>>, !fir.ref>>, !fir.ref>, !fir.llvm_ptr>, !fir.llvm_ptr>> +! CHECK-SAME: %[[MAPPED_MI0]] -> %[[MAPPED_ARG0:[^,]+]] +! CHECK-SAME: !fir.ref>>, !fir.ref>>, !fir.ref>, !fir.ref, !fir.llvm_ptr> ! 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.ref, !fir.ref, !fir.ref>>, !fir.ref>, !fir.boxchar<1>) { +! CHECK-SAME: @[[CHAR_PRIVATIZER_SYM]] %{{[^[:space:]]+}}#0 -> %[[CHAR_ARG:[^,]+]] [map_idx=2], +! CHECK-SAME: !fir.ref>>, !fir.ref, !fir.ref, !fir.ref>>, !fir.ref>, !fir.boxchar<1>, !fir.ref) { ! CHECK-NOT: fir.alloca ! CHECK: hlfir.declare %[[ALLOC_ARG]] ! CHECK: hlfir.declare %[[REAL_ARG]]