Author: Krzysztof Parzyszek Date: 2026-02-11T09:58:11Z New Revision: ece1c94778eeb06af23dfc0a3afebb4a1039ad47
URL: https://github.com/llvm/llvm-project/commit/ece1c94778eeb06af23dfc0a3afebb4a1039ad47 DIFF: https://github.com/llvm/llvm-project/commit/ece1c94778eeb06af23dfc0a3afebb4a1039ad47.diff LOG: [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 (cherry picked from commit 0e7ddf395a12e0201ed0ca7131439c2fd355a72a) Added: flang/test/Semantics/OpenMP/local-variables-1.f90 flang/test/Semantics/OpenMP/local-variables-2.f90 Modified: flang/lib/Semantics/resolve-directives.cpp Removed: flang/test/Semantics/OpenMP/local-variables.f90 ################################################################################ diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 9f2512713015c..45592b76dbfa6 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -400,6 +400,37 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { 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 @@ -407,8 +438,13 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { // 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); } diff --git a/flang/test/Semantics/OpenMP/local-variables.f90 b/flang/test/Semantics/OpenMP/local-variables-1.f90 similarity index 100% rename from flang/test/Semantics/OpenMP/local-variables.f90 rename to flang/test/Semantics/OpenMP/local-variables-1.f90 diff --git a/flang/test/Semantics/OpenMP/local-variables-2.f90 b/flang/test/Semantics/OpenMP/local-variables-2.f90 new file mode 100644 index 0000000000000..1924b5782d16d --- /dev/null +++ b/flang/test/Semantics/OpenMP/local-variables-2.f90 @@ -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 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
