Skip to content

[flang][fir] Basic PFT to MLIR lowering for do concurrent locality specifiers #138534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[flang][fir] Basic PFT to MLIR lowering for do concurrent locality specifiers #138534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b7bce43
[flang][fir] Basci lowering `fir.do_concurrent` locality specs to `fi…
ergawy May 5, 2025
a505cf5
add todos
ergawy May 7, 2025
5855225
More todos
ergawy May 9, 2025
b6893b6
[flang][fir] Basic PFT to MLIR lowering for do concurrent locality sp…
ergawy May 5, 2025
704322c
add todo
ergawy May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions flang/include/flang/Lower/AbstractConverter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ class AbstractConverter {
virtual Fortran::lower::SymbolBox
lookupOneLevelUpSymbol(const Fortran::semantics::Symbol &sym) = 0;

/// Find the symbol in the inner-most level of the local map or return null.
virtual Fortran::lower::SymbolBox
shallowLookupSymbol(const Fortran::semantics::Symbol &sym) = 0;

/// Return the mlir::SymbolTable associated to the ModuleOp.
/// Look-ups are faster using it than using module.lookup<>,
/// but the module op should be queried in case of failure
Expand Down
4 changes: 4 additions & 0 deletions flang/include/flang/Optimizer/Dialect/FIROps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ class CoordinateIndicesAdaptor {
mlir::ValueRange values;
};

struct LocalitySpecifierOperands {
llvm::SmallVector<::mlir::Value> privateVars;
llvm::SmallVector<::mlir::Attribute> privateSyms;
};
} // namespace fir

#endif // FORTRAN_OPTIMIZER_DIALECT_FIROPS_H
15 changes: 15 additions & 0 deletions flang/include/flang/Optimizer/Dialect/FIROps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3600,6 +3600,21 @@ def fir_LocalitySpecifierOp : fir_Op<"local", [IsolatedFromAbove]> {
];

let extraClassDeclaration = [{
mlir::BlockArgument getInitMoldArg() {
auto &region = getInitRegion();
return region.empty() ? nullptr : region.getArgument(0);
}
mlir::BlockArgument getInitPrivateArg() {
auto &region = getInitRegion();
return region.empty() ? nullptr : region.getArgument(1);
}

/// Returns true if the init region might read from the mold argument
bool initReadsFromMold() {
mlir::BlockArgument moldArg = getInitMoldArg();
return moldArg && !moldArg.use_empty();
}

/// Get the type for arguments to nested regions. This should
/// generally be either the same as getType() or some pointer
/// type (pointing to the type allocated by this op).
Expand Down
62 changes: 53 additions & 9 deletions flang/lib/Lower/Bridge.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#include "flang/Lower/Bridge.h"

#include "OpenMP/DataSharingProcessor.h"
#include "OpenMP/Utils.h"
#include "flang/Lower/Allocatable.h"
#include "flang/Lower/CallInterface.h"
#include "flang/Lower/Coarray.h"
Expand Down Expand Up @@ -1144,6 +1146,14 @@ class FirConverter : public Fortran::lower::AbstractConverter {
return name;
}

/// Find the symbol in the inner-most level of the local map or return null.
Fortran::lower::SymbolBox
shallowLookupSymbol(const Fortran::semantics::Symbol &sym) override {
if (Fortran::lower::SymbolBox v = localSymbols.shallowLookupSymbol(sym))
return v;
return {};
}

private:
FirConverter() = delete;
FirConverter(const FirConverter &) = delete;
Expand Down Expand Up @@ -1218,14 +1228,6 @@ class FirConverter : public Fortran::lower::AbstractConverter {
return {};
}

/// Find the symbol in the inner-most level of the local map or return null.
Fortran::lower::SymbolBox
shallowLookupSymbol(const Fortran::semantics::Symbol &sym) {
if (Fortran::lower::SymbolBox v = localSymbols.shallowLookupSymbol(sym))
return v;
return {};
}

/// Find the symbol in one level up of symbol map such as for host-association
/// in OpenMP code or return null.
Fortran::lower::SymbolBox
Expand Down Expand Up @@ -2029,9 +2031,33 @@ class FirConverter : public Fortran::lower::AbstractConverter {
void handleLocalitySpecs(const IncrementLoopInfo &info) {
Fortran::semantics::SemanticsContext &semanticsContext =
bridge.getSemanticsContext();
for (const Fortran::semantics::Symbol *sym : info.localSymList)
Fortran::lower::omp::DataSharingProcessor dsp(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reuse makes sense to me. The local/local_init regions should match omp's private/firstprivate regions. My hesitation here is that it uses OpenMP lowering to do something that is not omp specific. I would recommend that the utilities used to generate these regions be extracted from the OpenMP directory - and then omp::DataSharingProcessor can also call those same utilities.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 Can we extract this from OMP?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. Would it be ok to add a todo and do this is a separate PR? Just to keep the scope of the PR a bit more well defined?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to what I did here to generlize the CLI flag: #138816.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be ok to add a todo and do this is a separate PR?

Sure! Thank you!

*this, semanticsContext, getEval(),
/*useDelayedPrivatization=*/true, localSymbols);
fir::LocalitySpecifierOperands privateClauseOps;
auto doConcurrentLoopOp =
mlir::dyn_cast_if_present<fir::DoConcurrentLoopOp>(info.loopOp);
// TODO Promote to using `enableDelayedPrivatization` (which is enabled by
// default unlike the staging flag) once the implementation of this is more
// complete.
bool useDelayedPriv =
enableDelayedPrivatizationStaging && doConcurrentLoopOp;

for (const Fortran::semantics::Symbol *sym : info.localSymList) {
if (useDelayedPriv) {
dsp.privatizeSymbol<fir::LocalitySpecifierOp>(sym, &privateClauseOps);
continue;
}

createHostAssociateVarClone(*sym, /*skipDefaultInit=*/false);
}

for (const Fortran::semantics::Symbol *sym : info.localInitSymList) {
if (useDelayedPriv) {
dsp.privatizeSymbol<fir::LocalitySpecifierOp>(sym, &privateClauseOps);
continue;
}

createHostAssociateVarClone(*sym, /*skipDefaultInit=*/true);
const auto *hostDetails =
sym->detailsIf<Fortran::semantics::HostAssocDetails>();
Expand All @@ -2050,6 +2076,24 @@ class FirConverter : public Fortran::lower::AbstractConverter {
sym->detailsIf<Fortran::semantics::HostAssocDetails>();
copySymbolBinding(hostDetails->symbol(), *sym);
}

if (useDelayedPriv) {
doConcurrentLoopOp.getLocalVarsMutable().assign(
privateClauseOps.privateVars);
doConcurrentLoopOp.setLocalSymsAttr(
builder->getArrayAttr(privateClauseOps.privateSyms));

for (auto [sym, privateVar] : llvm::zip_equal(
dsp.getAllSymbolsToPrivatize(), privateClauseOps.privateVars)) {
auto arg = doConcurrentLoopOp.getRegion().begin()->addArgument(
privateVar.getType(), doConcurrentLoopOp.getLoc());
bindSymbol(*sym, hlfir::translateToExtendedValue(
privateVar.getLoc(), *builder, hlfir::Entity{arg},
/*contiguousHint=*/true)
.first);
}
}

// Note that allocatable, types with ultimate components, and type
// requiring finalization are forbidden in LOCAL/LOCAL_INIT (F2023 C1130),
// so no clean-up needs to be generated for these entities.
Expand Down
104 changes: 77 additions & 27 deletions flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "flang/Optimizer/Builder/BoxValue.h"
#include "flang/Optimizer/Builder/HLFIRTools.h"
#include "flang/Optimizer/Builder/Todo.h"
#include "flang/Optimizer/Dialect/FIROps.h"
#include "flang/Optimizer/HLFIR/HLFIRDialect.h"
#include "flang/Optimizer/HLFIR/HLFIROps.h"
#include "flang/Semantics/attr.h"
Expand Down Expand Up @@ -53,6 +54,15 @@ DataSharingProcessor::DataSharingProcessor(
});
}

DataSharingProcessor::DataSharingProcessor(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
bool useDelayedPrivatization,
lower::SymMap &symTable)
: DataSharingProcessor(converter, semaCtx, {}, eval,
/*shouldCollectPreDeterminedSymols=*/false,
useDelayedPrivatization, symTable) {}

void DataSharingProcessor::processStep1(
mlir::omp::PrivateClauseOps *clauseOps) {
collectSymbolsForPrivatization();
Expand Down Expand Up @@ -174,7 +184,8 @@ void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) {

void DataSharingProcessor::copyFirstPrivateSymbol(
const semantics::Symbol *sym, mlir::OpBuilder::InsertPoint *copyAssignIP) {
if (sym->test(semantics::Symbol::Flag::OmpFirstPrivate))
if (sym->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
sym->test(semantics::Symbol::Flag::LocalityLocalInit))
converter.copyHostAssociateVar(*sym, copyAssignIP);
}

Expand Down Expand Up @@ -487,9 +498,9 @@ void DataSharingProcessor::privatize(mlir::omp::PrivateClauseOps *clauseOps) {
if (const auto *commonDet =
sym->detailsIf<semantics::CommonBlockDetails>()) {
for (const auto &mem : commonDet->objects())
doPrivatize(&*mem, clauseOps);
privatizeSymbol(&*mem, clauseOps);
} else
doPrivatize(sym, clauseOps);
privatizeSymbol(sym, clauseOps);
}
}

Expand All @@ -506,22 +517,30 @@ void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) {
}
}

void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
mlir::omp::PrivateClauseOps *clauseOps) {
template <typename OpType, typename OperandsStructType>
void DataSharingProcessor::privatizeSymbol(
const semantics::Symbol *symToPrivatize, OperandsStructType *clauseOps) {
if (!useDelayedPrivatization) {
cloneSymbol(sym);
copyFirstPrivateSymbol(sym);
cloneSymbol(symToPrivatize);
copyFirstPrivateSymbol(symToPrivatize);
return;
}

lower::SymbolBox hsb = converter.lookupOneLevelUpSymbol(*sym);
const semantics::Symbol *sym = symToPrivatize->HasLocalLocality()
? &symToPrivatize->GetUltimate()
: symToPrivatize;
lower::SymbolBox hsb = symToPrivatize->HasLocalLocality()
? converter.shallowLookupSymbol(*sym)
: converter.lookupOneLevelUpSymbol(*sym);
assert(hsb && "Host symbol box not found");
hlfir::Entity entity{hsb.getAddr()};
bool cannotHaveNonDefaultLowerBounds = !entity.mayHaveNonDefaultLowerBounds();

mlir::Location symLoc = hsb.getAddr().getLoc();
std::string privatizerName = sym->name().ToString() + ".privatizer";
bool isFirstPrivate = sym->test(semantics::Symbol::Flag::OmpFirstPrivate);
bool isFirstPrivate =
symToPrivatize->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
symToPrivatize->test(semantics::Symbol::Flag::LocalityLocalInit);

mlir::Value privVal = hsb.getAddr();
mlir::Type allocType = privVal.getType();
Expand Down Expand Up @@ -555,24 +574,33 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,

mlir::Type argType = privVal.getType();

mlir::omp::PrivateClauseOp privatizerOp = [&]() {
OpType privatizerOp = [&]() {
auto moduleOp = firOpBuilder.getModule();
auto uniquePrivatizerName = fir::getTypeAsString(
allocType, converter.getKindMap(),
converter.mangleName(*sym) +
(isFirstPrivate ? "_firstprivate" : "_private"));

if (auto existingPrivatizer =
moduleOp.lookupSymbol<mlir::omp::PrivateClauseOp>(
uniquePrivatizerName))
moduleOp.lookupSymbol<OpType>(uniquePrivatizerName))
return existingPrivatizer;

mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
firOpBuilder.setInsertionPointToStart(moduleOp.getBody());
auto result = firOpBuilder.create<mlir::omp::PrivateClauseOp>(
symLoc, uniquePrivatizerName, allocType,
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
: mlir::omp::DataSharingClauseType::Private);
OpType result;

if constexpr (std::is_same_v<OpType, mlir::omp::PrivateClauseOp>) {
result = firOpBuilder.create<OpType>(
symLoc, uniquePrivatizerName, allocType,
isFirstPrivate ? mlir::omp::DataSharingClauseType::FirstPrivate
: mlir::omp::DataSharingClauseType::Private);
} else {
result = firOpBuilder.create<OpType>(
symLoc, uniquePrivatizerName, allocType,
isFirstPrivate ? fir::LocalitySpecifierType::LocalInit
: fir::LocalitySpecifierType::Local);
}

fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
lower::SymMapScope outerScope(symTable);

Expand Down Expand Up @@ -615,27 +643,36 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
&copyRegion, /*insertPt=*/{}, {argType, argType}, {symLoc, symLoc});
firOpBuilder.setInsertionPointToEnd(copyEntryBlock);

auto addSymbol = [&](unsigned argIdx, bool force = false) {
auto addSymbol = [&](unsigned argIdx, const semantics::Symbol *symToMap,
bool force = false) {
symExV.match(
[&](const fir::MutableBoxValue &box) {
symTable.addSymbol(
*sym, fir::substBase(box, copyRegion.getArgument(argIdx)),
force);
*symToMap,
fir::substBase(box, copyRegion.getArgument(argIdx)), force);
},
[&](const auto &box) {
symTable.addSymbol(*sym, copyRegion.getArgument(argIdx), force);
symTable.addSymbol(*symToMap, copyRegion.getArgument(argIdx),
force);
});
};

addSymbol(0, true);
addSymbol(0, sym, true);
lower::SymMapScope innerScope(symTable);
addSymbol(1);
addSymbol(1, symToPrivatize);

auto ip = firOpBuilder.saveInsertionPoint();
copyFirstPrivateSymbol(sym, &ip);

firOpBuilder.create<mlir::omp::YieldOp>(
hsb.getAddr().getLoc(), symTable.shallowLookupSymbol(*sym).getAddr());
copyFirstPrivateSymbol(symToPrivatize, &ip);

if constexpr (std::is_same_v<OpType, mlir::omp::PrivateClauseOp>) {
firOpBuilder.create<mlir::omp::YieldOp>(
hsb.getAddr().getLoc(),
symTable.shallowLookupSymbol(*symToPrivatize).getAddr());
} else {
firOpBuilder.create<fir::YieldOp>(
hsb.getAddr().getLoc(),
symTable.shallowLookupSymbol(*symToPrivatize).getAddr());
}
}

return result;
Expand All @@ -646,9 +683,22 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
clauseOps->privateVars.push_back(privVal);
}

symToPrivatizer[sym] = privatizerOp;
if (symToPrivatize->HasLocalLocality())
allPrivatizedSymbols.insert(symToPrivatize);
}

template void
DataSharingProcessor::privatizeSymbol<mlir::omp::PrivateClauseOp,
mlir::omp::PrivateClauseOps>(
const semantics::Symbol *symToPrivatize,
mlir::omp::PrivateClauseOps *clauseOps);

template void
DataSharingProcessor::privatizeSymbol<fir::LocalitySpecifierOp,
fir::LocalitySpecifierOperands>(
const semantics::Symbol *symToPrivatize,
fir::LocalitySpecifierOperands *clauseOps);

} // namespace omp
} // namespace lower
} // namespace Fortran
14 changes: 10 additions & 4 deletions flang/lib/Lower/OpenMP/DataSharingProcessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ class DataSharingProcessor {
llvm::SetVector<const semantics::Symbol *> preDeterminedSymbols;
llvm::SetVector<const semantics::Symbol *> allPrivatizedSymbols;

llvm::DenseMap<const semantics::Symbol *, mlir::omp::PrivateClauseOp>
symToPrivatizer;
lower::AbstractConverter &converter;
semantics::SemanticsContext &semaCtx;
fir::FirOpBuilder &firOpBuilder;
Expand All @@ -105,8 +103,6 @@ class DataSharingProcessor {
void collectImplicitSymbols();
void collectPreDeterminedSymbols();
void privatize(mlir::omp::PrivateClauseOps *clauseOps);
void doPrivatize(const semantics::Symbol *sym,
mlir::omp::PrivateClauseOps *clauseOps);
void copyLastPrivatize(mlir::Operation *op);
void insertLastPrivateCompare(mlir::Operation *op);
void cloneSymbol(const semantics::Symbol *sym);
Expand All @@ -125,6 +121,11 @@ class DataSharingProcessor {
bool shouldCollectPreDeterminedSymbols,
bool useDelayedPrivatization, lower::SymMap &symTable);

DataSharingProcessor(lower::AbstractConverter &converter,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
bool useDelayedPrivatization, lower::SymMap &symTable);

// Privatisation is split into two steps.
// Step1 performs cloning of all privatisation clauses and copying for
// firstprivates. Step1 is performed at the place where process/processStep1
Expand All @@ -151,6 +152,11 @@ class DataSharingProcessor {
? allPrivatizedSymbols.getArrayRef()
: llvm::ArrayRef<const semantics::Symbol *>();
}

template <typename OpType = mlir::omp::PrivateClauseOp,
typename OperandsStructType = mlir::omp::PrivateClauseOps>
void privatizeSymbol(const semantics::Symbol *symToPrivatize,
OperandsStructType *clauseOps);
};

} // namespace omp
Expand Down
Loading