shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[Flang][OpenMP] Fix crash with character types in declare_reduction (#178038)

Browse Source 
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"
/>
This commit is contained in:
Krish Gupta 2026-02-10 17:01:24 +05:30 committed by GitHub
parent 4280f0d241
commit aef8a2c483
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 121 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
flang/lib/Lower/OpenMP/ClauseProcessor.cpp
View File

@ -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);

19
flang/lib/Lower/OpenMP/OpenMP.cpp
View File

@ -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);

35
flang/lib/Lower/Support/PrivateReductionUtils.cpp
View File

@ -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;

5
flang/lib/Lower/Support/ReductionProcessor.cpp
View File

@ -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;

11
flang/test/Lower/OpenMP/Todo/reduction-character-dynamic-length.f90 Normal file
View File

@ -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

33
flang/test/Lower/OpenMP/declare-reduction-character-allocatable.f90 Normal file
View File

@ -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