[flang-commits] [flang] [Flang][Lower] NFC: Replace SmallVector with more suitable alternatives (PR #85227)
Sergio Afonso via flang-commits
flang-commits at lists.llvm.org
Fri Mar 15 06:28:08 PDT 2024
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/85227
>From 5da48c07078b236244282bc2711f19418f0b507c Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Thu, 14 Mar 2024 14:31:48 +0000
Subject: [PATCH 1/2] [Flang][Lower] NFC: Replace SmallVector with more
suitable alternatives
In this patch some uses of `llvm::SmallVector` in Flang's lowering to MLIR are
replaced by other types (i.e. `llvm::ArrayRef` and `llvm::SmallVectorImpl`)
which are intended for these uses. This generally prevents relying on always
passing small vectors with a particular number of elements in the stack.
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 6 +--
flang/lib/Lower/OpenMP/OpenMP.cpp | 59 +++++++++++-----------
flang/lib/Lower/OpenMP/Utils.h | 4 +-
3 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 6fefd04f409b9f..14276b0f10fdea 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -577,7 +577,7 @@ class TypeInfo {
}
// Returns the shape of array types.
- const llvm::SmallVector<int64_t> &getShape() const { return shape; }
+ llvm::ArrayRef<int64_t> getShape() const { return shape; }
// Is the type inside a box?
bool isBox() const { return inBox; }
@@ -788,8 +788,8 @@ bool ClauseProcessor::processLink(
mlir::omp::MapInfoOp
createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value baseAddr, mlir::Value varPtrPtr, std::string name,
- mlir::SmallVector<mlir::Value> bounds,
- mlir::SmallVector<mlir::Value> members, uint64_t mapType,
+ mlir::ArrayRef<mlir::Value> bounds,
+ mlir::ArrayRef<mlir::Value> members, uint64_t mapType,
mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type retTy,
bool isVal) {
if (auto boxTy = baseAddr.getType().dyn_cast<fir::BaseBoxType>()) {
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 5d4db06ddafa93..31c48837305db3 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -287,9 +287,9 @@ struct OpWithBodyGenInfo {
return *this;
}
- OpWithBodyGenInfo &
- setReductions(llvm::SmallVector<const Fortran::semantics::Symbol *> *value1,
- llvm::SmallVector<mlir::Type> *value2) {
+ OpWithBodyGenInfo &setReductions(
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *value1,
+ llvm::SmallVectorImpl<mlir::Type> *value2) {
reductionSymbols = value1;
reductionTypes = value2;
return *this;
@@ -317,10 +317,10 @@ struct OpWithBodyGenInfo {
/// [in] if provided, processes the construct's data-sharing attributes.
DataSharingProcessor *dsp = nullptr;
/// [in] if provided, list of reduction symbols
- llvm::SmallVector<const Fortran::semantics::Symbol *> *reductionSymbols =
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *reductionSymbols =
nullptr;
/// [in] if provided, list of reduction types
- llvm::SmallVector<mlir::Type> *reductionTypes = nullptr;
+ llvm::SmallVectorImpl<mlir::Type> *reductionTypes = nullptr;
/// [in] if provided, emits the op's region entry. Otherwise, an emtpy block
/// is created in the region.
GenOMPRegionEntryCBFn genRegionEntryCB = nullptr;
@@ -465,11 +465,9 @@ static void genBodyOfTargetDataOp(
Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval, bool genNested,
- mlir::omp::DataOp &dataOp,
- const llvm::SmallVector<mlir::Type> &useDeviceTypes,
- const llvm::SmallVector<mlir::Location> &useDeviceLocs,
- const llvm::SmallVector<const Fortran::semantics::Symbol *>
- &useDeviceSymbols,
+ mlir::omp::DataOp &dataOp, llvm::ArrayRef<mlir::Type> useDeviceTypes,
+ llvm::ArrayRef<mlir::Location> useDeviceLocs,
+ llvm::ArrayRef<const Fortran::semantics::Symbol *> useDeviceSymbols,
const mlir::Location ¤tLocation) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
mlir::Region ®ion = dataOp.getRegion();
@@ -814,11 +812,12 @@ genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
// clause. Support for such list items in a use_device_ptr clause
// is deprecated."
static void promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr(
- llvm::SmallVector<mlir::Value> &devicePtrOperands,
- llvm::SmallVector<mlir::Value> &deviceAddrOperands,
- llvm::SmallVector<mlir::Type> &useDeviceTypes,
- llvm::SmallVector<mlir::Location> &useDeviceLocs,
- llvm::SmallVector<const Fortran::semantics::Symbol *> &useDeviceSymbols) {
+ llvm::SmallVectorImpl<mlir::Value> &devicePtrOperands,
+ llvm::SmallVectorImpl<mlir::Value> &deviceAddrOperands,
+ llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
+ llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
+ &useDeviceSymbols) {
auto moveElementToBack = [](size_t idx, auto &vector) {
auto *iter = std::next(vector.begin(), idx);
vector.push_back(*iter);
@@ -951,15 +950,15 @@ genEnterExitUpdateDataOp(Fortran::lower::AbstractConverter &converter,
// This functions creates a block for the body of the targetOp's region. It adds
// all the symbols present in mapSymbols as block arguments to this block.
-static void genBodyOfTargetOp(
- Fortran::lower::AbstractConverter &converter,
- Fortran::semantics::SemanticsContext &semaCtx,
- Fortran::lower::pft::Evaluation &eval, bool genNested,
- mlir::omp::TargetOp &targetOp,
- const llvm::SmallVector<mlir::Type> &mapSymTypes,
- const llvm::SmallVector<mlir::Location> &mapSymLocs,
- const llvm::SmallVector<const Fortran::semantics::Symbol *> &mapSymbols,
- const mlir::Location ¤tLocation) {
+static void
+genBodyOfTargetOp(Fortran::lower::AbstractConverter &converter,
+ Fortran::semantics::SemanticsContext &semaCtx,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
+ mlir::omp::TargetOp &targetOp,
+ llvm::ArrayRef<mlir::Type> mapSymTypes,
+ llvm::ArrayRef<mlir::Location> mapSymLocs,
+ llvm::ArrayRef<const Fortran::semantics::Symbol *> mapSymbols,
+ const mlir::Location ¤tLocation) {
assert(mapSymTypes.size() == mapSymLocs.size());
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
@@ -1491,7 +1490,7 @@ static void convertLoopBounds(Fortran::lower::AbstractConverter &converter,
static llvm::SmallVector<const Fortran::semantics::Symbol *>
genLoopVars(mlir::Operation *op, Fortran::lower::AbstractConverter &converter,
mlir::Location &loc,
- const llvm::SmallVector<const Fortran::semantics::Symbol *> &args) {
+ llvm::ArrayRef<const Fortran::semantics::Symbol *> args) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
auto ®ion = op->getRegion(0);
@@ -1512,16 +1511,16 @@ genLoopVars(mlir::Operation *op, Fortran::lower::AbstractConverter &converter,
}
firOpBuilder.setInsertionPointAfter(storeOp);
- return args;
+ return llvm::SmallVector<const Fortran::semantics::Symbol *>(args);
}
static llvm::SmallVector<const Fortran::semantics::Symbol *>
genLoopAndReductionVars(
mlir::Operation *op, Fortran::lower::AbstractConverter &converter,
mlir::Location &loc,
- const llvm::SmallVector<const Fortran::semantics::Symbol *> &loopArgs,
- const llvm::SmallVector<const Fortran::semantics::Symbol *> &reductionArgs,
- llvm::SmallVector<mlir::Type> &reductionTypes) {
+ llvm::ArrayRef<const Fortran::semantics::Symbol *> loopArgs,
+ llvm::ArrayRef<const Fortran::semantics::Symbol *> reductionArgs,
+ llvm::SmallVectorImpl<mlir::Type> &reductionTypes) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
llvm::SmallVector<mlir::Type> blockArgTypes;
@@ -1564,7 +1563,7 @@ genLoopAndReductionVars(
converter.bindSymbol(*arg, prv);
}
- return loopArgs;
+ return llvm::SmallVector<const Fortran::semantics::Symbol *>(loopArgs);
}
static void
diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h
index 176ab2b5238a43..3ab0823a462148 100644
--- a/flang/lib/Lower/OpenMP/Utils.h
+++ b/flang/lib/Lower/OpenMP/Utils.h
@@ -46,8 +46,8 @@ using DeclareTargetCapturePair =
mlir::omp::MapInfoOp
createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value baseAddr, mlir::Value varPtrPtr, std::string name,
- mlir::SmallVector<mlir::Value> bounds,
- mlir::SmallVector<mlir::Value> members, uint64_t mapType,
+ mlir::ArrayRef<mlir::Value> bounds,
+ mlir::ArrayRef<mlir::Value> members, uint64_t mapType,
mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type retTy,
bool isVal = false);
>From 0eb81d2438cbfa4aac9bf724b41e09b1b59a3d17 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Fri, 15 Mar 2024 13:27:47 +0000
Subject: [PATCH 2/2] Address review comments
---
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 14276b0f10fdea..13347c8cf7b658 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -788,8 +788,8 @@ bool ClauseProcessor::processLink(
mlir::omp::MapInfoOp
createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value baseAddr, mlir::Value varPtrPtr, std::string name,
- mlir::ArrayRef<mlir::Value> bounds,
- mlir::ArrayRef<mlir::Value> members, uint64_t mapType,
+ llvm::ArrayRef<mlir::Value> bounds,
+ llvm::ArrayRef<mlir::Value> members, uint64_t mapType,
mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type retTy,
bool isVal) {
if (auto boxTy = baseAddr.getType().dyn_cast<fir::BaseBoxType>()) {
More information about the flang-commits
mailing list