Skip to content

Commit 0e7ddf3

Browse files
authored
[flang][OpenMP] Improve locality check when determining DSA (llvm#180583)
Follow-up to llvm#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
1 parent 8f37bf6 commit 0e7ddf3

3 files changed

Lines changed: 90 additions & 2 deletions

File tree

flang/lib/Semantics/resolve-directives.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,15 +421,51 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
421421
ultSym.flags().test(Symbol::Flag::InCommonBlock);
422422
}
423423

424+
static const Symbol &GetStorageOwner(const Symbol &symbol) {
425+
static auto getParent = [](const Symbol *s) -> const Symbol * {
426+
if (auto *details{s->detailsIf<UseDetails>()}) {
427+
return &details->symbol();
428+
} else if (auto *details{s->detailsIf<HostAssocDetails>()}) {
429+
return &details->symbol();
430+
} else {
431+
return nullptr;
432+
}
433+
};
434+
static auto isPrivate = [](const Symbol &symbol) {
435+
static const Symbol::Flags privatizing{Symbol::Flag::OmpPrivate,
436+
Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
437+
Symbol::Flag::OmpLinear};
438+
return (symbol.flags() & privatizing).any();
439+
};
440+
441+
const Symbol *sym = &symbol;
442+
while (true) {
443+
if (isPrivate(*sym)) {
444+
return *sym;
445+
}
446+
if (const Symbol *parent{getParent(sym)}) {
447+
sym = parent;
448+
} else {
449+
return *sym;
450+
}
451+
}
452+
llvm_unreachable("Error while looking for storage owning symbol");
453+
}
454+
424455
// Recognize symbols that are not created as a part of the OpenMP data-
425456
// sharing processing, and that are declared inside of the construct.
426457
// These symbols are predetermined private, but they shouldn't be marked
427458
// in any special way, because there is nothing to be done for them.
428459
// They are not symbols for which private copies need to be created,
429460
// they are already themselves private.
430461
static bool IsLocalInsideScope(const Symbol &symbol, const Scope &scope) {
431-
return symbol.owner() != scope && scope.Contains(symbol.owner()) &&
432-
!HasStaticStorageDuration(symbol);
462+
// A symbol that is marked with a DSA will be cloned in the construct
463+
// scope and marked as host-associated. This applies to privatized symbols
464+
// as well even though they will have their own storage. They should be
465+
// considered local regardless of the status of the original symbol.
466+
const Symbol &actual{GetStorageOwner(symbol)};
467+
return actual.owner() != scope && scope.Contains(actual.owner()) &&
468+
!HasStaticStorageDuration(actual);
433469
}
434470

435471
template <typename A> void Walk(const A &x) { parser::Walk(x, *this); }
File renamed without changes.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
!RUN: %flang_fc1 -fdebug-unparse-with-symbols -fopenmp -fopenmp-version=60 %s | FileCheck %s
2+
3+
! Shortened version of Fujitsu/Fortran/0160/0160_0000.f90
4+
! Make sure that j is privatized.
5+
6+
!CHECK-LABEL: !DEF: /MAIN MainProgram
7+
!CHECK-NEXT: program MAIN
8+
!CHECK-NEXT: implicit none
9+
!CHECK-NEXT: !DEF: /MAIN/j ObjectEntity INTEGER(4)
10+
!CHECK-NEXT: !DEF: /MAIN/k ObjectEntity INTEGER(4)
11+
!CHECK-NEXT: !DEF: /MAIN/ndim ObjectEntity INTEGER(4)
12+
!CHECK-NEXT: integer j, k, ndim
13+
!CHECK-NEXT: !DEF: /MAIN/flux (Subroutine) Subprogram
14+
!CHECK-NEXT: call flux
15+
!CHECK-NEXT: contains
16+
!CHECK-NEXT: !REF: /MAIN/flux
17+
!CHECK-NEXT: subroutine flux
18+
!CHECK-NEXT: !$omp parallel
19+
!CHECK-NEXT: !$omp do
20+
!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/k (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
21+
!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/ndim HostAssoc INTEGER(4)
22+
!CHECK-NEXT: do k=-1,ndim+1
23+
!CHECK-NEXT: !DEF: /MAIN/flux/OtherConstruct1/OtherConstruct1/j (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
24+
!CHECK-NEXT: !REF: /MAIN/flux/OtherConstruct1/OtherConstruct1/ndim
25+
!CHECK-NEXT: do j=-1,ndim+1
26+
!CHECK-NEXT: end do
27+
!CHECK-NEXT: end do
28+
!CHECK-NEXT: !$omp end do
29+
!CHECK-NEXT: !$omp end parallel
30+
!CHECK-NEXT: end subroutine flux
31+
!CHECK-NEXT: end program MAIN
32+
33+
program main
34+
implicit none
35+
integer :: j, k, ndim
36+
37+
call flux()
38+
39+
contains
40+
41+
subroutine flux
42+
!$omp parallel
43+
!$omp do
44+
do k = -1, ndim + 1
45+
do j = -1, ndim + 1
46+
enddo
47+
enddo
48+
!$omp end do
49+
!$omp end parallel
50+
end subroutine flux
51+
52+
end program main

0 commit comments

Comments
 (0)