[Flang][OpenMP] Refactor loop-related lowering for composite support #97566
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skatrak
requested review from
ergawy,
Meinersbur,
jsjodin,
tblah,
kiranchandramohan,
abidh,
mjklemm,
raghavendhra,
agozillon,
DominikAdamski,
TIFitis,
kparzysz,
anchuraj and
bhandarkar-pranav
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: Sergio Afonso (skatrak) ChangesThis patch splits the lowering for Lowering functions for loop constructs are split into "wrapper" and "standalone" variants, where the "wrapper" version only creates the specific operation with nothing inside of it and the "standalone" version calls the former and also handles clause processing and creates the nested "Wrapper" lowering functions can be used by "composite" lowering functions in follow-up patches, minimizing code duplication. Tests broken as a result of reordering between the processing of the loop wrapper's and the nested Patch is 40.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97566.diff 8 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 11b8b957330c8..1830e31349cfb 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -518,8 +518,8 @@ struct OpWithBodyGenInfo {
}
OpWithBodyGenInfo &
- setReductions(llvm::SmallVectorImpl<const semantics::Symbol *> *value1,
- llvm::SmallVectorImpl<mlir::Type> *value2) {
+ setReductions(llvm::ArrayRef<const semantics::Symbol *> *value1,
+ llvm::ArrayRef<mlir::Type> *value2) {
reductionSymbols = value1;
reductionTypes = value2;
return *this;
@@ -549,9 +549,9 @@ struct OpWithBodyGenInfo {
/// [in] if provided, processes the construct's data-sharing attributes.
DataSharingProcessor *dsp = nullptr;
/// [in] if provided, list of reduction symbols
- llvm::SmallVectorImpl<const semantics::Symbol *> *reductionSymbols = nullptr;
+ llvm::ArrayRef<const semantics::Symbol *> *reductionSymbols = nullptr;
/// [in] if provided, list of reduction types
- llvm::SmallVectorImpl<mlir::Type> *reductionTypes = nullptr;
+ llvm::ArrayRef<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;
@@ -1336,53 +1336,6 @@ genCriticalOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
queue, item, nameAttr);
}
-static mlir::omp::DistributeOp
-genDistributeOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
- DataSharingProcessor &dsp) {
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
-
- lower::StatementContext stmtCtx;
- mlir::omp::LoopNestClauseOps loopClauseOps;
- mlir::omp::DistributeClauseOps distributeClauseOps;
- llvm::SmallVector<const semantics::Symbol *> iv;
- genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
- loopClauseOps, iv);
- genDistributeClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
- distributeClauseOps);
-
- // Create omp.distribute wrapper.
- auto distributeOp =
- firOpBuilder.create<mlir::omp::DistributeOp>(loc, distributeClauseOps);
-
- firOpBuilder.createBlock(&distributeOp.getRegion());
- firOpBuilder.setInsertionPoint(
- lower::genOpenMPTerminator(firOpBuilder, distributeOp, loc));
-
- // Create nested omp.loop_nest and fill body with loop contents.
- auto loopOp = firOpBuilder.create<mlir::omp::LoopNestOp>(loc, loopClauseOps);
-
- auto *nestedEval =
- getCollapsedLoopEval(eval, getCollapseValue(item->clauses));
-
- auto ivCallback = [&](mlir::Operation *op) {
- genLoopVars(op, converter, loc, iv);
- return iv;
- };
-
- createBodyOfOp(*loopOp,
- OpWithBodyGenInfo(converter, symTable, semaCtx, loc,
- *nestedEval, llvm::omp::Directive::OMPD_simd)
- .setClauses(&item->clauses)
- .setDataSharingProcessor(&dsp)
- .setGenRegionEntryCb(ivCallback),
- queue, item);
-
- return distributeOp;
-}
-
static mlir::omp::FlushOp
genFlushOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
@@ -1396,6 +1349,33 @@ genFlushOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
converter.getCurrentLocation(), operandRange);
}
+static mlir::omp::LoopNestOp
+genLoopNestOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue, ConstructQueue::iterator item,
+ mlir::omp::LoopNestClauseOps &clauseOps,
+ llvm::ArrayRef<const semantics::Symbol *> iv,
+ llvm::ArrayRef<const semantics::Symbol *> wrapperSyms,
+ llvm::ArrayRef<mlir::BlockArgument> wrapperArgs,
+ llvm::omp::Directive directive, DataSharingProcessor &dsp) {
+ auto ivCallback = [&](mlir::Operation *op) {
+ genLoopVars(op, converter, loc, iv, wrapperSyms, wrapperArgs);
+ return llvm::SmallVector<const semantics::Symbol *>(iv);
+ };
+
+ auto *nestedEval =
+ getCollapsedLoopEval(eval, getCollapseValue(item->clauses));
+
+ return genOpWithBody<mlir::omp::LoopNestOp>(
+ OpWithBodyGenInfo(converter, symTable, semaCtx, loc, *nestedEval,
+ directive)
+ .setClauses(&item->clauses)
+ .setDataSharingProcessor(&dsp)
+ .setGenRegionEntryCb(ivCallback),
+ queue, item, clauseOps);
+}
+
static mlir::omp::MasterOp
genMasterOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
@@ -1430,24 +1410,18 @@ genOrderedRegionOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
queue, item, clauseOps);
}
-static mlir::omp::ParallelOp
-genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
- bool outerCombined = false) {
+static mlir::omp::ParallelOp genParallelOp(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+ mlir::Location loc, const ConstructQueue &queue,
+ ConstructQueue::iterator item, mlir::omp::ParallelClauseOps &clauseOps,
+ llvm::ArrayRef<const semantics::Symbol *> reductionSyms,
+ llvm::ArrayRef<mlir::Type> reductionTypes, bool outerCombined = false) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- lower::StatementContext stmtCtx;
- mlir::omp::ParallelClauseOps clauseOps;
- llvm::SmallVector<mlir::Type> reductionTypes;
- llvm::SmallVector<const semantics::Symbol *> reductionSyms;
- genParallelClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
- /*processReduction=*/!outerCombined, clauseOps,
- reductionTypes, reductionSyms);
auto reductionCallback = [&](mlir::Operation *op) {
genReductionVars(op, converter, loc, reductionSyms, reductionTypes);
- return reductionSyms;
+ return llvm::SmallVector<const semantics::Symbol *>(reductionSyms);
};
OpWithBodyGenInfo genInfo =
@@ -1477,7 +1451,7 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
clauseOps.reductionVars.size(), loc);
llvm::SmallVector<mlir::Type> allRegionArgTypes;
- mergePrivateVarsInfo(parallelOp, llvm::ArrayRef(reductionTypes),
+ mergePrivateVarsInfo(parallelOp, reductionTypes,
llvm::function_ref<mlir::Type(mlir::Value)>{
[](mlir::Value v) { return v.getType(); }},
allRegionArgTypes);
@@ -1492,7 +1466,7 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
firOpBuilder.createBlock(®ion, /*insertPt=*/{}, allRegionArgTypes,
allRegionArgLocs);
- llvm::SmallVector<const semantics::Symbol *> allSymbols = reductionSyms;
+ llvm::SmallVector<const semantics::Symbol *> allSymbols(reductionSyms);
allSymbols.append(dsp.getAllSymbolsToPrivatize().begin(),
dsp.getAllSymbolsToPrivatize().end());
@@ -1603,51 +1577,6 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
return sectionsOp;
}
-static mlir::omp::SimdOp
-genSimdOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, DataSharingProcessor &dsp) {
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
-
- lower::StatementContext stmtCtx;
- mlir::omp::LoopNestClauseOps loopClauseOps;
- mlir::omp::SimdClauseOps simdClauseOps;
- llvm::SmallVector<const semantics::Symbol *> iv;
- genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
- loopClauseOps, iv);
- genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
-
- // Create omp.simd wrapper.
- auto simdOp = firOpBuilder.create<mlir::omp::SimdOp>(loc, simdClauseOps);
-
- // TODO: Add reduction-related arguments to the wrapper's entry block.
- firOpBuilder.createBlock(&simdOp.getRegion());
- firOpBuilder.setInsertionPoint(
- lower::genOpenMPTerminator(firOpBuilder, simdOp, loc));
-
- // Create nested omp.loop_nest and fill body with loop contents.
- auto loopOp = firOpBuilder.create<mlir::omp::LoopNestOp>(loc, loopClauseOps);
-
- auto *nestedEval =
- getCollapsedLoopEval(eval, getCollapseValue(item->clauses));
-
- auto ivCallback = [&](mlir::Operation *op) {
- genLoopVars(op, converter, loc, iv);
- return iv;
- };
-
- createBodyOfOp(*loopOp,
- OpWithBodyGenInfo(converter, symTable, semaCtx, loc,
- *nestedEval, llvm::omp::Directive::OMPD_simd)
- .setClauses(&item->clauses)
- .setDataSharingProcessor(&dsp)
- .setGenRegionEntryCb(ivCallback),
- queue, item);
-
- return simdOp;
-}
-
static mlir::omp::SingleOp
genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
@@ -1879,15 +1808,6 @@ genTaskgroupOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
queue, item, clauseOps);
}
-static mlir::omp::TaskloopOp
-genTaskloopOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx,
- lower::pft::Evaluation &eval, mlir::Location loc,
- const ConstructQueue &queue, ConstructQueue::iterator item,
- DataSharingProcessor &dsp) {
- TODO(loc, "Taskloop construct");
-}
-
static mlir::omp::TaskwaitOp
genTaskwaitOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
@@ -1924,55 +1844,185 @@ genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
queue, item, clauseOps);
}
+//===----------------------------------------------------------------------===//
+// Code generation functions for loop wrappers
+//===----------------------------------------------------------------------===//
+
+static mlir::omp::DistributeOp
+genDistributeWrapperOp(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const mlir::omp::DistributeClauseOps &clauseOps) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+
+ // Create omp.distribute wrapper.
+ auto distributeOp =
+ firOpBuilder.create<mlir::omp::DistributeOp>(loc, clauseOps);
+
+ // TODO: Populate entry block arguments with private variables.
+ firOpBuilder.createBlock(&distributeOp.getRegion());
+ firOpBuilder.setInsertionPoint(
+ lower::genOpenMPTerminator(firOpBuilder, distributeOp, loc));
+
+ return distributeOp;
+}
+
+static mlir::omp::SimdOp
+genSimdWrapperOp(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const mlir::omp::SimdClauseOps &clauseOps) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+
+ // Create omp.simd wrapper.
+ auto simdOp = firOpBuilder.create<mlir::omp::SimdOp>(loc, clauseOps);
+
+ // TODO: Populate entry block arguments with reduction and private variables.
+ firOpBuilder.createBlock(&simdOp.getRegion());
+ firOpBuilder.setInsertionPoint(
+ lower::genOpenMPTerminator(firOpBuilder, simdOp, loc));
+
+ return simdOp;
+}
+
+static mlir::omp::TaskloopOp
+genTaskloopWrapperOp(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const mlir::omp::TaskloopClauseOps &clauseOps) {
+ TODO(loc, "Taskloop construct");
+}
+
static mlir::omp::WsloopOp
-genWsloopOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
- semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
- mlir::Location loc, const ConstructQueue &queue,
- ConstructQueue::iterator item, DataSharingProcessor &dsp) {
+genWsloopWrapperOp(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const mlir::omp::WsloopClauseOps &clauseOps,
+ llvm::ArrayRef<const semantics::Symbol *> reductionSyms,
+ llvm::ArrayRef<mlir::Type> reductionTypes) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ // Create omp.wsloop wrapper.
+ llvm::SmallVector<mlir::Location> reductionLocs(reductionSyms.size(), loc);
+ auto wsloopOp = firOpBuilder.create<mlir::omp::WsloopOp>(loc, clauseOps);
+
+ // Populate entry block arguments with reduction variables.
+ // TODO: Add private variables to entry block arguments.
+ firOpBuilder.createBlock(&wsloopOp.getRegion(), {}, reductionTypes,
+ reductionLocs);
+ firOpBuilder.setInsertionPoint(
+ lower::genOpenMPTerminator(firOpBuilder, wsloopOp, loc));
+
+ return wsloopOp;
+}
+
+//===----------------------------------------------------------------------===//
+// Code generation functions for the standalone version of constructs that can
+// also be a leaf of a composite construct
+//===----------------------------------------------------------------------===//
+
+static void genStandaloneDistribute(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+ mlir::Location loc, const ConstructQueue &queue,
+ ConstructQueue::iterator item, DataSharingProcessor &dsp) {
lower::StatementContext stmtCtx;
- mlir::omp::LoopNestClauseOps loopClauseOps;
- mlir::omp::WsloopClauseOps wsClauseOps;
+
+ mlir::omp::DistributeClauseOps distributeClauseOps;
+ genDistributeClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
+ distributeClauseOps);
+
+ mlir::omp::LoopNestClauseOps loopNestClauseOps;
llvm::SmallVector<const semantics::Symbol *> iv;
- llvm::SmallVector<mlir::Type> reductionTypes;
+ genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
+ loopNestClauseOps, iv);
+
+ auto distributeOp = genDistributeWrapperOp(converter, semaCtx, eval, loc,
+ distributeClauseOps);
+
+ genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
+ loopNestClauseOps, iv,
+ /*wrapperSyms=*/{}, distributeOp.getRegion().getArguments(),
+ llvm::omp::Directive::OMPD_distribute, dsp);
+}
+
+static void genStandaloneDo(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::iterator item,
+ DataSharingProcessor &dsp) {
+ lower::StatementContext stmtCtx;
+
+ mlir::omp::WsloopClauseOps wsloopClauseOps;
llvm::SmallVector<const semantics::Symbol *> reductionSyms;
+ llvm::SmallVector<mlir::Type> reductionTypes;
+ genWsloopClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
+ wsloopClauseOps, reductionTypes, reductionSyms);
+
+ mlir::omp::LoopNestClauseOps loopNestClauseOps;
+ llvm::SmallVector<const semantics::Symbol *> iv;
genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
- loopClauseOps, iv);
- genWsloopClauses(converter, semaCtx, stmtCtx, item->clauses, loc, wsClauseOps,
- reductionTypes, reductionSyms);
+ loopNestClauseOps, iv);
- // Create omp.wsloop wrapper and populate entry block arguments with reduction
- // variables.
- auto wsloopOp = firOpBuilder.create<mlir::omp::WsloopOp>(loc, wsClauseOps);
- llvm::SmallVector<mlir::Location> reductionLocs(reductionSyms.size(), loc);
- mlir::Block *wsloopEntryBlock = firOpBuilder.createBlock(
- &wsloopOp.getRegion(), {}, reductionTypes, reductionLocs);
- firOpBuilder.setInsertionPoint(
- lower::genOpenMPTerminator(firOpBuilder, wsloopOp, loc));
+ auto wsloopOp =
+ genWsloopWrapperOp(converter, semaCtx, eval, loc, wsloopClauseOps,
+ reductionSyms, reductionTypes);
- // Create nested omp.loop_nest and fill body with loop contents.
- auto loopOp = firOpBuilder.create<mlir::omp::LoopNestOp>(loc, loopClauseOps);
+ genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
+ loopNestClauseOps, iv, reductionSyms,
+ wsloopOp.getRegion().getArguments(),
+ llvm::omp::Directive::OMPD_do, dsp);
+}
- auto *nestedEval =
- getCollapsedLoopEval(eval, getCollapseValue(item->clauses));
+static void genStandaloneParallel(
+ lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+ mlir::Location loc, const ConstructQueue &queue,
+ ConstructQueue::iterator item, bool outerCombined = false) {
+ lower::StatementContext stmtCtx;
- auto ivCallback = [&](mlir::Operation *op) {
- genLoopVars(op, converter, loc, iv, reductionSyms,
- wsloopEntryBlock->getArguments());
- return iv;
- };
+ mlir::omp::ParallelClauseOps clauseOps;
+ llvm::SmallVector<const semantics::Symbol *> reductionSyms;
+ llvm::SmallVector<mlir::Type> reductionTypes;
+ genParallelClauses(converter, semaCtx, stmtCtx, item->clauses, loc,
+ /*processReduction=*/!outerCombined, clauseOps,
+ reductionTypes, reductionSyms);
- createBodyOfOp(*loopOp,
- OpWithBodyGenInfo(converter, symTable, semaCtx, loc,
- *nestedEval, llvm::omp::Directive::OMPD_do)
- .setClauses(&item->clauses)
- .setDataSharingProcessor(&dsp)
- .setReductions(&reductionSyms, &reductionTypes)
- .setGenRegionEntryCb(ivCallback),
- queue, item);
+ genParallelOp(converter, symTable, semaCtx, eval, loc, queue, item, clauseOps,
+ reductionSyms, reductionTypes, outerCombined);
+}
- return wsloopOp;
+static void genStandaloneSimd(lower::AbstractConverter &converter,
+ lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval, mlir::Location loc,
+ const ConstructQueue &queue,
+ ConstructQueue::iterator item,
+ DataSharingProcessor &dsp) {
+ mlir::omp::SimdClauseOps simdClauseOps;
+ genSimdClauses(converter, semaCtx, item->clauses, loc, simdClauseOps);
+
+ mlir::omp::LoopNestClauseOps loopNestClauseOps;
+ llvm::SmallVector<const semantics::Symbol *> iv;
+ genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc,
+ loopNestClauseOps, iv);
+
+ auto simdOp = genSimdWrapperOp(converter, semaCtx, eval, loc, simdClauseOps);
+
+ genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item,
+ loopNestClauseOps, iv,
+ /*wrapperSyms=*/{}, simdOp.getRegion().getArguments(),...
[truncated]
|
tblah
reviewed
Jul 4, 2024
There was a problem hiding this comment.
This looks good overall. Do you expect there to be a lot more wrapper operations in the near future? If so, they look like there is a common pattern that could be further abstracted. Something like
template <class OP>
static OP
genWrapperOp(..., llvm::ArrayRef<mlir::type> extraBlockArgTypes) {
fir::FirOpBuilder &firOpBuilder = ...;
auto op = firOpBuilder.create<OP>(loc, clauseOps);
llvm::SmallVector<mlir::Location> blockArgLocs(extraBlockArgTypes.size(), loc);
firOpBuilder.createBlock(...)
firOpBuilder.setInsertionPoint(...)
return op;
}
skatrak
force-pushed
the
users/skatrak/composite-lower-04-loop-dsp
branch
2 times, most recently
from
c2a4e9c to
c8bf447
Compare
Base automatically changed from
users/skatrak/composite-lower-04-loop-dsp
to
main
July 5, 2024 09:38
skatrak
force-pushed
the
users/skatrak/composite-lower-05-refactor
branch
from
49a22cd to
2a6e9fd
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Thank you for the suggestion. I think if new loop wrappers are added at some point there should be rather few of them, but I think it's still worth doing the refactor, since those functions were pretty much identical. |
This patch splits the lowering for `omp.loop_nest` into its own function and updates lowering for all supported loop wrappers to stop creating this operation themselves. Lowering functions for loop constructs are split into "wrapper" and "standalone" variants, where the "wrapper" version only creates the specific operation with nothing inside of it and the "standalone" version calls the former and also handles clause processing and creates the nested `omp.loop_nest`. "Wrapper" lowering functions can be used by "composite" lowering functions in follow-up patches, minimizing code duplication. Tests broken as a result of reordering between the processing of the loop wrapper's and the nested `omp.loop_nest`'s clauses are also updated.
skatrak
force-pushed
the
users/skatrak/composite-lower-05-refactor
branch
from
922c034 to
938a35a
Compare
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
…lvm#97566) This patch splits the lowering for `omp.loop_nest` into its own function and updates lowering for all supported loop wrappers to stop creating this operation themselves. Lowering functions for loop constructs are split into "wrapper" and "standalone" variants, where the "wrapper" version only creates the specific operation with nothing inside of it and the "standalone" version calls the former and also handles clause processing and creates the nested `omp.loop_nest`. "Wrapper" lowering functions can be used by "composite" lowering functions in follow-up patches, minimizing code duplication. Tests broken as a result of reordering between the processing of the loop wrapper's and the nested `omp.loop_nest`'s clauses are also updated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch splits the lowering for
omp.loop_nestinto its own function and updates lowering for all supported loop wrappers to stop creating this operation themselves.Lowering functions for loop constructs are split into "wrapper" and "standalone" variants, where the "wrapper" version only creates the specific operation with nothing inside of it and the "standalone" version calls the former and also handles clause processing and creates the nested
omp.loop_nest."Wrapper" lowering functions can be used by "composite" lowering functions in follow-up patches, minimizing code duplication.
Tests broken as a result of reordering between the processing of the loop wrapper's and the nested
omp.loop_nest's clauses are also updated.