[flang-commits] [flang] 3beee85 - [flang][OpenMP] Fix declare reduction accessibility in module scope (#197078)
via flang-commits
flang-commits at lists.llvm.org
Thu May 28 08:16:52 PDT 2026
Author: Matt
Date: 2026-05-28T10:16:46-05:00
New Revision: 3beee85e5244bc70fa32210d89634c0e53d2606d
URL: https://github.com/llvm/llvm-project/commit/3beee85e5244bc70fa32210d89634c0e53d2606d
DIFF: https://github.com/llvm/llvm-project/commit/3beee85e5244bc70fa32210d89634c0e53d2606d.diff
LOG: [flang][OpenMP] Fix declare reduction accessibility in module scope (#197078)
Fix four interacting issues with OpenMP declare reduction accessibility
when reductions are declared in Fortran modules:
1. Accessibility propagation (resolve-names.cpp): Reduction symbols like
`op.+` had no linkage to the corresponding `operator(+)` accessibility.
`ApplyDefaultAccess()` now reverse-maps mangled names to their Fortran
identifiers and inherits operator/procedure accessibility.
2. USE-associated duplicate detection (resolve-names.cpp):
`FindSymbol()`
searched parent scopes and found USE-associated symbols, causing false
"Duplicate definition" errors. Changed to scope-local `FindInScope()`
with proper `UseDetails` handling that shadows USE symbols.
3. Module file serialization (mod-file.cpp): `PutUserReduction()` never
emitted accessibility, so PRIVATE was lost on module file round-trips.
Now emits `private::<identifier>` when no GenericDetails symbol already
carries PRIVATE (avoiding duplicates with PutGeneric output).
4. Reduction clause checking (check-omp-structure.cpp):
`CheckSymbolSupportsType()` scanned all module scopes ignoring
accessibility. Now skips PRIVATE reductions in the module scope scan.
Also fixes a pre-existing bug in `MakeNameFromOperator()` where the
CharBlock lengths for OR, EQV, and NEQV included the null terminator
(6/7/8 instead of 5/6/7), causing silent mismatches in StringSwitch
comparisons.
Note: `CheckSymbolSupportsType` still scans all global module scopes
rather than only USE-reachable ones. This pre-existing over-broad lookup
is improved by the PRIVATE filter added here but a proper
scope-restricted resolution is left as future work.
Fixes #187415
Related: #192580
Assisted-by: Claude Opus 4.6.
Co-authored-by: Matt P. Dziubinski <matt-p.dziubinski at hpe.com>
Added:
flang/test/Semantics/OpenMP/declare-reduction-accessibility.f90
flang/test/Semantics/OpenMP/declare-reduction-default-private.f90
flang/test/Semantics/OpenMP/declare-reduction-modfile-private.f90
flang/test/Semantics/OpenMP/declare-reduction-public-regression.f90
flang/test/Semantics/OpenMP/declare-reduction-use-assoc.f90
Modified:
flang/lib/Semantics/check-omp-structure.cpp
flang/lib/Semantics/mod-file.cpp
flang/lib/Semantics/resolve-names.cpp
Removed:
################################################################################
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index a1961ee8105e7..ff41f49d88b32 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3954,12 +3954,16 @@ static bool CheckSymbolSupportsType(const Scope &scope,
}
}
// Look through module scopes in the global scope.
- // This covers reductions declared in a module and used via USE association
+ // This covers reductions declared in a module and used via USE association.
const SemanticsContext &semCtx{scope.context()};
Scope &global = const_cast<SemanticsContext &>(semCtx).globalScope();
for (const Scope &child : global.children()) {
if (child.kind() == Scope::Kind::Module) {
if (const auto *symbol{child.FindSymbol(name)}) {
+ // Skip PRIVATE reductions that aren't visible in the current scope.
+ if (symbol->attrs().test(Attr::PRIVATE)) {
+ continue;
+ }
if (const auto *reductionDetails{
symbol->detailsIf<UserReductionDetails>()}) {
return reductionDetails->SupportsType(type);
diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp
index 974e88caebc9f..f5e66a04c3f11 100644
--- a/flang/lib/Semantics/mod-file.cpp
+++ b/flang/lib/Semantics/mod-file.cpp
@@ -17,6 +17,8 @@
#include "flang/Semantics/semantics.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/Frontend/OpenMP/OMP.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -1110,6 +1112,33 @@ void ModFileWriter::PutTypeParam(llvm::raw_ostream &os, const Symbol &symbol) {
os << '\n';
}
+// Map a mangled reduction name to a valid Fortran accessibility identifier
+// for module file serialization (e.g., op.+ → operator(+), op.max → max).
+// Non-mangled names (procedure designators) are returned as-is.
+static std::string GetReductionFortranId(const SourceName &mangledName) {
+ llvm::StringRef name{mangledName.begin(), mangledName.size()};
+ if (!name.starts_with("op.")) {
+ return name.str();
+ }
+ llvm::StringRef suffix{name.drop_front(3)};
+ if (suffix == "+" || suffix == "-" || suffix == "*") {
+ return ("operator(" + suffix + ")").str();
+ }
+ llvm::StringRef logicalOp{llvm::StringSwitch<llvm::StringRef>(suffix)
+ .Case("AND", ".and.")
+ .Case("OR", ".or.")
+ .Case("EQV", ".eqv.")
+ .Case("NEQV", ".neqv.")
+ .Default("")};
+ if (!logicalOp.empty()) {
+ return ("operator(" + logicalOp + ")").str();
+ }
+ if (suffix.size() > 2 && suffix.front() == '.' && suffix.back() == '.') {
+ return ("operator(" + suffix + ")").str();
+ }
+ return suffix.str();
+}
+
void ModFileWriter::PutUserReduction(
llvm::raw_ostream &os, const Symbol &symbol) {
const auto &details{symbol.get<UserReductionDetails>()};
@@ -1119,6 +1148,25 @@ void ModFileWriter::PutUserReduction(
for (const auto *decl : details.GetDeclList()) {
Unparse(os, *decl, context_.langOptions());
}
+ // Emit a Fortran accessibility statement for the reduction identifier
+ // so that PRIVATE survives module file round-trips. Only needed when
+ // there is no corresponding generic interface that already carries
+ // PRIVATE (PutGeneric handles that case).
+ if (!isSubmodule_ && symbol.attrs().test(Attr::PRIVATE)) {
+ std::string fortranId{GetReductionFortranId(symbol.name())};
+ if (!fortranId.empty()) {
+ bool alreadyEmitted{false};
+ parser::CharBlock cb{fortranId};
+ auto it{symbol.owner().find(cb)};
+ if (it != symbol.owner().end() &&
+ it->second->detailsIf<GenericDetails>()) {
+ alreadyEmitted = it->second->attrs().test(Attr::PRIVATE);
+ }
+ if (!alreadyEmitted) {
+ os << "private::" << fortranId << '\n';
+ }
+ }
+ }
}
static void PutMapper(
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index fc7cc63144283..26fd702e248ae 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -2015,11 +2015,11 @@ parser::CharBlock MakeNameFromOperator(
case parser::DefinedOperator::IntrinsicOperator::AND:
return parser::CharBlock{"op.AND", 6};
case parser::DefinedOperator::IntrinsicOperator::OR:
- return parser::CharBlock{"op.OR", 6};
+ return parser::CharBlock{"op.OR", 5};
case parser::DefinedOperator::IntrinsicOperator::EQV:
- return parser::CharBlock{"op.EQV", 7};
+ return parser::CharBlock{"op.EQV", 6};
case parser::DefinedOperator::IntrinsicOperator::NEQV:
- return parser::CharBlock{"op.NEQV", 8};
+ return parser::CharBlock{"op.NEQV", 7};
default:
context.Say("Unsupported operator in DECLARE REDUCTION"_err_en_US);
@@ -2074,17 +2074,24 @@ void OmpVisitor::ProcessReductionSpecifier(
// the first, or only, instance with this name). The details then
// gets stored in the symbol when it's created.
UserReductionDetails *reductionDetails{&reductionDetailsTemp};
- Symbol *symbol{currScope().FindSymbol(mangledName)};
+ // Use scope-local lookup to avoid false matches from parent scopes.
+ Symbol *symbol{FindInScope(currScope(), mangledName)};
if (symbol) {
- // If we found a symbol, we append the type info to the
- // existing reductionDetails.
- reductionDetails = symbol->detailsIf<UserReductionDetails>();
-
- if (!reductionDetails) {
- context().Say(
- "Duplicate definition of '%s' in DECLARE REDUCTION"_err_en_US,
- mangledName);
- return;
+ if (symbol->detailsIf<UseDetails>()) {
+ // USE-associated reduction: shadow it with a new local declaration.
+ EraseSymbol(*symbol);
+ symbol = nullptr;
+ } else {
+ // If we found a local symbol, we append the type info to the
+ // existing reductionDetails.
+ reductionDetails = symbol->detailsIf<UserReductionDetails>();
+
+ if (!reductionDetails) {
+ context().Say(
+ "Duplicate definition of '%s' in DECLARE REDUCTION"_err_en_US,
+ mangledName);
+ return;
+ }
}
}
@@ -4463,6 +4470,38 @@ Scope *ModuleVisitor::FindModule(const parser::Name &name,
return scope;
}
+// Map a mangled declare reduction name (e.g., op.+, op.max, op..myop.) back
+// to the Fortran identifier that controls its accessibility in a module scope.
+// Intrinsic operators map to "operator(+)" etc., named functions to "max" etc.,
+// and defined operators to "operator(.myop.)" etc.
+static std::string GetReductionIdentifierName(const SourceName &mangledName) {
+ llvm::StringRef name{mangledName.begin(), mangledName.size()};
+ if (!name.starts_with("op.")) {
+ return {};
+ }
+ llvm::StringRef suffix{name.drop_front(3)};
+ // Intrinsic arithmetic operators: op.+ → operator(+)
+ if (suffix == "+" || suffix == "-" || suffix == "*") {
+ return ("operator(" + suffix + ")").str();
+ }
+ // Intrinsic logical operators (mangled uppercase, scope uses lowercase)
+ llvm::StringRef logicalOp{llvm::StringSwitch<llvm::StringRef>(suffix)
+ .Case("AND", ".and.")
+ .Case("OR", ".or.")
+ .Case("EQV", ".eqv.")
+ .Case("NEQV", ".neqv.")
+ .Default("")};
+ if (!logicalOp.empty()) {
+ return ("operator(" + logicalOp + ")").str();
+ }
+ // Defined operators: op..myop. → operator(.myop.)
+ if (suffix.size() > 2 && suffix.front() == '.' && suffix.back() == '.') {
+ return ("operator(" + suffix + ")").str();
+ }
+ // Named functions: op.max → max
+ return suffix.str();
+}
+
void ModuleVisitor::ApplyDefaultAccess() {
const auto *moduleDetails{
DEREF(currScope().symbol()).detailsIf<ModuleDetails>()};
@@ -4484,6 +4523,21 @@ void ModuleVisitor::ApplyDefaultAccess() {
attr = Attr::PRIVATE;
}
}
+ } else if (symbol.detailsIf<UserReductionDetails>()) {
+ // OpenMP 6.0 §7.6.14: A declare reduction directive that appears in
+ // a module has accessibility as if it were declared as a module entity.
+ // If the corresponding operator/procedure has explicit accessibility,
+ // the reduction inherits it.
+ std::string opName{GetReductionIdentifierName(symbol.name())};
+ if (!opName.empty()) {
+ if (auto *opSym{FindInScope(currScope(), SourceName{opName})}) {
+ if (opSym->attrs().test(Attr::PUBLIC)) {
+ attr = Attr::PUBLIC;
+ } else if (opSym->attrs().test(Attr::PRIVATE)) {
+ attr = Attr::PRIVATE;
+ }
+ }
+ }
}
SetImplicitAttr(symbol, attr);
}
diff --git a/flang/test/Semantics/OpenMP/declare-reduction-accessibility.f90 b/flang/test/Semantics/OpenMP/declare-reduction-accessibility.f90
new file mode 100644
index 0000000000000..8ab524d5866b7
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-reduction-accessibility.f90
@@ -0,0 +1,35 @@
+! RUN: not %flang_fc1 -fopenmp -fopenmp-version=52 %s 2>&1 | FileCheck %s
+
+! Test that PRIVATE operator accessibility is propagated to declare reduction
+! and enforced when the reduction is used from outside the module.
+! Related: https://github.com/llvm/llvm-project/issues/187415
+
+module m_private_op
+ type :: dt
+ integer :: val = 0
+ end type
+ private :: operator(+)
+ interface operator(+)
+ module procedure add_dt
+ end interface
+ !$omp declare reduction(+:dt:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=dt(0))
+contains
+ type(dt) function add_dt(a, b)
+ type(dt), intent(in) :: a, b
+ add_dt%val = a%val + b%val
+ end function
+end module
+
+program test_private_reduction
+ use m_private_op
+ type(dt) :: x
+ integer :: i
+ x = dt(0)
+ !CHECK: error: The type of 'x' is incompatible with the reduction operator.
+ !$omp parallel do reduction(+:x)
+ do i = 1, 10
+ x%val = x%val + 1
+ end do
+ !$omp end parallel do
+end program
diff --git a/flang/test/Semantics/OpenMP/declare-reduction-default-private.f90 b/flang/test/Semantics/OpenMP/declare-reduction-default-private.f90
new file mode 100644
index 0000000000000..075ca43f88500
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-reduction-default-private.f90
@@ -0,0 +1,36 @@
+! RUN: not %flang_fc1 -fopenmp -fopenmp-version=52 %s 2>&1 | FileCheck %s
+
+! Test that a module with default PRIVATE accessibility propagates
+! the PRIVATE attribute to declare reduction symbols.
+
+module m_default_private
+ private
+ type, public :: dt
+ integer :: val = 0
+ end type
+ interface operator(+)
+ module procedure add_dt
+ end interface
+ !$omp declare reduction(+:dt:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=dt(0))
+contains
+ type(dt) function add_dt(a, b)
+ type(dt), intent(in) :: a, b
+ add_dt%val = a%val + b%val
+ end function
+end module
+
+program test_default_private
+ use m_default_private
+ type(dt) :: x
+ integer :: i
+ x = dt(0)
+ ! The reduction should be PRIVATE because the module default is PRIVATE
+ ! and operator(+) has no explicit PUBLIC.
+ !CHECK: error: The type of 'x' is incompatible with the reduction operator.
+ !$omp parallel do reduction(+:x)
+ do i = 1, 10
+ x%val = x%val + 1
+ end do
+ !$omp end parallel do
+end program
diff --git a/flang/test/Semantics/OpenMP/declare-reduction-modfile-private.f90 b/flang/test/Semantics/OpenMP/declare-reduction-modfile-private.f90
new file mode 100644
index 0000000000000..17c704022bfd7
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-reduction-modfile-private.f90
@@ -0,0 +1,38 @@
+! RUN: %python %S/../test_modfile.py %s %flang_fc1 -fopenmp -fopenmp-version=52
+! Check that PRIVATE declare reduction accessibility is preserved in module files.
+
+!Expect: drm_private.mod
+!module drm_private
+!type::dt
+!integer(4)::val=0_4
+!endtype
+!!$OMP DECLARE REDUCTION(+:dt: omp_out%val=omp_out%val+omp_in%val) INITIALIZER(&
+!!$OMP&omp_priv=dt(0))
+!interface operator(+)
+!procedure::add_dt
+!end interface
+!private::operator(+)
+!contains
+!function add_dt(a,b)
+!type(dt),intent(in)::a
+!type(dt),intent(in)::b
+!type(dt)::add_dt
+!end
+!end
+
+module drm_private
+ type :: dt
+ integer :: val = 0
+ end type
+ private :: operator(+)
+ interface operator(+)
+ module procedure add_dt
+ end interface
+ !$omp declare reduction(+:dt:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=dt(0))
+contains
+ type(dt) function add_dt(a, b)
+ type(dt), intent(in) :: a, b
+ add_dt%val = a%val + b%val
+ end function
+end module drm_private
diff --git a/flang/test/Semantics/OpenMP/declare-reduction-public-regression.f90 b/flang/test/Semantics/OpenMP/declare-reduction-public-regression.f90
new file mode 100644
index 0000000000000..b86613a90b6ef
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-reduction-public-regression.f90
@@ -0,0 +1,38 @@
+! RUN: %flang_fc1 -fopenmp -fopenmp-version=52 -fdebug-dump-symbols %s 2>&1 | FileCheck %s
+
+! Test that PUBLIC operator accessibility still allows declare reduction
+! to be used from outside the module (regression test).
+
+module m_public_op
+ type :: dt
+ integer :: val = 0
+ end type
+ public :: operator(+)
+ interface operator(+)
+ module procedure add_dt
+ end interface
+ !$omp declare reduction(+:dt:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=dt(0))
+contains
+ type(dt) function add_dt(a, b)
+ type(dt), intent(in) :: a, b
+ add_dt%val = a%val + b%val
+ end function
+end module
+
+!CHECK: Module scope: m_public_op
+!CHECK: op.+, PUBLIC: UserReductionDetails TYPE(dt)
+
+program test_public_reduction
+ use m_public_op
+ type(dt) :: x
+ integer :: i
+ x = dt(0)
+ ! No error expected: operator(+) is PUBLIC so reduction is accessible.
+ !$omp parallel do reduction(+:x)
+ do i = 1, 10
+ x%val = x%val + 1
+ end do
+ !$omp end parallel do
+ print *, x%val
+end program
diff --git a/flang/test/Semantics/OpenMP/declare-reduction-use-assoc.f90 b/flang/test/Semantics/OpenMP/declare-reduction-use-assoc.f90
new file mode 100644
index 0000000000000..6ac17cdd79210
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-reduction-use-assoc.f90
@@ -0,0 +1,27 @@
+! RUN: %flang_fc1 -fopenmp -fopenmp-version=52 -fdebug-dump-symbols %s 2>&1 | FileCheck %s
+
+! Test that USE-associated declare reduction does not produce false
+! "Duplicate definition" errors when a new local declaration is made.
+! Related: https://github.com/llvm/llvm-project/issues/192580
+
+module m_use_reduction
+ type :: dt
+ integer :: val = 0
+ end type
+ !$omp declare reduction(+:dt:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=dt(0))
+end module
+
+module m_local_reduction
+ use m_use_reduction, only: dt
+ type :: dt2
+ real :: x = 0.0
+ end type
+ ! This should NOT produce "Duplicate definition" — the USE-associated
+ ! reduction for dt is shadowed by this new local declaration for dt2.
+ !$omp declare reduction(+:dt2:omp_out%x=omp_out%x+omp_in%x) &
+ !$omp initializer(omp_priv=dt2(0.0))
+end module
+
+!CHECK: Module scope: m_local_reduction
+!CHECK: op.+, PUBLIC: UserReductionDetails TYPE(dt2)
More information about the flang-commits
mailing list