[flang-commits] [flang] [flang][OpenMP] Support user-defined declare reduction with derived types (PR #184897)
via flang-commits
flang-commits at lists.llvm.org
Tue Mar 17 18:48:07 PDT 2026
https://github.com/MattPD updated https://github.com/llvm/llvm-project/pull/184897
>From 60209b488b62ef911bd51ac7761d2816fc006907 Mon Sep 17 00:00:00 2001
From: "Matt P. Dziubinski" <matt-p.dziubinski at hpe.com>
Date: Thu, 5 Mar 2026 15:56:29 -0600
Subject: [PATCH 1/6] [flang][OpenMP] Support user-defined declare reduction
with derived types
Fix lowering of `!$omp declare reduction` for intrinsic operators applied
to user-defined derived types (e.g., `+` on `type(t)`). Previously, this
hit a TODO in `ReductionProcessor::getReductionInitValue` because the code
tried to compute an init value for a non-predefined type, when it should
instead use the initializer region from the `DeclareReductionOp`.
The root cause was a naming mismatch: `genOMP` for
`OpenMPDeclareReductionConstruct` used a raw operator string (e.g., "Add")
as the reduction name, while `processReductionArguments` at the use site
computed a canonical name via `getReductionName` (e.g.,
"add_reduction_byref_rec__QFTt"). The `lookupSymbol` in
`createDeclareReductionHelper` never found the already-created op, so it
fell through to `createDeclareReduction` which called `getReductionInitValue`
with the derived type and hit the TODO.
The fix has three parts:
1. Consistent names: In `genOMP` for `OpenMPDeclareReductionConstruct`, compute
the reduction name using the same `getReductionName` scheme that
`processReductionArguments` uses, so both sites produce identical symbol names.
For intrinsic operators, this maps through `ReductionIdentifier` to get the
canonical name. For user-defined named reductions, the raw symbol name is used
directly, matching the existing custom-reduction lookup path.
2. Reuse reduction: In `processReductionArguments`, when an intrinsic operator
reduction is requested, check whether a user-defined declare reduction already
exists under that canonical name before attempting to create a new one. If
found, reuse it. This avoids calling `createDeclareReduction` (and thus
`getReductionInitValue`) for types that have user-provided initializers.
3. Reference semantics: Change `doReductionByRef` to return true for derived
types. Previously it returned false for both trivial and derived types, treating
derived types as by-val. This is incorrect for user-defined combiners that
operate on components via side-effects (e.g., `omp_out%x = omp_out%x +
omp_in%x`): the combiner mutates `omp_out` in place and doesn't produce a
whole-struct value, so `convertExprToValue` returns the component type
(`i32`) rather than the struct type, causing a type mismatch in the
`omp.yield`. By-ref is the correct model: the combiner stores into the
lhs reference and yields it.
The combiner callback in `processReductionCombiner` is also updated to
handle the by-ref derived-type case: when the combiner result type
doesn't match the element type (as happens with component-level
assignments), the store is skipped since the assignment already wrote
into omp_out as a side-effect, and only the lhs reference is yielded.
Tests updates:
- Update declare-reduction-intrinsic-op.f90 from a negative test (checking
for the TODO error) to a positive test checking the generated MLIR.
- Update omp-declare-reduction-derivedtype.f90 CHECK lines to match the
reference semantics fix: the `declare_reduction` now has type `!fir.ref<...>`
with a `byref_element_type` attribute, an alloc region, a two-argument init
region, and a combiner that stores into the lhs and yields the
reference. The function body checks for initme and mycombine are
unchanged in substance but use literal type names instead of a regex
capture to avoid greedy matching issues with nested angle brackets.
Remaining work: declare reduction without an initializer clause is not yet
supported. I plan to address that subsequently.
Assisted-by: Claude Opus 4.6.
---
flang/include/flang/Lower/OpenMP/Clauses.h | 11 +++
flang/lib/Lower/OpenMP/OpenMP.cpp | 73 ++++++++++++-------
.../lib/Lower/Support/ReductionProcessor.cpp | 18 ++++-
.../OpenMP/declare-reduction-intrinsic-op.f90 | 23 +++++-
.../omp-declare-reduction-derivedtype.f90 | 41 +++++------
5 files changed, 113 insertions(+), 53 deletions(-)
diff --git a/flang/include/flang/Lower/OpenMP/Clauses.h b/flang/include/flang/Lower/OpenMP/Clauses.h
index a325e74327240..f334374280c73 100644
--- a/flang/include/flang/Lower/OpenMP/Clauses.h
+++ b/flang/include/flang/Lower/OpenMP/Clauses.h
@@ -329,6 +329,17 @@ using UsesAllocators = tomp::clause::UsesAllocatorsT<TypeTy, IdTy, ExprTy>;
using Weak = tomp::clause::WeakT<TypeTy, IdTy, ExprTy>;
using When = tomp::clause::WhenT<TypeTy, IdTy, ExprTy>;
using Write = tomp::clause::WriteT<TypeTy, IdTy, ExprTy>;
+
+DefinedOperator makeDefinedOperator(const parser::DefinedOperator &inp,
+ semantics::SemanticsContext &semaCtx);
+
+ProcedureDesignator
+makeProcedureDesignator(const parser::ProcedureDesignator &inp,
+ semantics::SemanticsContext &semaCtx);
+
+ReductionOperator
+makeReductionOperator(const parser::OmpReductionIdentifier &inp,
+ semantics::SemanticsContext &semaCtx);
} // namespace clause
using tomp::type::operator==;
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index e2018add11206..818379d5a47b3 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3864,7 +3864,13 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
evalExpr.u);
stmtCtx.finalizeAndPop();
if (isByRef) {
- fir::StoreOp::create(builder, loc, result, lhs);
+ // For user-defined combiners the assignment expression (e.g.
+ // "omp_out%x = omp_out%x + omp_in%x") already wrote into omp_out
+ // as a side-effect. We only need to yield the lhs reference.
+ // Only store result back if its type actually matches the element type.
+ mlir::Type eleTy = fir::unwrapRefType(lhs.getType());
+ if (result.getType() == eleTy)
+ fir::StoreOp::create(builder, loc, result, lhs);
mlir::omp::YieldOp::create(builder, loc, lhs);
} else {
mlir::omp::YieldOp::create(builder, loc, result);
@@ -3957,41 +3963,56 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
const auto &identifier =
std::get<parser::OmpReductionIdentifier>(specifier.t);
- std::string reductionNameStr = Fortran::common::visit(
- common::visitors{
- [](const parser::ProcedureDesignator &pd) -> std::string {
- return std::get<parser::Name>(pd.u).ToString();
- },
- [](const parser::DefinedOperator &defOp) -> std::string {
- return Fortran::common::visit(
- common::visitors{
- [](const parser::DefinedOpName &opName) -> std::string {
- return opName.v.ToString();
- },
- [](parser::DefinedOperator::IntrinsicOperator intrOp)
- -> std::string {
- return std::string(
- parser::DefinedOperator::EnumToString(intrOp));
- },
- },
- defOp.u);
- },
- },
- identifier.u);
+ // Convert the parser-level reduction identifier to the clause-level
+ // representation, then use ReductionProcessor to derive the canonical name.
+ clause::ReductionOperator redOp =
+ clause::makeReductionOperator(identifier, semaCtx);
for (const auto &typeSpec : typeNameList.v) {
(void)typeSpec; // Currently unused
mlir::Type reductionType = getReductionType(converter, specifier);
+ bool isByRef = ReductionProcessor::doReductionByRef(reductionType);
+ // Compute the canonical reduction name the same way
+ // processReductionArguments does.
+ std::string reductionNameStr = Fortran::common::visit(
+ common::visitors{
+ [&](const clause::DefinedOperator &defOp) -> std::string {
+ return Fortran::common::visit(
+ common::visitors{
+ [&](const clause::DefinedOperator::IntrinsicOperator
+ &intrOp) -> std::string {
+ ReductionProcessor::ReductionIdentifier redId =
+ ReductionProcessor::getReductionType(intrOp);
+ return ReductionProcessor::getReductionName(
+ redId, converter.getFirOpBuilder().getKindMap(),
+ reductionType, isByRef);
+ },
+ [&](const clause::DefinedOperator::DefinedOpName &opName)
+ -> std::string {
+ return opName.v.sym()->name().ToString();
+ },
+ },
+ defOp.u);
+ },
+ [&](const clause::ProcedureDesignator &pd) -> std::string {
+ return pd.v.sym()->name().ToString();
+ },
+ },
+ redOp.u);
+
ReductionProcessor::GenCombinerCBTy genCombinerCB =
processReductionCombiner(converter, symTable, semaCtx, combiner);
ReductionProcessor::GenInitValueCBTy genInitValueCB;
ClauseProcessor cp(converter, semaCtx, clauses);
cp.processInitializer(symTable, genInitValueCB);
- bool isByRef = ReductionProcessor::doReductionByRef(reductionType);
+ mlir::Type redType =
+ isByRef
+ ? static_cast<mlir::Type>(fir::ReferenceType::get(reductionType))
+ : reductionType;
ReductionProcessor::createDeclareReductionHelper<
- mlir::omp::DeclareReductionOp>(
- converter, reductionNameStr, reductionType,
- converter.getCurrentLocation(), isByRef, genCombinerCB, genInitValueCB);
+ mlir::omp::DeclareReductionOp>(converter, reductionNameStr, redType,
+ converter.getCurrentLocation(), isByRef,
+ genCombinerCB, genInitValueCB);
}
}
diff --git a/flang/lib/Lower/Support/ReductionProcessor.cpp b/flang/lib/Lower/Support/ReductionProcessor.cpp
index e0cba4c512258..eaaf643ec7eeb 100644
--- a/flang/lib/Lower/Support/ReductionProcessor.cpp
+++ b/flang/lib/Lower/Support/ReductionProcessor.cpp
@@ -642,11 +642,11 @@ OpType ReductionProcessor::createDeclareReduction(
bool ReductionProcessor::doReductionByRef(mlir::Type reductionType) {
if (forceByrefReduction)
return true;
-
- if (!fir::isa_trivial(fir::unwrapRefType(reductionType)) &&
- !fir::isa_derived(fir::unwrapRefType(reductionType)))
+ // Non-trivial, non-derived types (e.g., boxes, arrays) must be by-ref.
+ // Derived types must also be by-ref because user-defined combiners
+ // operate on components via side-effects, not by producing a whole value.
+ if (!fir::isa_trivial(fir::unwrapRefType(reductionType)))
return true;
-
return false;
}
@@ -798,6 +798,16 @@ bool ReductionProcessor::processReductionArguments(
}
reductionName = getReductionName(redId, kindMap, redType, isByRef);
+ // If a user-defined declare reduction already exists for this
+ // operator+type, reuse it instead of generating a new one
+ // (which would fail for non-predefined types like derived types).
+ mlir::ModuleOp module = builder.getModule();
+ if (auto existingDecl = module.lookupSymbol<OpType>(reductionName)) {
+ reductionDeclSymbols.push_back(mlir::SymbolRefAttr::get(
+ builder.getContext(), existingDecl.getSymName()));
+ ++idx;
+ continue;
+ }
} else if (const auto *reductionIntrinsic =
std::get_if<omp::clause::ProcedureDesignator>(
&redOperator.u)) {
diff --git a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90 b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
index 8b5051b63afd4..a2ec52902bba2 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
@@ -1,10 +1,9 @@
-! RUN: not %flang_fc1 -emit-mlir -fopenmp %s -o - 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
program test
type t
integer :: x
end type t
- ! CHECK: not yet implemented: Reduction of some types is not supported
!$omp declare reduction(+:t: omp_out%x = omp_out%x + omp_in%x) initializer(omp_priv = t(0))
type(t) :: a
a = t(0)
@@ -12,3 +11,23 @@ program test
a%x = a%x + 1
!$omp end parallel
end program test
+
+! CHECK: omp.declare_reduction @add_reduction_byref_rec__QFTt :
+! CHECK: %[[ALLOCA:.*]] = fir.alloca [[TY:.*]]
+! CHECK: omp.yield(%[[ALLOCA]] : !fir.ref<[[TY]]>)
+! CHECK: } init {
+! CHECK: ^bb0(%[[INIT_ARG0:.*]]: !fir.ref<[[TY]]>, %[[INIT_ARG1:.*]]: !fir.ref<[[TY]]>):
+! CHECK: %{{.*}} = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_orig"}
+! CHECK: %{{.*}} = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_priv"}
+! CHECK: omp.yield(%[[INIT_ARG1]] : !fir.ref<[[TY]]>)
+! CHECK: } combiner {
+! CHECK: ^bb0(%[[ARG0:.*]]: !fir.ref<[[TY]]>, %[[ARG1:.*]]: !fir.ref<[[TY]]>):
+! CHECK: %[[OMP_IN:.*]]:2 = hlfir.declare %[[ARG1]] {uniq_name = "omp_in"}
+! CHECK: %[[OMP_OUT:.*]]:2 = hlfir.declare %[[ARG0]] {uniq_name = "omp_out"}
+! CHECK: %[[OUT_X:.*]] = hlfir.designate %[[OMP_OUT]]#0{"x"} : (!fir.ref<[[TY]]>) -> !fir.ref<i32>
+! CHECK: %[[OUT_X_VAL:.*]] = fir.load %[[OUT_X]] : !fir.ref<i32>
+! CHECK: %[[IN_X:.*]] = hlfir.designate %[[OMP_IN]]#0{"x"} : (!fir.ref<[[TY]]>) -> !fir.ref<i32>
+! CHECK: %[[IN_X_VAL:.*]] = fir.load %[[IN_X]] : !fir.ref<i32>
+! CHECK: %{{.*}} = arith.addi %[[OUT_X_VAL]], %[[IN_X_VAL]] : i32
+! CHECK: omp.yield(%[[ARG0]] : !fir.ref<[[TY]]>)
+! CHECK: }
\ No newline at end of file
diff --git a/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90 b/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
index ff70acbb10e32..4ca735d6105f1 100644
--- a/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
+++ b/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
@@ -1,5 +1,5 @@
! This test checks lowering of OpenMP declare reduction Directive, with initialization
-! via a subroutine. This functionality is currently not implemented.
+! via a subroutine.
!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s
module maxtype_mod
@@ -41,35 +41,34 @@ function func(x, n, init)
end function func
end module maxtype_mod
-!CHECK: omp.declare_reduction @red_add_max : [[MAXTYPE:.*]] init {
-!CHECK: ^bb0(%[[OMP_ORIG_ARG_I:.*]]: [[MAXTYPE]]):
-!CHECK: %[[OMP_PRIV:.*]] = fir.alloca [[MAXTYPE]]
-!CHECK: %[[OMP_ORIG:.*]] = fir.alloca [[MAXTYPE]]
-!CHECK: fir.store %[[OMP_ORIG_ARG_I]] to %[[OMP_ORIG]] : !fir.ref<[[MAXTYPE]]>
-!CHECK: %[[OMP_ORIG_DECL:.*]]:2 = hlfir.declare %[[OMP_ORIG]] {uniq_name = "omp_orig"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
-!CHECK: fir.store %[[OMP_ORIG_ARG_I]] to %[[OMP_PRIV]] : !fir.ref<[[MAXTYPE]]>
-!CHECK: %[[OMP_PRIV_DECL:.*]]:2 = hlfir.declare %[[OMP_PRIV]] {uniq_name = "omp_priv"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
-!CHECK: fir.call @_QMmaxtype_modPinitme(%[[OMP_PRIV_DECL]]#0, %[[OMP_ORIG_DECL]]#0) fastmath<contract> : (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>) -> ()
-!CHECK: %[[OMP_PRIV_VAL:.*]] = fir.load %[[OMP_PRIV_DECL]]#0 : !fir.ref<[[MAXTYPE]]>
-!CHECK: omp.yield(%[[OMP_PRIV_VAL]] : [[MAXTYPE]])
+!CHECK: omp.declare_reduction @red_add_max : !fir.ref<{{.*}}> attributes {byref_element_type = {{.*}}} alloc {
+!CHECK: %[[ALLOCA:.*]] = fir.alloca [[MAXTYPE:.*]]
+!CHECK: omp.yield(%[[ALLOCA]] : !fir.ref<[[MAXTYPE]]>)
+!CHECK: } init {
+!CHECK: ^bb0(%[[INIT_ARG0:.*]]: !fir.ref<[[MAXTYPE]]>, %[[INIT_ARG1:.*]]: !fir.ref<[[MAXTYPE]]>):
+!CHECK: %{{.*}} = fir.embox %[[INIT_ARG1]]
+!CHECK: %{{.*}} = fir.embox %[[INIT_ARG0]]
+!CHECK: %[[OMP_ORIG:.*]]:2 = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_orig"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
+!CHECK: %[[OMP_PRIV:.*]]:2 = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_priv"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
+!CHECK: fir.call @_QMmaxtype_modPinitme(%[[OMP_PRIV]]#0, %[[OMP_ORIG]]#0) fastmath<contract> : (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>) -> ()
+!CHECK: %{{.*}} = fir.load %[[OMP_PRIV]]#0 : !fir.ref<[[MAXTYPE]]>
+!CHECK: omp.yield(%[[INIT_ARG1]] : !fir.ref<[[MAXTYPE]]>)
!CHECK: } combiner {
-!CHECK: ^bb0(%[[LHS_ARG:.*]]: [[MAXTYPE]], %[[RHS_ARG:.*]]: [[MAXTYPE]]):
+!CHECK: ^bb0(%[[LHS_ARG:.*]]: !fir.ref<[[MAXTYPE]]>, %[[RHS_ARG:.*]]: !fir.ref<[[MAXTYPE]]>):
!CHECK: %[[RESULT:.*]] = fir.alloca [[MAXTYPE]] {bindc_name = ".result"}
-!CHECK: %[[OMP_OUT:.*]] = fir.alloca [[MAXTYPE]]
-!CHECK: %[[OMP_IN:.*]] = fir.alloca [[MAXTYPE]]
-!CHECK: fir.store %[[RHS_ARG]] to %[[OMP_IN]] : !fir.ref<[[MAXTYPE]]>
-!CHECK: %[[OMP_IN_DECL:.*]]:2 = hlfir.declare %[[OMP_IN]] {uniq_name = "omp_in"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
-!CHECK: fir.store %[[LHS_ARG]] to %[[OMP_OUT]] : !fir.ref<[[MAXTYPE]]>
-!CHECK: %[[OMP_OUT_DECL:.*]]:2 = hlfir.declare %[[OMP_OUT]] {uniq_name = "omp_out"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
+!CHECK: %[[OMP_IN:.*]]:2 = hlfir.declare %[[RHS_ARG]] {uniq_name = "omp_in"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
+!CHECK: %[[OMP_OUT:.*]]:2 = hlfir.declare %[[LHS_ARG]] {uniq_name = "omp_out"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
!CHECK: %[[TMPRESULT:.*]]:2 = hlfir.declare %[[RESULT]] {uniq_name = ".tmp.func_result"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
-!CHECK: %[[COMBINE_RESULT:.*]] = fir.call @_QMmaxtype_modPmycombine(%[[OMP_OUT_DECL]]#0, %[[OMP_IN_DECL]]#0) fastmath<contract> : (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>) -> [[MAXTYPE]]
+!CHECK: %[[COMBINE_RESULT:.*]] = fir.call @_QMmaxtype_modPmycombine(%[[OMP_OUT]]#0, %[[OMP_IN]]#0) fastmath<contract> : (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>) -> [[MAXTYPE]]
!CHECK: fir.save_result %[[COMBINE_RESULT]] to %[[TMPRESULT]]#0 : [[MAXTYPE]], !fir.ref<[[MAXTYPE]]>
!CHECK: %false = arith.constant false
!CHECK: %[[EXPRRESULT:.*]] = hlfir.as_expr %[[TMPRESULT]]#0 move %false : (!fir.ref<[[MAXTYPE]]>, i1) -> !hlfir.expr<[[MAXTYPE]]>
!CHECK: %[[ASSOCIATE:.*]]:3 = hlfir.associate %[[EXPRRESULT]] {adapt.valuebyref} : (!hlfir.expr<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>, i1)
!CHECK: %[[RESULT_VAL:.*]] = fir.load %[[ASSOCIATE]]#0 : !fir.ref<[[MAXTYPE]]>
!CHECK: hlfir.end_associate %[[ASSOCIATE]]#1, %[[ASSOCIATE]]#2 : !fir.ref<[[MAXTYPE]]>, i1
-!CHECK: omp.yield(%[[RESULT_VAL]] : [[MAXTYPE]])
+!CHECK: hlfir.destroy %[[EXPRRESULT]] : !hlfir.expr<[[MAXTYPE]]>
+!CHECK: fir.store %[[RESULT_VAL]] to %[[LHS_ARG]] : !fir.ref<[[MAXTYPE]]>
+!CHECK: omp.yield(%[[LHS_ARG]] : !fir.ref<[[MAXTYPE]]>)
!CHECK: }
!CHECK: func.func @_QMmaxtype_modPinitme(%[[X_ARG:.*]]: !fir.ref<[[MAXTYPE]]> {fir.bindc_name = "x"}, %[[N_ARG:.*]]: !fir.ref<[[MAXTYPE]]> {fir.bindc_name = "n"}) {
>From 420922117891f9c301b42fb1dc1aee686f1f840f Mon Sep 17 00:00:00 2001
From: "Matt P. Dziubinski" <matt-p.dziubinski at hpe.com>
Date: Fri, 13 Mar 2026 15:20:14 -0500
Subject: [PATCH 2/6] WIP: whole-variable assignment
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 30 ++++++++++---
flang/lib/Lower/OpenMP/OpenMP.cpp | 43 ++++++++++++++-----
.../OpenMP/declare-reduction-intrinsic-op.f90 | 11 +++--
.../omp-declare-reduction-derivedtype.f90 | 10 ++---
4 files changed, 69 insertions(+), 25 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 85493bf45453e..4fb04929052b1 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -396,6 +396,7 @@ bool ClauseProcessor::processInitializer(
for (const Object &object :
std::get<StylizedInstance::Variables>(inst.t)) {
mlir::Value addr;
+ std::string name = object.sym()->name().ToString();
mlir::Type ompOrigType = ompOrig.getType();
// Check for unsupported dynamic-length character reductions
mlir::Type unwrappedType = fir::unwrapRefType(ompOrigType);
@@ -408,8 +409,16 @@ bool ClauseProcessor::processInitializer(
"OpenMP reduction allocation for dynamic length character");
}
}
- // If ompOrig is already a reference, we can use it directly
- if (fir::isa_ref_type(ompOrigType)) {
+ // For by-ref reductions, the init block has two arguments:
+ // arg0 = mold/original, arg1 = private allocation.
+ // omp_priv must map to arg1 (the private copy), not arg0.
+ if (name == "omp_priv" && fir::isa_ref_type(ompOrigType)) {
+ mlir::Block *initBlock = builder.getInsertionBlock();
+ if (initBlock->getNumArguments() > 1)
+ addr = initBlock->getArgument(1);
+ else
+ addr = ompOrig;
+ } else if (fir::isa_ref_type(ompOrigType)) {
addr = ompOrig;
} else {
addr = builder.createTemporary(loc, ompOrigType);
@@ -419,7 +428,6 @@ bool ClauseProcessor::processInitializer(
fir::FortranVariableFlagsAttr attributes =
Fortran::lower::translateSymbolAttributes(
builder.getContext(), *object.sym(), extraFlags);
- std::string name = object.sym()->name().ToString();
// Get length parameters for types that need them (e.g., characters).
// Note: DeclareOp requires exactly one type parameter for non-boxed
// characters, unlike EmboxOp which doesn't allow them for constant-len.
@@ -451,13 +459,23 @@ bool ClauseProcessor::processInitializer(
[&](const auto &expr) -> mlir::Value {
mlir::Value exprResult = fir::getBase(convertExprToValue(
loc, converter, initExpr, symMap, stmtCtx));
- // Conversion can either give a value or a refrence to a value,
- // we need to return the reduction type, so an optional load may
- // be generated.
if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
exprResult.getType()))
if (ompPrivVar.getType() == refType)
exprResult = fir::LoadOp::create(builder, loc, exprResult);
+
+ // For derived types in by-ref reductions, the init value
+ // (e.g. t(0)) must be stored into omp_priv explicitly.
+ // populateByRefInitAndCleanupRegions doesn't handle
+ // scalarInitValue for unboxed derived types, so we store
+ // here and return null to prevent a redundant store attempt.
+ if (ompPrivVar &&
+ fir::isa_ref_type(ompPrivVar.getType()) &&
+ fir::isa_derived(
+ fir::unwrapRefType(ompPrivVar.getType()))) {
+ fir::StoreOp::create(builder, loc, exprResult, ompPrivVar);
+ return mlir::Value{};
+ }
return exprResult;
}},
initExpr.u);
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 818379d5a47b3..042fb37743327 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3839,6 +3839,10 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
}
lower::StatementContext stmtCtx;
+ mlir::Type eleTy = isByRef ? fir::unwrapRefType(lhs.getType())
+ : lhs.getType();
+ bool isDerived = fir::isa_derived(eleTy);
+
mlir::Value result = common::visit(
common::visitors{
[&](const evaluate::ProcedureRef &procRef) -> mlir::Value {
@@ -3848,29 +3852,48 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
return outVal;
},
[&](const auto &expr) -> mlir::Value {
+ if (isDerived && isByRef) {
+ mlir::Value exprResult = fir::getBase(convertExprToValue(
+ loc, converter, evalExpr, symTable, stmtCtx));
+ // Check if this is a component-level expression (has a
+ // designator on omp_out) or a whole-struct expression
+ // (function call returning a struct).
+ mlir::Value designator;
+ for (auto *user : ompOutVar.getUsers()) {
+ if (auto desig = mlir::dyn_cast<hlfir::DesignateOp>(user))
+ designator = desig.getResult();
+ }
+ if (designator) {
+ // Component-level: store into the designator.
+ fir::StoreOp::create(builder, loc, exprResult, designator);
+ return mlir::Value{};
+ }
+ // Whole-struct function result: return the value so the
+ // post-visit code can store it into lhs.
+ if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
+ exprResult.getType()))
+ exprResult = fir::LoadOp::create(builder, loc, exprResult);
+ return exprResult;
+ }
mlir::Value exprResult = fir::getBase(convertExprToValue(
loc, converter, evalExpr, symTable, stmtCtx));
- // Optional load may be generated if we get a reference to the
- // reduction type.
if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
exprResult.getType())) {
mlir::Type expectedType =
- isByRef ? fir::unwrapRefType(lhs.getType()) : lhs.getType();
+ isByRef ? fir::unwrapRefType(lhs.getType())
+ : lhs.getType();
if (expectedType == refType.getElementType())
- exprResult = fir::LoadOp::create(builder, loc, exprResult);
+ exprResult =
+ fir::LoadOp::create(builder, loc, exprResult);
}
return exprResult;
}},
evalExpr.u);
stmtCtx.finalizeAndPop();
if (isByRef) {
- // For user-defined combiners the assignment expression (e.g.
- // "omp_out%x = omp_out%x + omp_in%x") already wrote into omp_out
- // as a side-effect. We only need to yield the lhs reference.
- // Only store result back if its type actually matches the element type.
- mlir::Type eleTy = fir::unwrapRefType(lhs.getType());
- if (result.getType() == eleTy)
+ if (result) {
fir::StoreOp::create(builder, loc, result, lhs);
+ }
mlir::omp::YieldOp::create(builder, loc, lhs);
} else {
mlir::omp::YieldOp::create(builder, loc, result);
diff --git a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90 b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
index a2ec52902bba2..1ce41346d5468 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
@@ -17,8 +17,10 @@ end program test
! CHECK: omp.yield(%[[ALLOCA]] : !fir.ref<[[TY]]>)
! CHECK: } init {
! CHECK: ^bb0(%[[INIT_ARG0:.*]]: !fir.ref<[[TY]]>, %[[INIT_ARG1:.*]]: !fir.ref<[[TY]]>):
-! CHECK: %{{.*}} = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_orig"}
-! CHECK: %{{.*}} = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_priv"}
+! CHECK: %{{.*}} = fir.embox %[[INIT_ARG1]]
+! CHECK: %{{.*}} = fir.embox %[[INIT_ARG0]]
+! CHECK: %{{.*}}:2 = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_orig"}
+! CHECK: %{{.*}}:2 = hlfir.declare %[[INIT_ARG1]] {uniq_name = "omp_priv"}
! CHECK: omp.yield(%[[INIT_ARG1]] : !fir.ref<[[TY]]>)
! CHECK: } combiner {
! CHECK: ^bb0(%[[ARG0:.*]]: !fir.ref<[[TY]]>, %[[ARG1:.*]]: !fir.ref<[[TY]]>):
@@ -28,6 +30,7 @@ end program test
! CHECK: %[[OUT_X_VAL:.*]] = fir.load %[[OUT_X]] : !fir.ref<i32>
! CHECK: %[[IN_X:.*]] = hlfir.designate %[[OMP_IN]]#0{"x"} : (!fir.ref<[[TY]]>) -> !fir.ref<i32>
! CHECK: %[[IN_X_VAL:.*]] = fir.load %[[IN_X]] : !fir.ref<i32>
-! CHECK: %{{.*}} = arith.addi %[[OUT_X_VAL]], %[[IN_X_VAL]] : i32
+! CHECK: %[[ADD:.*]] = arith.addi %[[OUT_X_VAL]], %[[IN_X_VAL]] : i32
+! CHECK: fir.store %[[ADD]] to %[[OUT_X]] : !fir.ref<i32>
! CHECK: omp.yield(%[[ARG0]] : !fir.ref<[[TY]]>)
-! CHECK: }
\ No newline at end of file
+! CHECK: }
diff --git a/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90 b/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
index 4ca735d6105f1..0ab32bf5a9be6 100644
--- a/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
+++ b/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
@@ -41,17 +41,17 @@ function func(x, n, init)
end function func
end module maxtype_mod
-!CHECK: omp.declare_reduction @red_add_max : !fir.ref<{{.*}}> attributes {byref_element_type = {{.*}}} alloc {
+!CHECK: omp.declare_reduction @red_add_max : !fir.ref<[[MAXTYPE:.*]]> {{.*}} alloc {
!CHECK: %[[ALLOCA:.*]] = fir.alloca [[MAXTYPE:.*]]
!CHECK: omp.yield(%[[ALLOCA]] : !fir.ref<[[MAXTYPE]]>)
!CHECK: } init {
!CHECK: ^bb0(%[[INIT_ARG0:.*]]: !fir.ref<[[MAXTYPE]]>, %[[INIT_ARG1:.*]]: !fir.ref<[[MAXTYPE]]>):
!CHECK: %{{.*}} = fir.embox %[[INIT_ARG1]]
!CHECK: %{{.*}} = fir.embox %[[INIT_ARG0]]
-!CHECK: %[[OMP_ORIG:.*]]:2 = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_orig"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
-!CHECK: %[[OMP_PRIV:.*]]:2 = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_priv"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
-!CHECK: fir.call @_QMmaxtype_modPinitme(%[[OMP_PRIV]]#0, %[[OMP_ORIG]]#0) fastmath<contract> : (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>) -> ()
-!CHECK: %{{.*}} = fir.load %[[OMP_PRIV]]#0 : !fir.ref<[[MAXTYPE]]>
+!CHECK: %[[OMP_ORIG_DECL:.*]]:2 = hlfir.declare %[[INIT_ARG0]] {uniq_name = "omp_orig"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
+!CHECK: %[[OMP_PRIV_DECL:.*]]:2 = hlfir.declare %[[INIT_ARG1]] {uniq_name = "omp_priv"} : (!fir.ref<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>)
+!CHECK: fir.call @_QMmaxtype_modPinitme(%[[OMP_PRIV_DECL]]#0, %[[OMP_ORIG_DECL]]#0) fastmath<contract> : (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>) -> ()
+!CHECK: %[[OMP_PRIV_VAL:.*]] = fir.load %[[OMP_PRIV_DECL]]#0 : !fir.ref<[[MAXTYPE]]>
!CHECK: omp.yield(%[[INIT_ARG1]] : !fir.ref<[[MAXTYPE]]>)
!CHECK: } combiner {
!CHECK: ^bb0(%[[LHS_ARG:.*]]: !fir.ref<[[MAXTYPE]]>, %[[RHS_ARG:.*]]: !fir.ref<[[MAXTYPE]]>):
>From 4edbacaf1fcf736f33c4a3189190adca8102a817 Mon Sep 17 00:00:00 2001
From: "Matt P. Dziubinski" <matt-p.dziubinski at hpe.com>
Date: Fri, 13 Mar 2026 15:27:34 -0500
Subject: [PATCH 3/6] Formatting
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 14 ++++++++++----
flang/lib/Lower/OpenMP/OpenMP.cpp | 10 ++++------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 4fb04929052b1..09359f0fbef6c 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -465,15 +465,21 @@ bool ClauseProcessor::processInitializer(
exprResult = fir::LoadOp::create(builder, loc, exprResult);
// For derived types in by-ref reductions, the init value
- // (e.g. t(0)) must be stored into omp_priv explicitly.
+ // must be stored into omp_priv explicitly.
// populateByRefInitAndCleanupRegions doesn't handle
// scalarInitValue for unboxed derived types, so we store
// here and return null to prevent a redundant store attempt.
- if (ompPrivVar &&
- fir::isa_ref_type(ompPrivVar.getType()) &&
+ if (ompPrivVar && fir::isa_ref_type(ompPrivVar.getType()) &&
fir::isa_derived(
fir::unwrapRefType(ompPrivVar.getType()))) {
- fir::StoreOp::create(builder, loc, exprResult, ompPrivVar);
+ // Only store if the expression result type matches the
+ // whole derived type. For component-level initializers
+ // (e.g. omp_priv%i=0), the assignment was already
+ // performed as a side effect during expression lowering.
+ mlir::Type privEleTy =
+ fir::unwrapRefType(ompPrivVar.getType());
+ if (exprResult.getType() == privEleTy)
+ fir::StoreOp::create(builder, loc, exprResult, ompPrivVar);
return mlir::Value{};
}
return exprResult;
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 042fb37743327..0eb3dc9e304c4 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3839,8 +3839,8 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
}
lower::StatementContext stmtCtx;
- mlir::Type eleTy = isByRef ? fir::unwrapRefType(lhs.getType())
- : lhs.getType();
+ mlir::Type eleTy =
+ isByRef ? fir::unwrapRefType(lhs.getType()) : lhs.getType();
bool isDerived = fir::isa_derived(eleTy);
mlir::Value result = common::visit(
@@ -3880,11 +3880,9 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
exprResult.getType())) {
mlir::Type expectedType =
- isByRef ? fir::unwrapRefType(lhs.getType())
- : lhs.getType();
+ isByRef ? fir::unwrapRefType(lhs.getType()) : lhs.getType();
if (expectedType == refType.getElementType())
- exprResult =
- fir::LoadOp::create(builder, loc, exprResult);
+ exprResult = fir::LoadOp::create(builder, loc, exprResult);
}
return exprResult;
}},
>From 708283471cf6256b7b78842dd8b7ceb4e61573c8 Mon Sep 17 00:00:00 2001
From: "Matt P. Dziubinski" <matt-p.dziubinski at hpe.com>
Date: Tue, 17 Mar 2026 20:26:45 -0500
Subject: [PATCH 4/6] Fix derived-type reduction combiner lowering to use typed
assignments
Improve the combiner callback in processReductionCombiner to correctly handle derived-type reductions by extracting the typed evaluate::Assignment from the parser-level OmpStylizedInstance.
For whole-struct combiners (e.g., omp_out = mycombine(omp_out, omp_in)), use hlfir::AssignOp or user-defined assignment via the typed assignment instead of the previous approach that scanned for DesignateOp users. This properly handles both intrinsic and user-defined assignment.
For component-level combiners (e.g. omp_out%x = omp_out%x + omp_in%x), the expression is evaluated via convertExprToValue which only produces the RHS value. Since the result type (i32) doesn't match the reduction variable type (the derived type), use hlfir.assign to store the result into omp_out and skip the whole-variable fir.store.
Update tests to match the new output:
- declare-reduction-intrinsic-op.f90: expect hlfir.assign instead of fir.store for the component-level combiner result.
- omp-declare-reduction-derivedtype.f90: expect hlfir.assign + destroy instead of hlfir.associate + fir.load + hlfir.end_associate for the whole-struct combiner result.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 113 +++++++++++++-----
.../OpenMP/declare-reduction-intrinsic-op.f90 | 2 +-
.../omp-declare-reduction-derivedtype.f90 | 5 +-
3 files changed, 83 insertions(+), 37 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 0eb3dc9e304c4..762772070f6a2 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -18,6 +18,7 @@
#include "Decomposer.h"
#include "Utils.h"
#include "flang/Common/idioms.h"
+#include "flang/Evaluate/expression.h"
#include "flang/Evaluate/tools.h"
#include "flang/Evaluate/type.h"
#include "flang/Lower/Bridge.h"
@@ -3796,12 +3797,24 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, const clause::Combiner &combiner) {
+ semantics::SemanticsContext &semaCtx, const clause::Combiner &combiner,
+ const parser::OmpStylizedInstance &parserInst) {
+ // Extract the typed assignment from the parser-level instance, if
+ // the combiner is an assignment statement (as opposed to a call).
+ const evaluate::Assignment *assign = nullptr;
+ const auto &instance = std::get<parser::OmpStylizedInstance::Instance>(
+ parserInst.t);
+ if (const auto *assignStmt =
+ std::get_if<parser::AssignmentStmt>(&instance.u)) {
+ if (auto *wrapper = assignStmt->typedAssignment.get())
+ if (wrapper->v)
+ assign = &*wrapper->v;
+ }
ReductionProcessor::GenCombinerCBTy genCombinerCB;
const StylizedInstance &inst = combiner.v.front();
semantics::SomeExpr evalExpr = std::get<StylizedInstance::Instance>(inst.t);
- genCombinerCB = [&, evalExpr](fir::FirOpBuilder &builder, mlir::Location loc,
+ genCombinerCB = [&, evalExpr, assign](fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Type type, mlir::Value lhs,
mlir::Value rhs, bool isByRef) {
lower::SymMapScope scope(symTable);
@@ -3838,11 +3851,44 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
symTable.addVariableDefinition(*object.sym(), declareOp);
}
- lower::StatementContext stmtCtx;
- mlir::Type eleTy =
- isByRef ? fir::unwrapRefType(lhs.getType()) : lhs.getType();
- bool isDerived = fir::isa_derived(eleTy);
+ // For derived types with a typed assignment available, use
+ // hlfir::AssignOp or user-defined assignment directly instead of
+ // trying to convert the expression to a value (which doesn't work
+ // for record types). Only take this path when the assignment RHS
+ // itself is a derived type -- i.e. the combiner assigns to the whole
+ // derived-type variable (e.g. omp_out = mycombine(omp_out, omp_in)).
+ // When the combiner assigns to a component (e.g. omp_out%x = ...),
+ // the RHS is a scalar intrinsic type and the existing convertExprToValue
+ // path handles it correctly.
+ bool rhsIsDerived = assign && assign->rhs.GetType() &&
+ assign->rhs.GetType()->category() == common::TypeCategory::Derived;
+ if (rhsIsDerived && isByRef &&
+ mlir::isa<fir::RecordType>(fir::unwrapRefType(lhs.getType()))) {
+ lower::StatementContext stmtCtx;
+ hlfir::Entity lhsEntity{ompOutVar};
+ hlfir::Entity rhsEntity = lower::convertExprToHLFIR(
+ loc, converter, assign->rhs, symTable, stmtCtx);
+ common::visit(
+ common::visitors{
+ [&](const evaluate::Assignment::Intrinsic &) {
+ hlfir::AssignOp::create(builder, loc, rhsEntity, lhsEntity);
+ },
+ [&](const evaluate::ProcedureRef &procRef) {
+ lower::convertUserDefinedAssignmentToHLFIR(
+ loc, converter, procRef, lhsEntity, rhsEntity, symTable);
+ },
+ [&](const auto &) {
+ llvm_unreachable(
+ "Unexpected assignment type in reduction combiner");
+ },
+ },
+ assign->u);
+ stmtCtx.finalizeAndPop();
+ mlir::omp::YieldOp::create(builder, loc, lhs);
+ return;
+ }
+ lower::StatementContext stmtCtx;
mlir::Value result = common::visit(
common::visitors{
[&](const evaluate::ProcedureRef &procRef) -> mlir::Value {
@@ -3852,31 +3898,10 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
return outVal;
},
[&](const auto &expr) -> mlir::Value {
- if (isDerived && isByRef) {
- mlir::Value exprResult = fir::getBase(convertExprToValue(
- loc, converter, evalExpr, symTable, stmtCtx));
- // Check if this is a component-level expression (has a
- // designator on omp_out) or a whole-struct expression
- // (function call returning a struct).
- mlir::Value designator;
- for (auto *user : ompOutVar.getUsers()) {
- if (auto desig = mlir::dyn_cast<hlfir::DesignateOp>(user))
- designator = desig.getResult();
- }
- if (designator) {
- // Component-level: store into the designator.
- fir::StoreOp::create(builder, loc, exprResult, designator);
- return mlir::Value{};
- }
- // Whole-struct function result: return the value so the
- // post-visit code can store it into lhs.
- if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
- exprResult.getType()))
- exprResult = fir::LoadOp::create(builder, loc, exprResult);
- return exprResult;
- }
mlir::Value exprResult = fir::getBase(convertExprToValue(
loc, converter, evalExpr, symTable, stmtCtx));
+ // Optional load may be generated if we get a reference to the
+ // reduction type.
if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
exprResult.getType())) {
mlir::Type expectedType =
@@ -3884,14 +3909,26 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
if (expectedType == refType.getElementType())
exprResult = fir::LoadOp::create(builder, loc, exprResult);
}
+ // For component-level derived-type combiners (e.g.
+ // omp_out%x = omp_out%x + omp_in%x), the assignment was
+ // not performed during expression lowering since
+ // convertExprToValue only evaluates the RHS value.
+ // The result type won't match the reduction variable type,
+ // so store to omp_out via hlfir.assign and skip the
+ // whole-variable fir.store below.
+ if (isByRef && exprResult.getType() !=
+ fir::unwrapRefType(lhs.getType())) {
+ hlfir::Entity lhsEntity{ompOutVar};
+ hlfir::AssignOp::create(builder, loc, exprResult, lhsEntity);
+ return mlir::Value{};
+ }
return exprResult;
}},
evalExpr.u);
stmtCtx.finalizeAndPop();
if (isByRef) {
- if (result) {
+ if (result)
fir::StoreOp::create(builder, loc, result, lhs);
- }
mlir::omp::YieldOp::create(builder, loc, lhs);
} else {
mlir::omp::YieldOp::create(builder, loc, result);
@@ -3989,8 +4026,20 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
clause::ReductionOperator redOp =
clause::makeReductionOperator(identifier, semaCtx);
+ // Get the parser-level combiner expression so we can pass each
+ // parser::OmpStylizedInstance to processReductionCombiner.
+ // The combiner expression's instances correspond 1:1 to typeNameList entries.
+ const auto *combinerExpr = parser::omp::GetCombinerExpr(specifier);
+ assert(combinerExpr && "Expecting combiner expression");
+ auto parserInstIt = combinerExpr->v.begin();
+
for (const auto &typeSpec : typeNameList.v) {
(void)typeSpec; // Currently unused
+
+ assert(parserInstIt != combinerExpr->v.end() &&
+ "Mismatched combiner instance count");
+ const parser::OmpStylizedInstance &parserInst = *parserInstIt++;
+
mlir::Type reductionType = getReductionType(converter, specifier);
bool isByRef = ReductionProcessor::doReductionByRef(reductionType);
// Compute the canonical reduction name the same way
@@ -4022,7 +4071,7 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
redOp.u);
ReductionProcessor::GenCombinerCBTy genCombinerCB =
- processReductionCombiner(converter, symTable, semaCtx, combiner);
+ processReductionCombiner(converter, symTable, semaCtx, combiner, parserInst);
ReductionProcessor::GenInitValueCBTy genInitValueCB;
ClauseProcessor cp(converter, semaCtx, clauses);
cp.processInitializer(symTable, genInitValueCB);
diff --git a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90 b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
index 1ce41346d5468..f90326fff08fb 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
@@ -31,6 +31,6 @@ end program test
! CHECK: %[[IN_X:.*]] = hlfir.designate %[[OMP_IN]]#0{"x"} : (!fir.ref<[[TY]]>) -> !fir.ref<i32>
! CHECK: %[[IN_X_VAL:.*]] = fir.load %[[IN_X]] : !fir.ref<i32>
! CHECK: %[[ADD:.*]] = arith.addi %[[OUT_X_VAL]], %[[IN_X_VAL]] : i32
-! CHECK: fir.store %[[ADD]] to %[[OUT_X]] : !fir.ref<i32>
+! CHECK: hlfir.assign %[[ADD]] to %[[OMP_OUT]]#0 : i32, !fir.ref<[[TY]]>
! CHECK: omp.yield(%[[ARG0]] : !fir.ref<[[TY]]>)
! CHECK: }
diff --git a/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90 b/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
index 0ab32bf5a9be6..6d3cb4378a9b2 100644
--- a/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
+++ b/flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90
@@ -63,11 +63,8 @@ end module maxtype_mod
!CHECK: fir.save_result %[[COMBINE_RESULT]] to %[[TMPRESULT]]#0 : [[MAXTYPE]], !fir.ref<[[MAXTYPE]]>
!CHECK: %false = arith.constant false
!CHECK: %[[EXPRRESULT:.*]] = hlfir.as_expr %[[TMPRESULT]]#0 move %false : (!fir.ref<[[MAXTYPE]]>, i1) -> !hlfir.expr<[[MAXTYPE]]>
-!CHECK: %[[ASSOCIATE:.*]]:3 = hlfir.associate %[[EXPRRESULT]] {adapt.valuebyref} : (!hlfir.expr<[[MAXTYPE]]>) -> (!fir.ref<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>, i1)
-!CHECK: %[[RESULT_VAL:.*]] = fir.load %[[ASSOCIATE]]#0 : !fir.ref<[[MAXTYPE]]>
-!CHECK: hlfir.end_associate %[[ASSOCIATE]]#1, %[[ASSOCIATE]]#2 : !fir.ref<[[MAXTYPE]]>, i1
+!CHECK: hlfir.assign %[[EXPRRESULT]] to %[[OMP_OUT]]#0 : !hlfir.expr<[[MAXTYPE]]>, !fir.ref<[[MAXTYPE]]>
!CHECK: hlfir.destroy %[[EXPRRESULT]] : !hlfir.expr<[[MAXTYPE]]>
-!CHECK: fir.store %[[RESULT_VAL]] to %[[LHS_ARG]] : !fir.ref<[[MAXTYPE]]>
!CHECK: omp.yield(%[[LHS_ARG]] : !fir.ref<[[MAXTYPE]]>)
!CHECK: }
>From 01c622ea1b2a80bdc69e3da9b4905322f66ecc39 Mon Sep 17 00:00:00 2001
From: "Matt P. Dziubinski" <matt-p.dziubinski at hpe.com>
Date: Tue, 17 Mar 2026 20:28:10 -0500
Subject: [PATCH 5/6] Formatting
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 762772070f6a2..78a3cec485856 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3802,8 +3802,8 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
// Extract the typed assignment from the parser-level instance, if
// the combiner is an assignment statement (as opposed to a call).
const evaluate::Assignment *assign = nullptr;
- const auto &instance = std::get<parser::OmpStylizedInstance::Instance>(
- parserInst.t);
+ const auto &instance =
+ std::get<parser::OmpStylizedInstance::Instance>(parserInst.t);
if (const auto *assignStmt =
std::get_if<parser::AssignmentStmt>(&instance.u)) {
if (auto *wrapper = assignStmt->typedAssignment.get())
@@ -3814,9 +3814,10 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
const StylizedInstance &inst = combiner.v.front();
semantics::SomeExpr evalExpr = std::get<StylizedInstance::Instance>(inst.t);
- genCombinerCB = [&, evalExpr, assign](fir::FirOpBuilder &builder, mlir::Location loc,
- mlir::Type type, mlir::Value lhs,
- mlir::Value rhs, bool isByRef) {
+ genCombinerCB = [&, evalExpr, assign](fir::FirOpBuilder &builder,
+ mlir::Location loc, mlir::Type type,
+ mlir::Value lhs, mlir::Value rhs,
+ bool isByRef) {
lower::SymMapScope scope(symTable);
mlir::Value ompOutVar;
for (const Object &object : std::get<StylizedInstance::Variables>(inst.t)) {
@@ -3860,7 +3861,8 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
// When the combiner assigns to a component (e.g. omp_out%x = ...),
// the RHS is a scalar intrinsic type and the existing convertExprToValue
// path handles it correctly.
- bool rhsIsDerived = assign && assign->rhs.GetType() &&
+ bool rhsIsDerived =
+ assign && assign->rhs.GetType() &&
assign->rhs.GetType()->category() == common::TypeCategory::Derived;
if (rhsIsDerived && isByRef &&
mlir::isa<fir::RecordType>(fir::unwrapRefType(lhs.getType()))) {
@@ -3916,8 +3918,8 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
// The result type won't match the reduction variable type,
// so store to omp_out via hlfir.assign and skip the
// whole-variable fir.store below.
- if (isByRef && exprResult.getType() !=
- fir::unwrapRefType(lhs.getType())) {
+ if (isByRef &&
+ exprResult.getType() != fir::unwrapRefType(lhs.getType())) {
hlfir::Entity lhsEntity{ompOutVar};
hlfir::AssignOp::create(builder, loc, exprResult, lhsEntity);
return mlir::Value{};
@@ -4071,7 +4073,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
redOp.u);
ReductionProcessor::GenCombinerCBTy genCombinerCB =
- processReductionCombiner(converter, symTable, semaCtx, combiner, parserInst);
+ processReductionCombiner(converter, symTable, semaCtx, combiner,
+ parserInst);
ReductionProcessor::GenInitValueCBTy genInitValueCB;
ClauseProcessor cp(converter, semaCtx, clauses);
cp.processInitializer(symTable, genInitValueCB);
>From 00518791e0999e475d508496fbf9131cfaebf855 Mon Sep 17 00:00:00 2001
From: "Matt P. Dziubinski" <matt-p.dziubinski at hpe.com>
Date: Tue, 17 Mar 2026 20:47:22 -0500
Subject: [PATCH 6/6] Fix the assignment lowering for component-level combiners
For component-level combiners, the assignment LHS is lowered from the
typed assignment (`assign->lhs`) to obtain the correct component
designator (e.g., `omp_out%x`), rather than assigning the scalar
result to the whole derived-type variable, which would cause
an assertion failure in `genComponentByComponentAssignment` during
HLFIR-to-FIR lowering.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 19 ++++++++++++++-----
.../OpenMP/declare-reduction-intrinsic-op.f90 | 3 ++-
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 78a3cec485856..f66fe314f28ee 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3915,13 +3915,22 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
// omp_out%x = omp_out%x + omp_in%x), the assignment was
// not performed during expression lowering since
// convertExprToValue only evaluates the RHS value.
- // The result type won't match the reduction variable type,
- // so store to omp_out via hlfir.assign and skip the
- // whole-variable fir.store below.
+ // The result type won't match the reduction variable type.
+ // Use the typed assignment LHS to store to the correct
+ // component, then skip the whole-variable fir.store below.
if (isByRef &&
exprResult.getType() != fir::unwrapRefType(lhs.getType())) {
- hlfir::Entity lhsEntity{ompOutVar};
- hlfir::AssignOp::create(builder, loc, exprResult, lhsEntity);
+ if (assign) {
+ lower::StatementContext assignCtx;
+ hlfir::Entity lhsEntity = lower::convertExprToHLFIR(
+ loc, converter, assign->lhs, symTable, assignCtx);
+ hlfir::AssignOp::create(builder, loc, exprResult, lhsEntity);
+ assignCtx.finalizeAndPop();
+ } else {
+ // Fallback: store to omp_out directly (shouldn't normally
+ // happen for well-formed component-level combiners).
+ fir::StoreOp::create(builder, loc, exprResult, ompOutVar);
+ }
return mlir::Value{};
}
return exprResult;
diff --git a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90 b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
index f90326fff08fb..f519ddb6b5989 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-intrinsic-op.f90
@@ -31,6 +31,7 @@ end program test
! CHECK: %[[IN_X:.*]] = hlfir.designate %[[OMP_IN]]#0{"x"} : (!fir.ref<[[TY]]>) -> !fir.ref<i32>
! CHECK: %[[IN_X_VAL:.*]] = fir.load %[[IN_X]] : !fir.ref<i32>
! CHECK: %[[ADD:.*]] = arith.addi %[[OUT_X_VAL]], %[[IN_X_VAL]] : i32
-! CHECK: hlfir.assign %[[ADD]] to %[[OMP_OUT]]#0 : i32, !fir.ref<[[TY]]>
+! CHECK: %[[OUT_X2:.*]] = hlfir.designate %[[OMP_OUT]]#0{"x"} : (!fir.ref<[[TY]]>) -> !fir.ref<i32>
+! CHECK: hlfir.assign %[[ADD]] to %[[OUT_X2]] : i32, !fir.ref<i32>
! CHECK: omp.yield(%[[ARG0]] : !fir.ref<[[TY]]>)
! CHECK: }
More information about the flang-commits
mailing list