[flang-commits] [flang] [Flang][OpenMP] Fix crash with character types in declare_reduction (PR #178038)
Krish Gupta via flang-commits
flang-commits at lists.llvm.org
Wed Jan 28 04:43:00 PST 2026
https://github.com/KrxGu updated https://github.com/llvm/llvm-project/pull/178038
>From 8cf6a3246d6e0aaa6caabfacf8f4add5e13c94fb Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Tue, 27 Jan 2026 01:27:27 +0530
Subject: [PATCH 1/9] [Flang][OpenMP] Fix crash with character types in
declare_reduction
Fixes #177501
Character types in OpenMP reductions were causing crashes during lowering.
The issue was that character types need explicit handling for type parameters
and reference wrapping throughout the reduction lowering pipeline. This fix
ensures hlfir.declare operations get proper character length parameters and
that by-ref reductions wrap character types correctly.
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 26 +++++++++--
flang/lib/Lower/OpenMP/OpenMP.cpp | 21 +++++++--
.../Lower/Support/PrivateReductionUtils.cpp | 45 +++++++++++++++++--
.../lib/Lower/Support/ReductionProcessor.cpp | 5 +++
...eclare-reduction-character-allocatable.f90 | 34 ++++++++++++++
5 files changed, 119 insertions(+), 12 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 51458b2a281d5..8ca35ea83676e 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -394,16 +394,34 @@ bool ClauseProcessor::processInitializer(
for (const Object &object :
std::get<StylizedInstance::Variables>(inst.t)) {
- mlir::Value addr = builder.createTemporary(loc, ompOrig.getType());
- fir::StoreOp::create(builder, loc, ompOrig, addr);
+ mlir::Value addr;
+ mlir::Type ompOrigType = ompOrig.getType();
+ // If ompOrig is already a reference, we can use it directly
+ if (fir::isa_ref_type(ompOrigType)) {
+ addr = ompOrig;
+ } else {
+ addr = builder.createTemporary(loc, ompOrigType);
+ fir::StoreOp::create(builder, loc, ompOrig, addr);
+ }
fir::FortranVariableFlagsEnum extraFlags = {};
fir::FortranVariableFlagsAttr attributes =
Fortran::lower::translateSymbolAttributes(
builder.getContext(), *object.sym(), extraFlags);
std::string name = object.sym()->name().ToString();
+ // For character types, we need to provide the length parameter
+ llvm::SmallVector<mlir::Value> typeParams;
+ mlir::Type unwrappedType = fir::unwrapRefType(ompOrigType);
+ if (auto charTy = mlir::dyn_cast<fir::CharacterType>(unwrappedType)) {
+ if (charTy.hasConstantLen()) {
+ mlir::Value lenValue = builder.createIntegerConstant(
+ loc, builder.getIndexType(), charTy.getLen());
+ typeParams.push_back(lenValue);
+ }
+ }
auto declareOp =
- hlfir::DeclareOp::create(builder, loc, addr, name, nullptr, {},
- nullptr, nullptr, 0, attributes);
+ hlfir::DeclareOp::create(builder, loc, addr, name, nullptr,
+ typeParams, nullptr, nullptr, 0,
+ attributes);
if (name == "omp_priv")
ompPrivVar = declareOp.getResult(0);
symMap.addVariableDefinition(*object.sym(), declareOp);
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 27e7e6e7c1d25..b65c302f084c1 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3767,9 +3767,19 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
fir::FortranVariableFlagsAttr attributes =
Fortran::lower::translateSymbolAttributes(builder.getContext(),
*object.sym(), extraFlags);
+ // For character types, we need to provide the length parameter
+ llvm::SmallVector<mlir::Value> typeParams;
+ mlir::Type unwrappedType = fir::unwrapRefType(type);
+ if (auto charTy = mlir::dyn_cast<fir::CharacterType>(unwrappedType)) {
+ if (charTy.hasConstantLen()) {
+ mlir::Value lenValue = builder.createIntegerConstant(
+ loc, builder.getIndexType(), charTy.getLen());
+ typeParams.push_back(lenValue);
+ }
+ }
auto declareOp =
- hlfir::DeclareOp::create(builder, loc, addr, name, nullptr, {},
- nullptr, nullptr, 0, attributes);
+ hlfir::DeclareOp::create(builder, loc, addr, name, nullptr,
+ typeParams, nullptr, nullptr, 0, attributes);
if (name == "omp_out")
ompOutVar = declareOp.getResult(0);
symTable.addVariableDefinition(*object.sym(), declareOp);
@@ -3790,9 +3800,12 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
// Optional load may be generated if we get a reference to the
// reduction type.
if (auto refType =
- llvm::dyn_cast<fir::ReferenceType>(exprResult.getType()))
- if (lhs.getType() == refType.getElementType())
+ llvm::dyn_cast<fir::ReferenceType>(exprResult.getType())) {
+ mlir::Type expectedType =
+ isByRef ? fir::unwrapRefType(lhs.getType()) : lhs.getType();
+ if (expectedType == refType.getElementType())
exprResult = fir::LoadOp::create(builder, loc, exprResult);
+ }
return exprResult;
}},
evalExpr.u);
diff --git a/flang/lib/Lower/Support/PrivateReductionUtils.cpp b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
index c6c428860bca1..617a4f4699459 100644
--- a/flang/lib/Lower/Support/PrivateReductionUtils.cpp
+++ b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
@@ -89,6 +89,31 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
mlir::Value arg = builder.loadIfRef(loc, block->getArgument(0));
assert(mlir::isa<fir::BaseBoxType>(arg.getType()));
+ // Special handling for boxed character types - they need to use
+ // fir.box_addr instead of hlfir::genVariableRawAddress to avoid creating
+ // an hlfir::Entity from a value that doesn't satisfy isFortranEntity.
+ mlir::Type innerTy = fir::unwrapRefType(
+ mlir::cast<fir::BaseBoxType>(arg.getType()).getEleTy());
+ if (fir::isa_char(innerTy)) {
+ mlir::Value addr = fir::BoxAddrOp::create(builder, loc, arg);
+ mlir::Value isAllocated = builder.genIsNotNullAddr(loc, addr);
+ fir::IfOp ifOp = fir::IfOp::create(builder, loc, isAllocated,
+ /*withElseRegion=*/false);
+ builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
+
+ mlir::Value cast = builder.createConvert(
+ loc, fir::HeapType::get(fir::dyn_cast_ptrEleTy(addr.getType())),
+ addr);
+ fir::FreeMemOp::create(builder, loc, cast);
+
+ builder.setInsertionPointAfter(ifOp);
+ if (isDoConcurrent)
+ fir::YieldOp::create(builder, loc);
+ else
+ mlir::omp::YieldOp::create(builder, loc);
+ return;
+ }
+
// Deallocate box
// The FIR type system doesn't nesecarrily know that this is a mutable box
// if we allocated the thread local array on the heap to avoid looped stack
@@ -620,7 +645,9 @@ void PopulateInitAndCleanupRegionsHelper::populateByRefInitAndCleanupRegions() {
assert(sym && "Symbol information is required to privatize derived types");
assert(!scalarInitValue && "ScalarInitvalue is unused for privatization");
}
- if (hlfir::Entity{moldArg}.isAssumedRank())
+ // Only check for assumed rank if moldArg is a valid Fortran entity.
+ // Boxed types (like allocatable characters) may not be valid entities yet.
+ if (hlfir::isFortranEntity(moldArg) && hlfir::Entity{moldArg}.isAssumedRank())
TODO(loc, "Privatization of assumed rank variable");
mlir::Type valTy = fir::unwrapRefType(argType);
@@ -649,8 +676,9 @@ void PopulateInitAndCleanupRegionsHelper::populateByRefInitAndCleanupRegions() {
bool isChar = fir::isa_char(innerTy);
if (fir::isa_trivial(innerTy) || isDerived || isChar) {
// boxed non-sequence value e.g. !fir.box<!fir.heap<i32>>
- if ((isDerived || isChar) && (isReduction(kind) || scalarInitValue))
- TODO(loc, "Reduction of an unsupported boxed type");
+ // Character types in reductions are supported, but derived types are not yet.
+ if (isDerived && (isReduction(kind) || scalarInitValue))
+ TODO(loc, "Reduction of an unsupported boxed derived type");
initAndCleanupBoxedScalar(boxTy, needsInitialization);
return;
}
@@ -663,10 +691,19 @@ void PopulateInitAndCleanupRegionsHelper::populateByRefInitAndCleanupRegions() {
}
// Unboxed types:
- if (auto boxCharTy = mlir::dyn_cast<fir::BoxCharType>(argType)) {
+ if (auto boxCharTy = mlir::dyn_cast<fir::BoxCharType>(valTy)) {
initAndCleanupBoxchar(boxCharTy);
return;
}
+ // Handle unboxed character types (e.g., !fir.char<1,1>).
+ // For fixed-length character types, we just need to initialize the value.
+ if (fir::isa_char(valTy)) {
+ builder.setInsertionPointToEnd(initBlock);
+ if (scalarInitValue)
+ builder.createStoreWithConvert(loc, scalarInitValue, allocatedPrivVarArg);
+ createYield(allocatedPrivVarArg);
+ return;
+ }
if (fir::isa_derived(valType)) {
initAndCleanupUnboxedDerivedType(needsInitialization);
return;
diff --git a/flang/lib/Lower/Support/ReductionProcessor.cpp b/flang/lib/Lower/Support/ReductionProcessor.cpp
index 0e01268dd74ff..e0cba4c512258 100644
--- a/flang/lib/Lower/Support/ReductionProcessor.cpp
+++ b/flang/lib/Lower/Support/ReductionProcessor.cpp
@@ -582,6 +582,11 @@ DeclareRedType ReductionProcessor::createDeclareReductionHelper(
if (isByRef) {
boxedTy = fir::unwrapPassByRefType(valTy);
boxedTyAttr = mlir::TypeAttr::get(boxedTy);
+ // For character types that are not already references, we need to wrap
+ // them in a reference type for by-ref reductions.
+ if (fir::isa_char(valTy) && !fir::isa_ref_type(type)) {
+ type = fir::ReferenceType::get(valTy);
+ }
} else
type = valTy;
diff --git a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90 b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
new file mode 100644
index 0000000000000..ccc5669a19ee1
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
@@ -0,0 +1,34 @@
+! Test OpenMP declare reduction with character type and allocatable variable.
+! This test ensures that Flang doesn't crash when processing user-defined
+! reductions with character types on allocatable variables.
+! See issue: https://github.com/llvm/llvm-project/issues/177501
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! Test basic character reduction with allocatable variable
+program test_character_reduction
+ character(len=1), allocatable :: k1
+
+ !$omp declare reduction (char_max:character(len=1):omp_out=max(omp_out,omp_in)) &
+ !$omp initializer(omp_priv='a')
+
+ !$omp parallel sections reduction(char_max:k1)
+ k1 = max(k1, 'z')
+ !$omp end parallel sections
+
+end program test_character_reduction
+
+! Verify the declare_reduction is generated with reference type for character
+! CHECK-LABEL: omp.declare_reduction @char_max : !fir.ref<!fir.char<1>>
+! CHECK-SAME: init
+! CHECK: omp.yield
+
+! Verify the combiner region works
+! CHECK: combiner
+! CHECK: hlfir.declare
+! CHECK: omp.yield
+
+! Verify the reduction is used in the parallel sections
+! CHECK: omp.parallel
+! CHECK: omp.sections reduction(byref @char_max
+
>From d47dd52b3f1932e465d618b4b241b2bfe61a2ac1 Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Tue, 27 Jan 2026 01:38:56 +0530
Subject: [PATCH 2/9] Fix clang-format issues
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 7 +++----
flang/lib/Lower/OpenMP/OpenMP.cpp | 4 ++--
flang/lib/Lower/Support/PrivateReductionUtils.cpp | 3 ++-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 8ca35ea83676e..c53d4a00f5be5 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -418,10 +418,9 @@ bool ClauseProcessor::processInitializer(
typeParams.push_back(lenValue);
}
}
- auto declareOp =
- hlfir::DeclareOp::create(builder, loc, addr, name, nullptr,
- typeParams, nullptr, nullptr, 0,
- attributes);
+ auto declareOp = hlfir::DeclareOp::create(builder, loc, addr, name,
+ nullptr, typeParams, nullptr,
+ nullptr, 0, attributes);
if (name == "omp_priv")
ompPrivVar = declareOp.getResult(0);
symMap.addVariableDefinition(*object.sym(), declareOp);
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index b65c302f084c1..653ea7ebaa82c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3799,8 +3799,8 @@ static ReductionProcessor::GenCombinerCBTy processReductionCombiner(
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())) {
+ if (auto refType = llvm::dyn_cast<fir::ReferenceType>(
+ exprResult.getType())) {
mlir::Type expectedType =
isByRef ? fir::unwrapRefType(lhs.getType()) : lhs.getType();
if (expectedType == refType.getElementType())
diff --git a/flang/lib/Lower/Support/PrivateReductionUtils.cpp b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
index 617a4f4699459..5d1a3195adc9c 100644
--- a/flang/lib/Lower/Support/PrivateReductionUtils.cpp
+++ b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
@@ -676,7 +676,8 @@ void PopulateInitAndCleanupRegionsHelper::populateByRefInitAndCleanupRegions() {
bool isChar = fir::isa_char(innerTy);
if (fir::isa_trivial(innerTy) || isDerived || isChar) {
// boxed non-sequence value e.g. !fir.box<!fir.heap<i32>>
- // Character types in reductions are supported, but derived types are not yet.
+ // Character types in reductions are supported, but derived types are not
+ // yet.
if (isDerived && (isReduction(kind) || scalarInitValue))
TODO(loc, "Reduction of an unsupported boxed derived type");
initAndCleanupBoxedScalar(boxTy, needsInitialization);
>From d33a487baf41f7f704e2d778585bb8c3233b7a82 Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Tue, 27 Jan 2026 01:41:49 +0530
Subject: [PATCH 3/9] Fix typo in comment: nesecarrily -> necessarily
---
flang/lib/Lower/Support/PrivateReductionUtils.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Lower/Support/PrivateReductionUtils.cpp b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
index 5d1a3195adc9c..c02e1622c429a 100644
--- a/flang/lib/Lower/Support/PrivateReductionUtils.cpp
+++ b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
@@ -115,7 +115,7 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
}
// Deallocate box
- // The FIR type system doesn't nesecarrily know that this is a mutable box
+ // The FIR type system doesn't necessarily know that this is a mutable box
// if we allocated the thread local array on the heap to avoid looped stack
// allocations.
mlir::Value addr =
>From 56fdcfcbbcbbf6a7ffdadbc1284b2f49af7e2c5d Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Tue, 27 Jan 2026 01:57:51 +0530
Subject: [PATCH 4/9] Fix test: Change CHECK-SAME to CHECK for init keyword
The init keyword appears on a separate line in the IR output,
so using CHECK instead of CHECK-SAME to properly match the format.
---
.../Lower/OpenMP/declare-reduction-character-allocatable.f90 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90 b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
index ccc5669a19ee1..81ae0f0dce9da 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
@@ -20,7 +20,7 @@ end program test_character_reduction
! Verify the declare_reduction is generated with reference type for character
! CHECK-LABEL: omp.declare_reduction @char_max : !fir.ref<!fir.char<1>>
-! CHECK-SAME: init
+! CHECK: init {
! CHECK: omp.yield
! Verify the combiner region works
>From c3b758102276fbfb0a325d9a181e288e9cfc9361 Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Tue, 27 Jan 2026 23:30:34 +0530
Subject: [PATCH 5/9] Address review feedback: improve character type handling
- Use hlfir::genLengthParameters helper instead of manual extraction
to support both constant and runtime-length characters
- Refactor cleanup region to share deallocation logic, with only
different paths for addr extraction (cleaner code as suggested)
- Add clarifying comments about boxchar handling for runtime-length
characters passed by value
Per Tom's review feedback.
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 17 ++++----
.../Lower/Support/PrivateReductionUtils.cpp | 42 +++++++------------
2 files changed, 26 insertions(+), 33 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index c53d4a00f5be5..9a2412e43d3fa 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -408,14 +408,17 @@ bool ClauseProcessor::processInitializer(
Fortran::lower::translateSymbolAttributes(
builder.getContext(), *object.sym(), extraFlags);
std::string name = object.sym()->name().ToString();
- // For character types, we need to provide the length parameter
+ // Get length parameters for types that need them (e.g., characters)
llvm::SmallVector<mlir::Value> typeParams;
- mlir::Type unwrappedType = fir::unwrapRefType(ompOrigType);
- if (auto charTy = mlir::dyn_cast<fir::CharacterType>(unwrappedType)) {
- if (charTy.hasConstantLen()) {
- mlir::Value lenValue = builder.createIntegerConstant(
- loc, builder.getIndexType(), charTy.getLen());
- typeParams.push_back(lenValue);
+ if (hlfir::isFortranEntity(addr)) {
+ hlfir::genLengthParameters(loc, builder, hlfir::Entity{addr},
+ typeParams);
+ // EmboxOp verifier doesn't allow length parameters when character
+ // already has static LEN, so clear them in that case
+ if (auto charTy = mlir::dyn_cast<fir::CharacterType>(
+ fir::getFortranElementType(addr.getType()))) {
+ if (charTy.hasConstantLen())
+ typeParams.clear();
}
}
auto declareOp = hlfir::DeclareOp::create(builder, loc, addr, name,
diff --git a/flang/lib/Lower/Support/PrivateReductionUtils.cpp b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
index c02e1622c429a..395235fb904db 100644
--- a/flang/lib/Lower/Support/PrivateReductionUtils.cpp
+++ b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
@@ -89,37 +89,23 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
mlir::Value arg = builder.loadIfRef(loc, block->getArgument(0));
assert(mlir::isa<fir::BaseBoxType>(arg.getType()));
- // Special handling for boxed character types - they need to use
- // fir.box_addr instead of hlfir::genVariableRawAddress to avoid creating
- // an hlfir::Entity from a value that doesn't satisfy isFortranEntity.
+ // Extract address from the box. For character types, use fir.box_addr
+ // directly to avoid creating an hlfir::Entity from a value that doesn't
+ // satisfy isFortranEntity (boxed characters may not be valid entities yet).
mlir::Type innerTy = fir::unwrapRefType(
mlir::cast<fir::BaseBoxType>(arg.getType()).getEleTy());
+ mlir::Value addr;
if (fir::isa_char(innerTy)) {
- mlir::Value addr = fir::BoxAddrOp::create(builder, loc, arg);
- mlir::Value isAllocated = builder.genIsNotNullAddr(loc, addr);
- fir::IfOp ifOp = fir::IfOp::create(builder, loc, isAllocated,
- /*withElseRegion=*/false);
- builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
-
- mlir::Value cast = builder.createConvert(
- loc, fir::HeapType::get(fir::dyn_cast_ptrEleTy(addr.getType())),
- addr);
- fir::FreeMemOp::create(builder, loc, cast);
-
- builder.setInsertionPointAfter(ifOp);
- if (isDoConcurrent)
- fir::YieldOp::create(builder, loc);
- else
- mlir::omp::YieldOp::create(builder, loc);
- return;
+ addr = fir::BoxAddrOp::create(builder, loc, arg);
+ } else {
+ // For non-character types, use genVariableRawAddress
+ // The FIR type system doesn't necessarily know that this is a mutable
+ // box if we allocated the thread local array on the heap to avoid looped
+ // stack allocations.
+ addr = hlfir::genVariableRawAddress(loc, builder, hlfir::Entity{arg});
}
- // Deallocate box
- // The FIR type system doesn't necessarily know that this is a mutable box
- // if we allocated the thread local array on the heap to avoid looped stack
- // allocations.
- mlir::Value addr =
- hlfir::genVariableRawAddress(loc, builder, hlfir::Entity{arg});
+ // Deallocate if allocated
mlir::Value isAllocated = builder.genIsNotNullAddr(loc, addr);
fir::IfOp ifOp =
fir::IfOp::create(builder, loc, isAllocated, /*withElseRegion=*/false);
@@ -137,6 +123,10 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
return;
}
+ // Handle !fir.boxchar (passed by VALUE for runtime-length characters).
+ // Note: This is distinct from !fir.box<!fir.char<>> which is handled above.
+ // BoxChar is a special tuple type (addr, len) used when character length
+ // is only known at runtime.
if (auto boxCharTy = mlir::dyn_cast<fir::BoxCharType>(argType)) {
auto [addr, len] =
fir::factory::CharacterExprHelper{builder, loc}.createUnboxChar(
>From c841402246d7bb248256c2813196bace86956607 Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Tue, 27 Jan 2026 23:51:10 +0530
Subject: [PATCH 6/9] Fix: Don't clear type params for DeclareOp with
constant-len chars
DeclareOp requires exactly one type parameter for non-boxed characters,
even when they have constant length. The previous logic was incorrectly
clearing type params for constant-len chars (which is only needed for
EmboxOp, not DeclareOp).
This fixes the 'must be provided exactly one type parameter' error.
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 9a2412e43d3fa..fc3aef34b256f 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -408,18 +408,13 @@ bool ClauseProcessor::processInitializer(
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)
+ // 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.
llvm::SmallVector<mlir::Value> typeParams;
if (hlfir::isFortranEntity(addr)) {
hlfir::genLengthParameters(loc, builder, hlfir::Entity{addr},
typeParams);
- // EmboxOp verifier doesn't allow length parameters when character
- // already has static LEN, so clear them in that case
- if (auto charTy = mlir::dyn_cast<fir::CharacterType>(
- fir::getFortranElementType(addr.getType()))) {
- if (charTy.hasConstantLen())
- typeParams.clear();
- }
}
auto declareOp = hlfir::DeclareOp::create(builder, loc, addr, name,
nullptr, typeParams, nullptr,
>From 578a83d4eb946f0253c2076cf8e4d4b3b759cb96 Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Wed, 28 Jan 2026 16:49:25 +0530
Subject: [PATCH 7/9] Address review: remove unnecessary character special case
and add dynamic-length test
Per Tom's review:
1. Removed special character handling in cleanup region - boxed characters
ARE valid entities per isFortranVariableType, so genVariableRawAddress
should work for them too. Using standard path for all types now.
2. Added test case for dynamic-length characters (character(len=n) where n
is a runtime variable) to verify boxchar handling works correctly.
---
.../Lower/Support/PrivateReductionUtils.cpp | 21 ++++++-------------
...eclare-reduction-character-allocatable.f90 | 20 +++++++++++++++++-
2 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/flang/lib/Lower/Support/PrivateReductionUtils.cpp b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
index 395235fb904db..d1a965d288cad 100644
--- a/flang/lib/Lower/Support/PrivateReductionUtils.cpp
+++ b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
@@ -89,21 +89,12 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
mlir::Value arg = builder.loadIfRef(loc, block->getArgument(0));
assert(mlir::isa<fir::BaseBoxType>(arg.getType()));
- // Extract address from the box. For character types, use fir.box_addr
- // directly to avoid creating an hlfir::Entity from a value that doesn't
- // satisfy isFortranEntity (boxed characters may not be valid entities yet).
- mlir::Type innerTy = fir::unwrapRefType(
- mlir::cast<fir::BaseBoxType>(arg.getType()).getEleTy());
- mlir::Value addr;
- if (fir::isa_char(innerTy)) {
- addr = fir::BoxAddrOp::create(builder, loc, arg);
- } else {
- // For non-character types, use genVariableRawAddress
- // The FIR type system doesn't necessarily know that this is a mutable
- // box if we allocated the thread local array on the heap to avoid looped
- // stack allocations.
- addr = hlfir::genVariableRawAddress(loc, builder, hlfir::Entity{arg});
- }
+ // Extract address from the box for deallocation.
+ // The FIR type system doesn't necessarily know that this is a mutable
+ // box if we allocated the thread local array on the heap to avoid looped
+ // stack allocations.
+ mlir::Value addr =
+ hlfir::genVariableRawAddress(loc, builder, hlfir::Entity{arg});
// Deallocate if allocated
mlir::Value isAllocated = builder.genIsNotNullAddr(loc, addr);
diff --git a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90 b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
index 81ae0f0dce9da..183c8dc7a0717 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
@@ -5,7 +5,7 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
-! Test basic character reduction with allocatable variable
+! Test basic character reduction with allocatable variable (constant length)
program test_character_reduction
character(len=1), allocatable :: k1
@@ -32,3 +32,21 @@ end program test_character_reduction
! CHECK: omp.parallel
! CHECK: omp.sections reduction(byref @char_max
+! Test character reduction with dynamic length
+subroutine test_dynamic_length(n)
+ integer, intent(in) :: n
+ character(len=n) :: var
+
+ !$omp declare reduction (char_max_dyn:character(len=*):omp_out=max(omp_out,omp_in))
+
+ !$omp parallel reduction(char_max_dyn:var)
+ var = max(var, 'x')
+ !$omp end parallel
+
+end subroutine test_dynamic_length
+
+! Verify dynamic-length character reduction
+! CHECK-LABEL: func.func @_QPtest_dynamic_length
+! CHECK: omp.declare_reduction @char_max_dyn : !fir.boxchar<1>
+! CHECK: omp.parallel
+! CHECK: reduction(@char_max_dyn
>From 6e070d0c783ab8a21ac8ec1e2d3d1a3fae0a929c Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Wed, 28 Jan 2026 17:17:42 +0530
Subject: [PATCH 8/9] Fix test: use character(len=n) instead of
character(len=*) for dynamic-length test
character(len=*) is only valid for function parameters, not local variables.
Changed to character(len=n) where n is passed as an integer parameter.
---
.../OpenMP/declare-reduction-character-allocatable.f90 | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90 b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
index 183c8dc7a0717..a53e6bdc38e57 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
@@ -37,12 +37,7 @@ subroutine test_dynamic_length(n)
integer, intent(in) :: n
character(len=n) :: var
- !$omp declare reduction (char_max_dyn:character(len=*):omp_out=max(omp_out,omp_in))
-
- !$omp parallel reduction(char_max_dyn:var)
- var = max(var, 'x')
- !$omp end parallel
-
+ !$omp declare reduction (char_max_dyn:character(len=n):omp_out=max(omp_out,omp_in))
end subroutine test_dynamic_length
! Verify dynamic-length character reduction
>From 73f41b3896abf52268e694cae2cc5c5ef7f22aec Mon Sep 17 00:00:00 2001
From: KrxGu <krishom70 at gmail.com>
Date: Wed, 28 Jan 2026 18:12:35 +0530
Subject: [PATCH 9/9] Add missing initializer clause to dynamic-length
reduction test
Flang requires all declare_reduction directives to have an initializer
clause. Added initializer(omp_priv='a') to the dynamic-length test.
---
.../Lower/OpenMP/declare-reduction-character-allocatable.f90 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90 b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
index a53e6bdc38e57..68a1a8a250cd9 100644
--- a/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
+++ b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
@@ -37,7 +37,8 @@ subroutine test_dynamic_length(n)
integer, intent(in) :: n
character(len=n) :: var
- !$omp declare reduction (char_max_dyn:character(len=n):omp_out=max(omp_out,omp_in))
+ !$omp declare reduction (char_max_dyn:character(len=n):omp_out=max(omp_out,omp_in)) &
+ !$omp initializer(omp_priv='a')
end subroutine test_dynamic_length
! Verify dynamic-length character reduction
More information about the flang-commits
mailing list