[flang][OpenMP] Improve locality check when determining DSA (#180583)
Follow-up to https://github.com/llvm/llvm-project/pull/178739. The locality check assumed that immediately after the initial symbol resolution (i.e. prior to the OpenMP code in resolve-directives.cpp), the scope that owns a given symbol is the scope which owns the symbol's storage. Turns out that this isn't necessarily true as illustrated by the included testcase, roughly something like: ``` program main integer :: j ! host j (storage-owning) contains subroutine f !$omp parallel ! scope that owns j, but j is host-associated do j = ... end do !$omp end parallel end end program ``` In such cases, the locality should be checked for the symbol that owns storage, i.e. a clone of the symbol that is has been privatized or a symbol that is not host- or use-associated. This is similar to obtaning the ultimate symbol (i.e. from the end of association chain), except the chain traversal would stop at a privatized symbol, potentially before reaching the end. This fixes a few regressions in the Fujitsu test suite: Fujitsu/Fortran/0160/Fujitsu-Fortran-0160_0000.test Fujitsu/Fortran/0160/Fujitsu-Fortran-0160_0012.test Fujitsu/Fortran/0160/Fujitsu-Fortran-0160_0013.test Fujitsu/Fortran/0660/Fujitsu-Fortran-0660_0096.test Fujitsu/Fortran/0660/Fujitsu-Fortran-0660_0097.test Fujitsu/Fortran/1052/Fujitsu-Fortran-1052_0108.test Fujitsu/Fortran/1052/Fujitsu-Fortran-1052_0112.test
This commit is contained in:
parent
8f37bf6b10
commit
0e7ddf395a
@ -421,6 +421,37 @@ public:
|
||||
ultSym.flags().test(Symbol::Flag::InCommonBlock);
|
||||
}
|
||||
|
||||
static const Symbol &GetStorageOwner(const Symbol &symbol) {
|
||||
static auto getParent = [](const Symbol *s) -> const Symbol * {
|
||||
if (auto *details{s->detailsIf<UseDetails>()}) {
|
||||
return &details->symbol();
|
||||
} else if (auto *details{s->detailsIf<HostAssocDetails>()}) {
|
||||
return &details->symbol();
|
||||
} else {
|
||||
return nullptr;
|
||||
}
|
||||
};
|
||||
static auto isPrivate = [](const Symbol &symbol) {
|
||||
static const Symbol::Flags privatizing{Symbol::Flag::OmpPrivate,
|
||||
Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
|
||||
Symbol::Flag::OmpLinear};
|
||||
return (symbol.flags() & privatizing).any();
|
||||
};
|
||||
|
||||
const Symbol *sym = &symbol;
|
||||
while (true) {
|
||||
if (isPrivate(*sym)) {
|
||||
return *sym;
|
||||
}
|
||||
if (const Symbol *parent{getParent(sym)}) {
|
||||
sym = parent;
|
||||
} else {
|
||||
return *sym;
|
||||
}
|
||||
}
|
||||
llvm_unreachable("Error while looking for storage owning symbol");
|
||||
}
|
||||
|
||||
// Recognize symbols that are not created as a part of the OpenMP data-
|
||||
// sharing processing, and that are declared inside of the construct.
|
||||
// These symbols are predetermined private, but they shouldn't be marked
|
||||
@ -428,8 +459,13 @@ public:
|
||||
// They are not symbols for which private copies need to be created,
|
||||
// they are already themselves private.
|
||||
static bool IsLocalInsideScope(const Symbol &symbol, const Scope &scope) {
|
||||
return symbol.owner() != scope && scope.Contains(symbol.owner()) &&
|
||||
!HasStaticStorageDuration(symbol);
|
||||
// A symbol that is marked with a DSA will be cloned in the construct
|
||||
// scope and marked as host-associated. This applies to privatized symbols
|
||||
// as well even though they will have their own storage. They should be
|
||||
// considered local regardless of the status of the original symbol.
|
||||
const Symbol &actual{GetStorageOwner(symbol)};
|
||||
return actual.owner() != scope && scope.Contains(actual.owner()) &&
|
||||
!HasStaticStorageDuration(actual);
|
||||
}
|
||||
|
||||
template <typename A> void Walk(const A &x) { parser::Walk(x, *this); }
|
||||
|
||||
52
flang/test/Semantics/OpenMP/local-variables-2.f90
Normal file
52
flang/test/Semantics/OpenMP/local-variables-2.f90
Normal file
@ -0,0 +1,52 @@
|
||||
!RUN: %flang_fc1 -fdebug-unparse-with-symbols -fopenmp -fopenmp-version=60 %s | FileCheck %s
|
||||
|
||||
! Shortened version of Fujitsu/Fortran/0160/0160_0000.f90
|
||||
! Make sure that j is privatized.
|
||||
|
||||
!CHECK-LABEL: !DEF: /MAIN MainProgram
|
||||
!CHECK-NEXT: program MAIN
|
||||
!CHECK-NEXT: implicit none
|
||||
!CHECK-NEXT: !DEF: /MAIN/j ObjectEntity INTEGER(4)
|
||||
!CHECK-NEXT: !DEF: /MAIN/k ObjectEntity INTEGER(4)
|
||||
!CHECK-NEXT: !DEF: /MAIN/ndim ObjectEntity INTEGER(4)
|
||||
!CHECK-NEXT: integer j, k, ndim
|
||||
!CHECK-NEXT: !DEF: /MAIN/flux (Subroutine) Subprogram
|
||||
!CHECK-NEXT: call flux
|
||||
!CHECK-NEXT: contains
|
||||
!CHECK-NEXT: !REF: /MAIN/flux
|
||||
!CHECK-NEXT: subroutine flux
|
||||
!CHECK-NEXT: !$omp parallel
|
||||
!CHECK-NEXT: !$omp do
|
||||
!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
|
||||
!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/ndim HostAssoc INTEGER(4)
|
||||
!CHECK-NEXT: do k=-1,ndim+1
|
||||
!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
|
||||
!CHECK-NEXT: !REF: /MAIN/flux/OtherConstruct1/OtherConstruct1/ndim
|
||||
!CHECK-NEXT: do j=-1,ndim+1
|
||||
!CHECK-NEXT: end do
|
||||
!CHECK-NEXT: end do
|
||||
!CHECK-NEXT: !$omp end do
|
||||
!CHECK-NEXT: !$omp end parallel
|
||||
!CHECK-NEXT: end subroutine flux
|
||||
!CHECK-NEXT: end program MAIN
|
||||
|
||||
program main
|
||||
implicit none
|
||||
integer :: j, k, ndim
|
||||
|
||||
call flux()
|
||||
|
||||
contains
|
||||
|
||||
subroutine flux
|
||||
!$omp parallel
|
||||
!$omp do
|
||||
do k = -1, ndim + 1
|
||||
do j = -1, ndim + 1
|
||||
enddo
|
||||
enddo
|
||||
!$omp end do
|
||||
!$omp end parallel
|
||||
end subroutine flux
|
||||
|
||||
end program main
|
||||
Loading…
x
Reference in New Issue
Block a user