Skip to content

[Flang][OpenMP] Make implicitly captured scalars fully firstprivatized #147442

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion flang/lib/Semantics/resolve-directives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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() ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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;
Expand Down Expand Up @@ -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 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

IsTargetCaptureImplicitlyFirstPrivatizeable(*symbol, prevDSA)) {
prevDSA.set(Symbol::Flag::OmpImplicit);
prevDSA.set(Symbol::Flag::OmpFirstPrivate);
makeSymbol(prevDSA);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this line, to keep debug messages consistent:
PRINT_IMPLICIT_RULE("4) not mapped target variable -> firstprivate");

dsa = prevDSA;
} else if (taskGenDir) {
// TODO 5) dummy arg in orphaned taskgen construct -> firstprivate
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
Loading