[flang-commits] [flang] aef8a2c - [Flang][OpenMP] Fix crash with character types in declare_reduction (#178038)
via flang-commits
flang-commits at lists.llvm.org
Tue Feb 10 03:31:29 PST 2026
Author: Krish Gupta
Date: 2026-02-10T11:31:24Z
New Revision: aef8a2c483149d4ad2546843350002f11eb48a20
URL: https://github.com/llvm/llvm-project/commit/aef8a2c483149d4ad2546843350002f11eb48a20
DIFF: https://github.com/llvm/llvm-project/commit/aef8a2c483149d4ad2546843350002f11eb48a20.diff
LOG: [Flang][OpenMP] Fix crash with character types in declare_reduction (#178038)
Fixes #177501
This PR fixes a compilation crash when using character types in OpenMP
REDUCTION clauses with declare_reduction directives.
The problem was that character types weren't being handled properly
during OpenMP lowering. Specifically:
- Missing character length parameters in hlfir.declare operations
- Incorrect type wrapping for by-ref reductions
- Missing special case handling for boxed/unboxed character types
The fix ensures character types are treated similarly to derived types
throughout the reduction pipeline, since fir::isa_trivial() excludes
them.
Added a regression test to verify the fix works for both allocatable and
non-allocatable character reductions.
<img width="654" height="47" alt="image"
src="https://github.com/user-attachments/assets/cc962f01-3432-44ce-befb-324644767c8b"
/>
Added:
flang/test/Lower/OpenMP/Todo/reduction-character-dynamic-length.f90
flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
Modified:
flang/lib/Lower/OpenMP/ClauseProcessor.cpp
flang/lib/Lower/OpenMP/OpenMP.cpp
flang/lib/Lower/Support/PrivateReductionUtils.cpp
flang/lib/Lower/Support/ReductionProcessor.cpp
Removed:
################################################################################
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index b1973a3b8bf06..9f9dd436dc6a8 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -394,16 +394,42 @@ 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();
+ // Check for unsupported dynamic-length character reductions
+ mlir::Type unwrappedType = fir::unwrapRefType(ompOrigType);
+ if (mlir::isa<fir::BoxCharType>(unwrappedType)) {
+ TODO(loc, "OpenMP reduction allocation for dynamic length character");
+ }
+ if (auto charTy = mlir::dyn_cast<fir::CharacterType>(unwrappedType)) {
+ if (!charTy.hasConstantLen()) {
+ TODO(loc,
+ "OpenMP reduction allocation for dynamic length character");
+ }
+ }
+ // 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();
- auto declareOp =
- hlfir::DeclareOp::create(builder, loc, addr, name, nullptr, {},
- nullptr, nullptr, 0, attributes);
+ // 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);
+ }
+ 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 1d5b8b5ac436e..f7be823335bd9 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3765,9 +3765,15 @@ 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;
+ if (hlfir::isFortranEntity(addr)) {
+ hlfir::genLengthParameters(loc, builder, hlfir::Entity{addr},
+ typeParams);
+ }
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);
@@ -3787,10 +3793,13 @@ 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 (lhs.getType() == refType.getElementType())
+ if (auto refType = 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..d1a965d288cad 100644
--- a/flang/lib/Lower/Support/PrivateReductionUtils.cpp
+++ b/flang/lib/Lower/Support/PrivateReductionUtils.cpp
@@ -89,12 +89,14 @@ static void createCleanupRegion(Fortran::lower::AbstractConverter &converter,
mlir::Value arg = builder.loadIfRef(loc, block->getArgument(0));
assert(mlir::isa<fir::BaseBoxType>(arg.getType()));
- // 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
- // allocations.
+ // 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);
fir::IfOp ifOp =
fir::IfOp::create(builder, loc, isAllocated, /*withElseRegion=*/false);
@@ -112,6 +114,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(
@@ -620,7 +626,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 +657,10 @@ 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 +673,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/Todo/reduction-character-dynamic-length.f90 b/flang/test/Lower/OpenMP/Todo/reduction-character-dynamic-length.f90
new file mode 100644
index 0000000000000..b18c6f6c6fb70
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/reduction-character-dynamic-length.f90
@@ -0,0 +1,11 @@
+! RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: OpenMP reduction allocation for dynamic length character
+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 initializer(omp_priv='a')
+end subroutine test_dynamic_length
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..daa0d41063858
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90
@@ -0,0 +1,33 @@
+! 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 (constant length)
+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: 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
More information about the flang-commits
mailing list