[flang-commits] [flang] f12b167 - [flang][OpenMP] Fix ICE lowering user-defined operator declare reduction
via flang-commits
flang-commits at lists.llvm.org
Tue Jun 23 17:07:22 PDT 2026
Author: Matt
Date: 2026-06-23T19:07:16-05:00
New Revision: f12b16713185fe244b7f21f9c0fa42f96fc066e8
URL: https://github.com/llvm/llvm-project/commit/f12b16713185fe244b7f21f9c0fa42f96fc066e8
DIFF: https://github.com/llvm/llvm-project/commit/f12b16713185fe244b7f21f9c0fa42f96fc066e8.diff
LOG: [flang][OpenMP] Fix ICE lowering user-defined operator declare reduction
A REDUCTION clause naming a user-defined operator (e.g.,
reduction(.myop.:x)) crashed in lowering: ReductionProcessor assumed the
DefinedOperator clause variant always held an intrinsic operator and called
std::get<IntrinsicOperator> unconditionally, which aborts for the
DefinedOpName alternative.
Handle DefinedOpName in the reduction clause processor, adding the
clause-side counterpart to the directive handling from #190288. For a
locally declared user-defined operator reduction, resolve the operator to
its reduction symbol and reference the omp.declare_reduction op materialized
for the declare reduction directive. The op name is now module-scoped via
AbstractConverter::mangleName, on the directive and clause sides in
lockstep, so reductions with the same operator spelling in different modules
no longer collide.
Cases that are not yet supported (reductions imported by USE association,
renamed or merged operators, and declarations with multiple types) now emit
a clean "not yet implemented" diagnostic instead of crashing or silently
binding the wrong combiner. Support for the USE-associated and cross-module
cases is a follow-up that builds on the semantic fix in #200329.
Tests cover the issue's integer case and a derived-type case (both lower,
with a module-scoped op name), plus the USE-associated and multiple-type
cases (clean TODO).
Fixes #204299
Assisted-by: Claude Opus 4.8, GPT-5.5.
Added:
flang/test/Lower/OpenMP/Todo/declare-reduction-operator-multiple-types.f90
flang/test/Lower/OpenMP/Todo/declare-reduction-operator-use-assoc.f90
flang/test/Lower/OpenMP/declare-reduction-operator-derived.f90
flang/test/Lower/OpenMP/declare-reduction-operator.f90
Modified:
flang/include/flang/Lower/Support/ReductionProcessor.h
flang/lib/Lower/OpenMP/OpenMP.cpp
flang/lib/Lower/Support/ReductionProcessor.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Lower/Support/ReductionProcessor.h b/flang/include/flang/Lower/Support/ReductionProcessor.h
index 0b4a692827a79..a949da875b3a2 100644
--- a/flang/include/flang/Lower/Support/ReductionProcessor.h
+++ b/flang/include/flang/Lower/Support/ReductionProcessor.h
@@ -96,6 +96,16 @@ class ReductionProcessor {
const fir::KindMapping &kindMap,
mlir::Type ty, bool isByRef);
+ /// Returns the module-unique name of the omp.declare_reduction op that
+ /// materializes a user-defined reduction (named or operator). The name is
+ /// derived from the reduction symbol's ultimate name, qualified with its
+ /// owning scope via AbstractConverter::mangleName, so that reductions with
+ /// the same spelling in
diff erent modules do not collide. The directive and
+ /// clause lowering must both use this to agree on the op's symbol name.
+ static std::string
+ getScopedUserReductionName(AbstractConverter &converter,
+ const semantics::Symbol &reductionSymbol);
+
/// This function returns the identity value of the operator \p
/// reductionOpName. For example:
/// 0 + x = x,
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 99ce48206c33b..094cec737d481 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -4514,7 +4514,25 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
},
[&](const clause::DefinedOperator::DefinedOpName &opName)
-> std::string {
- return opName.v.sym()->name().ToString();
+ // Directive side of the user-defined operator reduction
+ // naming contract (the clause side is in
+ // ReductionProcessor::processReductionArguments).
+ // opName.v.sym() is the reduction symbol
+ // "op<spelling>". Only single-declaration, single-type
+ // reductions are supported; otherwise emit a clean
+ // TODO.
+ const semantics::Symbol &redSym =
+ opName.v.sym()->GetUltimate();
+ const auto *userDetails =
+ redSym.detailsIf<semantics::UserReductionDetails>();
+ if (!userDetails || typeNameList.v.size() != 1 ||
+ userDetails->GetDeclList().size() != 1 ||
+ userDetails->GetTypeList().size() != 1)
+ TODO(converter.getCurrentLocation(),
+ "OpenMP user-defined operator declare reduction "
+ "with multiple declarations or multiple types");
+ return ReductionProcessor::getScopedUserReductionName(
+ converter, redSym);
},
},
defOp.u);
diff --git a/flang/lib/Lower/Support/ReductionProcessor.cpp b/flang/lib/Lower/Support/ReductionProcessor.cpp
index 7db48601d5aba..d57aa48fd82b8 100644
--- a/flang/lib/Lower/Support/ReductionProcessor.cpp
+++ b/flang/lib/Lower/Support/ReductionProcessor.cpp
@@ -216,6 +216,17 @@ ReductionProcessor::getReductionName(ReductionIdentifier redId,
return getReductionName(reductionName, kindMap, ty, isByRef);
}
+std::string ReductionProcessor::getScopedUserReductionName(
+ AbstractConverter &converter, const semantics::Symbol &reductionSymbol) {
+ // Qualify the reduction symbol's ultimate name with its owning scope so that
+ // user-defined reductions with the same spelling in
diff erent modules get
+ // distinct op names. Use the (name, scope) mangleName overload: the
+ // (symbol) overload does not handle UserReductionDetails.
+ const semantics::Symbol &ultimate = reductionSymbol.GetUltimate();
+ std::string name = ultimate.name().ToString();
+ return converter.mangleName(name, ultimate.owner());
+}
+
mlir::Value
ReductionProcessor::getReductionInitValue(mlir::Location loc, mlir::Type type,
ReductionIdentifier redId,
@@ -810,6 +821,59 @@ bool ReductionProcessor::processReductionArguments(
redOperatorList.front();
if (const auto &redDefinedOp =
std::get_if<omp::clause::DefinedOperator>(&redOperator.u)) {
+ if (const auto *definedOpName =
+ std::get_if<omp::clause::DefinedOperator::DefinedOpName>(
+ &redDefinedOp->u)) {
+ // User-defined operator reduction (e.g. reduction(.myop.:x)). Resolve
+ // the use-site operator to its reduction symbol, which semantics
+ // names "op<spelling>" (MangleDefinedOperator in resolve-names), in
+ // the current scope, then reference the omp.declare_reduction op the
+ // directive materialized for it. Only a locally-declared,
+ // single-declaration, single-type reduction whose type the variable
+ // supports is handled here; anything else (imported, renamed, merged,
+ // or multiple declarations/types) is a clean TODO rather than a crash
+ // or a wrong binding.
+ const semantics::Symbol *opSym = definedOpName->v.sym();
+ std::string mangledName = "op" + opSym->name().ToString();
+ const semantics::Symbol *redSym =
+ converter.getCurrentScope().FindSymbol(
+ parser::CharBlock{mangledName});
+ const semantics::Symbol *ultimate =
+ redSym ? &redSym->GetUltimate() : nullptr;
+ const semantics::UserReductionDetails *userDetails =
+ ultimate ? ultimate->detailsIf<semantics::UserReductionDetails>()
+ : nullptr;
+ const semantics::DeclTypeSpec *varType =
+ reductionSymbols[idx]->GetUltimate().GetType();
+ if (!redSym || ultimate != redSym || !userDetails ||
+ userDetails->GetDeclList().size() != 1 ||
+ userDetails->GetTypeList().size() != 1 || !varType ||
+ !userDetails->SupportsType(*varType)) {
+ TODO(currentLocation,
+ "OpenMP user-defined operator reduction is not yet supported "
+ "for imported, renamed, or multiple-declaration/type "
+ "reductions");
+ }
+ std::string opName = ReductionProcessor::getScopedUserReductionName(
+ converter, *redSym);
+ mlir::ModuleOp module = builder.getModule();
+ auto existingDecl = module.lookupSymbol<OpType>(opName);
+ // The MLIR verifier does not type-check these ops (they have no
+ // atomic region), so this is the only guard against binding a
+ // mismatched declaration. Compare unwrapped types: the clause redType
+ // is always a reference type, while the op stores the unwrapped type
+ // for by-value reductions.
+ if (!existingDecl || fir::unwrapRefType(existingDecl.getType()) !=
+ fir::unwrapRefType(redType)) {
+ TODO(currentLocation,
+ "OpenMP user-defined operator reduction declaration was not "
+ "materialized for this type");
+ }
+ reductionDeclSymbols.push_back(mlir::SymbolRefAttr::get(
+ builder.getContext(), existingDecl.getSymName()));
+ ++idx;
+ continue;
+ }
const auto &intrinsicOp{
std::get<omp::clause::DefinedOperator::IntrinsicOperator>(
redDefinedOp->u)};
diff --git a/flang/test/Lower/OpenMP/Todo/declare-reduction-operator-multiple-types.f90 b/flang/test/Lower/OpenMP/Todo/declare-reduction-operator-multiple-types.f90
new file mode 100644
index 0000000000000..626316221af1b
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/declare-reduction-operator-multiple-types.f90
@@ -0,0 +1,38 @@
+! A user-defined operator declare reduction listing multiple types reaches
+! lowering but is not yet supported (the op name is not type-specific, so the
+! per-type ops would collide). It must emit a clean TODO rather than ICE or
+! miscompile (#204299). The guard is on the directive side, which is lowered
+! before any reduction clause.
+
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: OpenMP user-defined operator declare reduction with multiple declarations or multiple types
+
+program p
+ type :: t1
+ integer :: v = 0
+ end type
+ type :: t2
+ integer :: w = 0
+ end type
+ interface operator(.mt.)
+ function f1(a, b)
+ import :: t1
+ type(t1), intent(in) :: a, b
+ type(t1) :: f1
+ end function f1
+ function f2(a, b)
+ import :: t2
+ type(t2), intent(in) :: a, b
+ type(t2) :: f2
+ end function f2
+ end interface
+ !$omp declare reduction(.mt.:t1,t2:omp_out=omp_in)
+ type(t1) :: x1
+ integer :: i
+ !$omp parallel do reduction(.mt.:x1)
+ do i = 1, 5
+ x1 = x1 .mt. t1(1)
+ end do
+ !$omp end parallel do
+end program p
diff --git a/flang/test/Lower/OpenMP/Todo/declare-reduction-operator-use-assoc.f90 b/flang/test/Lower/OpenMP/Todo/declare-reduction-operator-use-assoc.f90
new file mode 100644
index 0000000000000..68be5781ce5cf
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/declare-reduction-operator-use-assoc.f90
@@ -0,0 +1,36 @@
+! A user-defined operator reduction whose declaration is USE-associated from a
+! module (plain USE, so the "op<spelling>" reduction symbol is imported) reaches
+! lowering but is not yet supported: lowering does not materialize imported
+! declare reductions. It must emit a clean TODO rather than ICE (#204299).
+
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: OpenMP user-defined operator reduction is not yet supported for imported, renamed, or multiple-declaration/type reductions
+
+module m_use_op
+ type :: t
+ integer :: val = 0
+ end type
+ interface operator(.plus.)
+ module procedure add_t
+ end interface
+ !$omp declare reduction(.plus.:t:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=t(0))
+contains
+ type(t) function add_t(a, b)
+ type(t), intent(in) :: a, b
+ add_t%val = a%val + b%val
+ end function add_t
+end module m_use_op
+
+program p
+ use m_use_op
+ type(t) :: x
+ integer :: i
+ x = t(0)
+ !$omp parallel do reduction(.plus.:x)
+ do i = 1, 100
+ x = x .plus. t(1)
+ end do
+ !$omp end parallel do
+end program p
diff --git a/flang/test/Lower/OpenMP/declare-reduction-operator-derived.f90 b/flang/test/Lower/OpenMP/declare-reduction-operator-derived.f90
new file mode 100644
index 0000000000000..62b30cb5d4542
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-reduction-operator-derived.f90
@@ -0,0 +1,36 @@
+! Test lowering of an OpenMP REDUCTION clause that uses a locally-declared
+! user-defined operator on a derived type (#204299). This exercises the by-ref
+! reduction path and the unwrapped-type guard: the clause variable type is a
+! reference type while the op stores the unwrapped reduction type.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+subroutine red_derived
+ type :: t
+ integer :: val = 0
+ end type
+ interface operator(.myop.)
+ function add_t(a, b)
+ import :: t
+ type(t), intent(in) :: a, b
+ type(t) :: add_t
+ end function add_t
+ end interface
+ !$omp declare reduction(.myop.:t:omp_out%val=omp_out%val+omp_in%val) &
+ !$omp initializer(omp_priv=t(0))
+ type(t) :: x
+ integer :: i
+ x = t(0)
+ !$omp parallel do reduction(.myop.:x)
+ do i = 1, 100
+ x = x .myop. t(1)
+ end do
+ !$omp end parallel do
+end subroutine red_derived
+
+! The op name must be module-scoped (a mangled "_QQ..." generated name), NOT the
+! bare operator spelling. The directive and clause must reference the same name.
+! CHECK: omp.declare_reduction @[[RED:_QQ[A-Za-z0-9_.]*op\.myop\.]] : !fir.ref
+! CHECK-NOT: omp.declare_reduction @op.myop.
+! CHECK: omp.wsloop
+! CHECK-SAME: reduction(byref @[[RED]]
diff --git a/flang/test/Lower/OpenMP/declare-reduction-operator.f90 b/flang/test/Lower/OpenMP/declare-reduction-operator.f90
new file mode 100644
index 0000000000000..11897212078eb
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-reduction-operator.f90
@@ -0,0 +1,31 @@
+! Test lowering of an OpenMP REDUCTION clause that uses a locally-declared
+! user-defined operator. See https://github.com/llvm/llvm-project/issues/204299:
+! this used to ICE in ReductionProcessor because the defined-operator variant
+! was assumed to be an intrinsic operator. This is the trivial (by-value)
+! integer case from the issue reproducer.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+subroutine red_integer
+ interface operator(.zzzz.)
+ function zzzz_op(a, b)
+ integer, intent(in) :: a, b
+ integer :: zzzz_op
+ end function zzzz_op
+ end interface
+ !$omp declare reduction(.zzzz.:integer:omp_out=omp_out+omp_in) &
+ !$omp initializer(omp_priv=0)
+ integer :: x
+ x = 0
+ !$omp parallel reduction(.zzzz.:x)
+ x = x .zzzz. 1
+ !$omp end parallel
+end subroutine red_integer
+
+! The op name must be module-scoped (a mangled "_QQ..." generated name), NOT the
+! bare operator spelling, so reductions with the same spelling in
diff erent
+! modules do not collide. The directive and clause must reference the same name.
+! CHECK: omp.declare_reduction @[[RED:_QQ[A-Za-z0-9_.]*op\.zzzz\.]] : i32
+! CHECK-NOT: omp.declare_reduction @op.zzzz.
+! CHECK: omp.parallel
+! CHECK-SAME: reduction(@[[RED]]
More information about the flang-commits
mailing list