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

[flang][OpenMP] share global variable initialization code (#138672)

Browse Source 
Fixes #108136

In #108136 (the new testcase), flang was missing the length parameter
required for the variable length string when boxing the global variable.
The code that is initializing global variables for OpenMP did not
support types with length parameters.

Instead of duplicating this initialization logic in OpenMP, I decided to
use the exact same initialization as is used in the base language
because this will already be well tested and will be updated for any new
types. The difference for OpenMP is that the global variables will be
zero initialized instead of left undefined.

Previously `Fortran::lower::createGlobalInitialization` was used to
share a smaller amount of the logic with the base language lowering. I
think this bug has demonstrated that helper was too low level to be
helpful, and it was only used in OpenMP so I have made it static inside
of ConvertVariable.cpp.
This commit is contained in:
Tom Eccles 2025-05-07 10:18:13 +01:00 committed by GitHub
parent 18c5ad5c6c
commit 75e5643abf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 81 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
flang/docs/OpenMPSupport.md
View File

@ -64,4 +64,4 @@ Note : No distinction is made between the support in Parser/Semantics, MLIR, Low
| target teams distribute parallel loop simd construct | P | device, reduction, dist_schedule and linear clauses are not supported | | target teams distribute parallel loop simd construct | P | device, reduction, dist_schedule and linear clauses are not supported |
## OpenMP 3.1, OpenMP 2.5, OpenMP 1.1 ## OpenMP 3.1, OpenMP 2.5, OpenMP 1.1
All features except a few corner cases in atomic (complex type, different but compatible types in lhs and rhs), threadprivate (character type) constructs/clauses are supported. All features except a few corner cases in atomic (complex type, different but compatible types in lhs and rhs) are supported.

9
flang/include/flang/Lower/ConvertVariable.h
View File

@ -134,10 +134,11 @@ mlir::Value genInitialDataTarget(Fortran::lower::AbstractConverter &,
const SomeExpr &initialTarget, const SomeExpr &initialTarget,
bool couldBeInEquivalence = false); bool couldBeInEquivalence = false);
/// Call \p genInit to generate code inside \p global initializer region. /// Create the global op and its init if it has one
void createGlobalInitialization( fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
fir::FirOpBuilder &builder, fir::GlobalOp global, const Fortran::lower::pft::Variable &var,
std::function<void(fir::FirOpBuilder &)> genInit); llvm::StringRef globalName, mlir::StringAttr linkage,
cuf::DataAttributeAttr dataAttr = {});
/// Generate address \p addr inside an initializer. /// Generate address \p addr inside an initializer.
fir::ExtendedValue fir::ExtendedValue

97
flang/lib/Lower/ConvertVariable.cpp
View File

@ -145,11 +145,10 @@ static bool isConstant(const Fortran::semantics::Symbol &sym) {
sym.test(Fortran::semantics::Symbol::Flag::ReadOnly); sym.test(Fortran::semantics::Symbol::Flag::ReadOnly);
} }
static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter, /// Call \p genInit to generate code inside \p global initializer region.
const Fortran::lower::pft::Variable &var, static void
llvm::StringRef globalName, createGlobalInitialization(fir::FirOpBuilder &builder, fir::GlobalOp global,
mlir::StringAttr linkage, std::function<void(fir::FirOpBuilder &)> genInit);
cuf::DataAttributeAttr dataAttr = {});
static mlir::Location genLocation(Fortran::lower::AbstractConverter &converter, static mlir::Location genLocation(Fortran::lower::AbstractConverter &converter,
const Fortran::semantics::Symbol &sym) { const Fortran::semantics::Symbol &sym) {
@ -467,9 +466,9 @@ static bool globalIsInitialized(fir::GlobalOp global) {
} }
/// Call \p genInit to generate code inside \p global initializer region. /// Call \p genInit to generate code inside \p global initializer region.
void Fortran::lower::createGlobalInitialization( static void
fir::FirOpBuilder &builder, fir::GlobalOp global, createGlobalInitialization(fir::FirOpBuilder &builder, fir::GlobalOp global,
std::function<void(fir::FirOpBuilder &)> genInit) { std::function<void(fir::FirOpBuilder &)> genInit) {
mlir::Region &region = global.getRegion(); mlir::Region &region = global.getRegion();
region.push_back(new mlir::Block); region.push_back(new mlir::Block);
mlir::Block &block = region.back(); mlir::Block &block = region.back();
@ -479,7 +478,7 @@ void Fortran::lower::createGlobalInitialization(
builder.restoreInsertionPoint(insertPt); builder.restoreInsertionPoint(insertPt);
} }
static unsigned getAllocatorIdx(cuf::DataAttributeAttr dataAttr) { static unsigned getAllocatorIdxFromDataAttr(cuf::DataAttributeAttr dataAttr) {
if (dataAttr) { if (dataAttr) {
if (dataAttr.getValue() == cuf::DataAttribute::Pinned) if (dataAttr.getValue() == cuf::DataAttribute::Pinned)
return kPinnedAllocatorPos; return kPinnedAllocatorPos;
@ -494,11 +493,10 @@ static unsigned getAllocatorIdx(cuf::DataAttributeAttr dataAttr) {
} }
/// Create the global op and its init if it has one /// Create the global op and its init if it has one
static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter, fir::GlobalOp Fortran::lower::defineGlobal(
const Fortran::lower::pft::Variable &var, Fortran::lower::AbstractConverter &converter,
llvm::StringRef globalName, const Fortran::lower::pft::Variable &var, llvm::StringRef globalName,
mlir::StringAttr linkage, mlir::StringAttr linkage, cuf::DataAttributeAttr dataAttr) {
cuf::DataAttributeAttr dataAttr) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder(); fir::FirOpBuilder &builder = converter.getFirOpBuilder();
const Fortran::semantics::Symbol &sym = var.getSymbol(); const Fortran::semantics::Symbol &sym = var.getSymbol();
mlir::Location loc = genLocation(converter, sym); mlir::Location loc = genLocation(converter, sym);
@ -545,27 +543,25 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
sym.detailsIf<Fortran::semantics::ObjectEntityDetails>(); sym.detailsIf<Fortran::semantics::ObjectEntityDetails>();
if (details && details->init()) { if (details && details->init()) {
auto expr = *details->init(); auto expr = *details->init();
Fortran::lower::createGlobalInitialization( createGlobalInitialization(builder, global, [&](fir::FirOpBuilder &b) {
builder, global, [&](fir::FirOpBuilder &b) { mlir::Value box =
mlir::Value box = Fortran::lower::genInitialDataTarget( Fortran::lower::genInitialDataTarget(converter, loc, symTy, expr);
converter, loc, symTy, expr); b.create<fir::HasValueOp>(loc, box);
b.create<fir::HasValueOp>(loc, box); });
});
} else { } else {
// Create unallocated/disassociated descriptor if no explicit init // Create unallocated/disassociated descriptor if no explicit init
Fortran::lower::createGlobalInitialization( createGlobalInitialization(builder, global, [&](fir::FirOpBuilder &b) {
builder, global, [&](fir::FirOpBuilder &b) { mlir::Value box = fir::factory::createUnallocatedBox(
mlir::Value box = fir::factory::createUnallocatedBox( b, loc, symTy,
b, loc, symTy, /*nonDeferredParams=*/std::nullopt,
/*nonDeferredParams=*/std::nullopt, /*typeSourceBox=*/{}, getAllocatorIdxFromDataAttr(dataAttr));
/*typeSourceBox=*/{}, getAllocatorIdx(dataAttr)); b.create<fir::HasValueOp>(loc, box);
b.create<fir::HasValueOp>(loc, box); });
});
} }
} else if (const auto *details = } else if (const auto *details =
sym.detailsIf<Fortran::semantics::ObjectEntityDetails>()) { sym.detailsIf<Fortran::semantics::ObjectEntityDetails>()) {
if (details->init()) { if (details->init()) {
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) { builder, global, [&](fir::FirOpBuilder &builder) {
Fortran::lower::StatementContext stmtCtx( Fortran::lower::StatementContext stmtCtx(
/*cleanupProhibited=*/true); /*cleanupProhibited=*/true);
@ -576,7 +572,7 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
builder.create<fir::HasValueOp>(loc, castTo); builder.create<fir::HasValueOp>(loc, castTo);
}); });
} else if (Fortran::lower::hasDefaultInitialization(sym)) { } else if (Fortran::lower::hasDefaultInitialization(sym)) {
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) { builder, global, [&](fir::FirOpBuilder &builder) {
Fortran::lower::StatementContext stmtCtx( Fortran::lower::StatementContext stmtCtx(
/*cleanupProhibited=*/true); /*cleanupProhibited=*/true);
@ -591,7 +587,7 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
if (details && details->init()) { if (details && details->init()) {
auto sym{*details->init()}; auto sym{*details->init()};
if (sym) // Has a procedure target. if (sym) // Has a procedure target.
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &b) { builder, global, [&](fir::FirOpBuilder &b) {
Fortran::lower::StatementContext stmtCtx( Fortran::lower::StatementContext stmtCtx(
/*cleanupProhibited=*/true); /*cleanupProhibited=*/true);
@ -601,19 +597,17 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
b.create<fir::HasValueOp>(loc, castTo); b.create<fir::HasValueOp>(loc, castTo);
}); });
else { // Has NULL() target. else { // Has NULL() target.
Fortran::lower::createGlobalInitialization( createGlobalInitialization(builder, global, [&](fir::FirOpBuilder &b) {
builder, global, [&](fir::FirOpBuilder &b) { auto box{fir::factory::createNullBoxProc(b, loc, symTy)};
auto box{fir::factory::createNullBoxProc(b, loc, symTy)}; b.create<fir::HasValueOp>(loc, box);
b.create<fir::HasValueOp>(loc, box); });
});
} }
} else { } else {
// No initialization. // No initialization.
Fortran::lower::createGlobalInitialization( createGlobalInitialization(builder, global, [&](fir::FirOpBuilder &b) {
builder, global, [&](fir::FirOpBuilder &b) { auto box{fir::factory::createNullBoxProc(b, loc, symTy)};
auto box{fir::factory::createNullBoxProc(b, loc, symTy)}; b.create<fir::HasValueOp>(loc, box);
b.create<fir::HasValueOp>(loc, box); });
});
} }
} else if (sym.has<Fortran::semantics::CommonBlockDetails>()) { } else if (sym.has<Fortran::semantics::CommonBlockDetails>()) {
mlir::emitError(loc, "COMMON symbol processed elsewhere"); mlir::emitError(loc, "COMMON symbol processed elsewhere");
@ -634,7 +628,7 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
// file. // file.
if (sym.attrs().test(Fortran::semantics::Attr::BIND_C)) if (sym.attrs().test(Fortran::semantics::Attr::BIND_C))
global.setLinkName(builder.createCommonLinkage()); global.setLinkName(builder.createCommonLinkage());
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) { builder, global, [&](fir::FirOpBuilder &builder) {
mlir::Value initValue; mlir::Value initValue;
if (converter.getLoweringOptions().getInitGlobalZero()) if (converter.getLoweringOptions().getInitGlobalZero())
@ -826,7 +820,7 @@ void Fortran::lower::defaultInitializeAtRuntime(
/*isConst=*/true, /*isConst=*/true,
/*isTarget=*/false, /*isTarget=*/false,
/*dataAttr=*/{}); /*dataAttr=*/{});
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) { builder, global, [&](fir::FirOpBuilder &builder) {
Fortran::lower::StatementContext stmtCtx( Fortran::lower::StatementContext stmtCtx(
/*cleanupProhibited=*/true); /*cleanupProhibited=*/true);
@ -842,7 +836,7 @@ void Fortran::lower::defaultInitializeAtRuntime(
/*isConst=*/true, /*isConst=*/true,
/*isTarget=*/false, /*isTarget=*/false,
/*dataAttr=*/{}); /*dataAttr=*/{});
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) { builder, global, [&](fir::FirOpBuilder &builder) {
Fortran::lower::StatementContext stmtCtx( Fortran::lower::StatementContext stmtCtx(
/*cleanupProhibited=*/true); /*cleanupProhibited=*/true);
@ -1207,7 +1201,7 @@ static fir::GlobalOp defineGlobalAggregateStore(
if (const auto *objectDetails = if (const auto *objectDetails =
initSym->detailsIf<Fortran::semantics::ObjectEntityDetails>()) initSym->detailsIf<Fortran::semantics::ObjectEntityDetails>())
if (objectDetails->init()) { if (objectDetails->init()) {
Fortran::lower::createGlobalInitialization( createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) { builder, global, [&](fir::FirOpBuilder &builder) {
Fortran::lower::StatementContext stmtCtx; Fortran::lower::StatementContext stmtCtx;
mlir::Value initVal = fir::getBase(genInitializerExprValue( mlir::Value initVal = fir::getBase(genInitializerExprValue(
@ -1219,12 +1213,11 @@ static fir::GlobalOp defineGlobalAggregateStore(
// Equivalence has no Fortran initial value. Create an undefined FIR initial // Equivalence has no Fortran initial value. Create an undefined FIR initial
// value to ensure this is consider an object definition in the IR regardless // value to ensure this is consider an object definition in the IR regardless
// of the linkage. // of the linkage.
Fortran::lower::createGlobalInitialization( createGlobalInitialization(builder, global, [&](fir::FirOpBuilder &builder) {
builder, global, [&](fir::FirOpBuilder &builder) { Fortran::lower::StatementContext stmtCtx;
Fortran::lower::StatementContext stmtCtx; mlir::Value initVal = builder.create<fir::ZeroOp>(loc, aggTy);
mlir::Value initVal = builder.create<fir::ZeroOp>(loc, aggTy); builder.create<fir::HasValueOp>(loc, initVal);
builder.create<fir::HasValueOp>(loc, initVal); });
});
return global; return global;
} }
@ -1543,7 +1536,7 @@ static void finalizeCommonBlockDefinition(
LLVM_DEBUG(llvm::dbgs() << "}\n"); LLVM_DEBUG(llvm::dbgs() << "}\n");
builder.create<fir::HasValueOp>(loc, cb); builder.create<fir::HasValueOp>(loc, cb);
}; };
Fortran::lower::createGlobalInitialization(builder, global, initFunc); createGlobalInitialization(builder, global, initFunc);
} }
void Fortran::lower::defineCommonBlocks( void Fortran::lower::defineCommonBlocks(

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

@ -662,32 +662,9 @@ static fir::GlobalOp globalInitialization(lower::AbstractConverter &converter,
const semantics::Symbol &sym, const semantics::Symbol &sym,
const lower::pft::Variable &var, const lower::pft::Variable &var,
mlir::Location currentLocation) { mlir::Location currentLocation) {
mlir::Type ty = converter.genType(sym);
std::string globalName = converter.mangleName(sym); std::string globalName = converter.mangleName(sym);
mlir::StringAttr linkage = firOpBuilder.createInternalLinkage(); mlir::StringAttr linkage = firOpBuilder.createInternalLinkage();
fir::GlobalOp global = return Fortran::lower::defineGlobal(converter, var, globalName, linkage);
firOpBuilder.createGlobal(currentLocation, ty, globalName, linkage);
// Create default initialization for non-character scalar.
if (semantics::IsAllocatableOrObjectPointer(&sym)) {
mlir::Type baseAddrType = mlir::dyn_cast<fir::BoxType>(ty).getEleTy();
lower::createGlobalInitialization(
firOpBuilder, global, [&](fir::FirOpBuilder &b) {
mlir::Value nullAddr =
b.createNullConstant(currentLocation, baseAddrType);
mlir::Value box =
b.create<fir::EmboxOp>(currentLocation, ty, nullAddr);
b.create<fir::HasValueOp>(currentLocation, box);
});
} else {
lower::createGlobalInitialization(
firOpBuilder, global, [&](fir::FirOpBuilder &b) {
mlir::Value undef = b.create<fir::UndefOp>(currentLocation, ty);
b.create<fir::HasValueOp>(currentLocation, undef);
});
}
return global;
} }
// Get the extended value for \p val by extracting additional variable // Get the extended value for \p val by extracting additional variable

2
flang/test/Lower/OpenMP/omp-declare-target-program-var.f90
View File

@ -6,7 +6,7 @@ PROGRAM main
! HOST-DAG: %[[I_DECL:.*]]:2 = hlfir.declare %[[I_REF]] {uniq_name = "_QFEi"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>) ! HOST-DAG: %[[I_DECL:.*]]:2 = hlfir.declare %[[I_REF]] {uniq_name = "_QFEi"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
REAL :: I REAL :: I
! ALL-DAG: fir.global internal @_QFEi {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : f32 { ! ALL-DAG: fir.global internal @_QFEi {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : f32 {
! ALL-DAG: %[[UNDEF:.*]] = fir.undefined f32 ! ALL-DAG: %[[UNDEF:.*]] = fir.zero_bits f32
! ALL-DAG: fir.has_value %[[UNDEF]] : f32 ! ALL-DAG: fir.has_value %[[UNDEF]] : f32
! ALL-DAG: } ! ALL-DAG: }
!$omp declare target(I) !$omp declare target(I)

2
flang/test/Lower/OpenMP/threadprivate-host-association-2.f90
View File

@ -27,7 +27,7 @@
!CHECK: return !CHECK: return
!CHECK: } !CHECK: }
!CHECK: fir.global internal @_QFEa : i32 { !CHECK: fir.global internal @_QFEa : i32 {
!CHECK: %[[A:.*]] = fir.undefined i32 !CHECK: %[[A:.*]] = fir.zero_bits i32
!CHECK: fir.has_value %[[A]] : i32 !CHECK: fir.has_value %[[A]] : i32
!CHECK: } !CHECK: }

2
flang/test/Lower/OpenMP/threadprivate-host-association-3.f90
View File

@ -27,7 +27,7 @@
!CHECK: return !CHECK: return
!CHECK: } !CHECK: }
!CHECK: fir.global internal @_QFEa : i32 { !CHECK: fir.global internal @_QFEa : i32 {
!CHECK: %[[A:.*]] = fir.undefined i32 !CHECK: %[[A:.*]] = fir.zero_bits i32
!CHECK: fir.has_value %[[A]] : i32 !CHECK: fir.has_value %[[A]] : i32
!CHECK: } !CHECK: }

22
flang/test/Lower/OpenMP/threadprivate-lenparams.f90 Normal file
View File

@ -0,0 +1,22 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
! Regression test for https://github.com/llvm/llvm-project/issues/108136
character(:), pointer :: c
character(2), pointer :: c2
!$omp threadprivate(c, c2)
end
! CHECK-LABEL: fir.global internal @_QFEc : !fir.box<!fir.ptr<!fir.char<1,?>>> {
! CHECK: %[[VAL_0:.*]] = fir.zero_bits !fir.ptr<!fir.char<1,?>>
! CHECK: %[[VAL_1:.*]] = arith.constant 0 : index
! CHECK: %[[VAL_2:.*]] = fir.embox %[[VAL_0]] typeparams %[[VAL_1]] : (!fir.ptr<!fir.char<1,?>>, index) -> !fir.box<!fir.ptr<!fir.char<1,?>>>
! CHECK: fir.has_value %[[VAL_2]] : !fir.box<!fir.ptr<!fir.char<1,?>>>
! CHECK: }
! CHECK-LABEL: fir.global internal @_QFEc2 : !fir.box<!fir.ptr<!fir.char<1,2>>> {
! CHECK: %[[VAL_0:.*]] = fir.zero_bits !fir.ptr<!fir.char<1,2>>
! CHECK: %[[VAL_1:.*]] = fir.embox %[[VAL_0]] : (!fir.ptr<!fir.char<1,2>>) -> !fir.box<!fir.ptr<!fir.char<1,2>>>
! CHECK: fir.has_value %[[VAL_1]] : !fir.box<!fir.ptr<!fir.char<1,2>>>
! CHECK: }

8
flang/test/Lower/OpenMP/threadprivate-non-global.f90
View File

@ -85,19 +85,19 @@ program test
!CHECK-DAG: fir.has_value [[E1]] : !fir.box<!fir.heap<f32>> !CHECK-DAG: fir.has_value [[E1]] : !fir.box<!fir.heap<f32>>
!CHECK-DAG: } !CHECK-DAG: }
!CHECK-DAG: fir.global internal @_QFEw : complex<f32> { !CHECK-DAG: fir.global internal @_QFEw : complex<f32> {
!CHECK-DAG: [[Z2:%.*]] = fir.undefined complex<f32> !CHECK-DAG: [[Z2:%.*]] = fir.zero_bits complex<f32>
!CHECK-DAG: fir.has_value [[Z2]] : complex<f32> !CHECK-DAG: fir.has_value [[Z2]] : complex<f32>
!CHECK-DAG: } !CHECK-DAG: }
!CHECK-DAG: fir.global internal @_QFEx : i32 { !CHECK-DAG: fir.global internal @_QFEx : i32 {
!CHECK-DAG: [[Z3:%.*]] = fir.undefined i32 !CHECK-DAG: [[Z3:%.*]] = fir.zero_bits i32
!CHECK-DAG: fir.has_value [[Z3]] : i32 !CHECK-DAG: fir.has_value [[Z3]] : i32
!CHECK-DAG: } !CHECK-DAG: }
!CHECK-DAG: fir.global internal @_QFEy : f32 { !CHECK-DAG: fir.global internal @_QFEy : f32 {
!CHECK-DAG: [[Z4:%.*]] = fir.undefined f32 !CHECK-DAG: [[Z4:%.*]] = fir.zero_bits f32
!CHECK-DAG: fir.has_value [[Z4]] : f32 !CHECK-DAG: fir.has_value [[Z4]] : f32
!CHECK-DAG: } !CHECK-DAG: }
!CHECK-DAG: fir.global internal @_QFEz : !fir.logical<4> { !CHECK-DAG: fir.global internal @_QFEz : !fir.logical<4> {
!CHECK-DAG: [[Z5:%.*]] = fir.undefined !fir.logical<4> !CHECK-DAG: [[Z5:%.*]] = fir.zero_bits !fir.logical<4>
!CHECK-DAG: fir.has_value [[Z5]] : !fir.logical<4> !CHECK-DAG: fir.has_value [[Z5]] : !fir.logical<4>
!CHECK-DAG: } !CHECK-DAG: }
end end