https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/99506
>From 96ddba7f30440a83602e8f89d1193813a03dceee Mon Sep 17 00:00:00 2001 From: Sergio Afonso <safon...@amd.com> Date: Tue, 16 Jul 2024 15:48:00 +0100 Subject: [PATCH 1/2] [MLIR][OpenMP] Create `LoopRelatedClause` This patch introduces a new OpenMP clause definition not defined by the spec. Its main purpose is to define the `loop_inclusive` (previously "inclusive", renamed according to the parent of this PR in the stack) argument of `omp.loop_nest` in such a way that a followup implementation of a tablegen backend to automatically generate clause and operation operand structures directly from `OpenMP_Op` and `OpenMP_Clause` definitions can properly generate the `LoopNestOperands` structure. --- .../mlir/Dialect/OpenMP/OpenMPClauses.td | 19 +++++++++++++++++++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 6 ++---- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 4 ++-- .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 2 +- mlir/test/Dialect/OpenMP/ops.mlir | 2 +- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td index b6be8be63b328..24bd9b69e6cc1 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td @@ -564,6 +564,25 @@ class OpenMP_LinearClauseSkip< def OpenMP_LinearClause : OpenMP_LinearClauseSkip<>; +//===----------------------------------------------------------------------===// +// Not in the spec: Clause-like structure to hold loop related information. +//===----------------------------------------------------------------------===// + +class OpenMP_LoopRelatedClauseSkip< + bit traits = false, bit arguments = false, bit assemblyFormat = false, + bit description = false, bit extraClassDeclaration = false + > : OpenMP_Clause</*isRequired=*/false, traits, arguments, assemblyFormat, + description, extraClassDeclaration> { + let arguments = (ins + UnitAttr:$loop_inclusive + ); + + // Description and formatting integrated in the `omp.loop_nest` operation, + // which is the only one currently accepting this clause. +} + +def OpenMP_LoopRelatedClause : OpenMP_LoopRelatedClauseSkip<>; + //===----------------------------------------------------------------------===// // V5.2: [5.8.3] `map` clause //===----------------------------------------------------------------------===// diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td index a42e32587e570..978b359e13280 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -297,7 +297,7 @@ def SingleOp : OpenMP_Op<"single", traits = [ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ RecursiveMemoryEffects, SameVariadicOperandSize ], clauses = [ - OpenMP_CollapseClause + OpenMP_CollapseClause, OpenMP_LoopRelatedClause ], singleRegion = true> { let summary = "rectangular loop nest"; let description = [{ @@ -306,7 +306,7 @@ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ lower and upper bounds, as well as a step variable, must be defined. The lower and upper bounds specify a half-open range: the range includes the - lower bound but does not include the upper bound. If the `inclusive` + lower bound but does not include the upper bound. If the `loop_inclusive` attribute is specified then the upper bound is also included. The body region can contain any number of blocks. The region is terminated @@ -335,8 +335,6 @@ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ non-perfectly nested loops. }]; - let arguments = !con(clausesArgs, (ins UnitAttr:$inclusive)); - let builders = [ OpBuilder<(ins CArg<"const LoopNestOperands &">:$clauses)> ]; diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp index 18a1c2a1bd587..bcee137604eb7 100644 --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -2047,7 +2047,7 @@ ParseResult LoopNestOp::parse(OpAsmParser &parser, OperationState &result) { // Parse "inclusive" flag. if (succeeded(parser.parseOptionalKeyword("inclusive"))) - result.addAttribute("inclusive", + result.addAttribute("loop_inclusive", UnitAttr::get(parser.getBuilder().getContext())); // Parse step values. @@ -2076,7 +2076,7 @@ void LoopNestOp::print(OpAsmPrinter &p) { auto args = region.getArguments(); p << " (" << args << ") : " << args[0].getType() << " = (" << getCollapseLowerBounds() << ") to (" << getCollapseUpperBounds() << ") "; - if (getInclusive()) + if (getLoopInclusive()) p << "inclusive "; p << "step (" << getCollapseSteps() << ") "; p.printRegion(region, /*printEntryBlockArgs=*/false); diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index b7d1792852b08..f8a67de6e9f6c 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -1196,7 +1196,7 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder, } loopInfos.push_back(ompBuilder->createCanonicalLoop( loc, bodyGen, lowerBound, upperBound, step, - /*IsSigned=*/true, loopOp.getInclusive(), computeIP)); + /*IsSigned=*/true, loopOp.getLoopInclusive(), computeIP)); if (failed(bodyGenStatus)) return failure(); diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir index 9db98883113b4..f639af9b86722 100644 --- a/mlir/test/Dialect/OpenMP/ops.mlir +++ b/mlir/test/Dialect/OpenMP/ops.mlir @@ -184,7 +184,7 @@ func.func @omp_loop_nest(%lb : index, %ub : index, %step : index) -> () { "omp.loop_nest" (%lb, %ub, %step) ({ ^bb0(%iv: index): omp.yield - }) {inclusive} : (index, index, index) -> () + }) {loop_inclusive} : (index, index, index) -> () omp.terminator } >From 52ec167e31ab8b1135d797de11eb982f3e55af97 Mon Sep 17 00:00:00 2001 From: Sergio Afonso <safon...@amd.com> Date: Fri, 26 Jul 2024 11:06:22 +0100 Subject: [PATCH 2/2] Merge CollapseClause arguments into LoopRelatedClause --- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 27 ++++++------ flang/lib/Lower/OpenMP/ClauseProcessor.h | 2 +- .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 4 +- .../Dialect/OpenMP/OpenMPClauseOperands.h | 8 +--- .../mlir/Dialect/OpenMP/OpenMPClauses.td | 44 ++++++------------- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 6 +-- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 14 +++--- .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 17 +++---- 8 files changed, 48 insertions(+), 74 deletions(-) diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index facf95e17707e..310c0b0b5fb63 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -181,20 +181,19 @@ static void addUseDeviceClause( static void convertLoopBounds(lower::AbstractConverter &converter, mlir::Location loc, - mlir::omp::CollapseClauseOps &result, + mlir::omp::LoopRelatedOps &result, std::size_t loopVarTypeSize) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); // The types of lower bound, upper bound, and step are converted into the // type of the loop variable if necessary. mlir::Type loopVarType = getLoopVarType(converter, loopVarTypeSize); - for (unsigned it = 0; it < (unsigned)result.collapseLowerBounds.size(); - it++) { - result.collapseLowerBounds[it] = firOpBuilder.createConvert( - loc, loopVarType, result.collapseLowerBounds[it]); - result.collapseUpperBounds[it] = firOpBuilder.createConvert( - loc, loopVarType, result.collapseUpperBounds[it]); - result.collapseSteps[it] = - firOpBuilder.createConvert(loc, loopVarType, result.collapseSteps[it]); + for (unsigned it = 0; it < (unsigned)result.loopLowerBounds.size(); it++) { + result.loopLowerBounds[it] = firOpBuilder.createConvert( + loc, loopVarType, result.loopLowerBounds[it]); + result.loopUpperBounds[it] = firOpBuilder.createConvert( + loc, loopVarType, result.loopUpperBounds[it]); + result.loopSteps[it] = + firOpBuilder.createConvert(loc, loopVarType, result.loopSteps[it]); } } @@ -204,7 +203,7 @@ static void convertLoopBounds(lower::AbstractConverter &converter, bool ClauseProcessor::processCollapse( mlir::Location currentLocation, lower::pft::Evaluation &eval, - mlir::omp::CollapseClauseOps &result, + mlir::omp::LoopRelatedOps &result, llvm::SmallVectorImpl<const semantics::Symbol *> &iv) const { bool found = false; fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); @@ -233,15 +232,15 @@ bool ClauseProcessor::processCollapse( std::get_if<parser::LoopControl::Bounds>(&loopControl->u); assert(bounds && "Expected bounds for worksharing do loop"); lower::StatementContext stmtCtx; - result.collapseLowerBounds.push_back(fir::getBase( + result.loopLowerBounds.push_back(fir::getBase( converter.genExprValue(*semantics::GetExpr(bounds->lower), stmtCtx))); - result.collapseUpperBounds.push_back(fir::getBase( + result.loopUpperBounds.push_back(fir::getBase( converter.genExprValue(*semantics::GetExpr(bounds->upper), stmtCtx))); if (bounds->step) { - result.collapseSteps.push_back(fir::getBase( + result.loopSteps.push_back(fir::getBase( converter.genExprValue(*semantics::GetExpr(bounds->step), stmtCtx))); } else { // If `step` is not present, assume it as `1`. - result.collapseSteps.push_back(firOpBuilder.createIntegerConstant( + result.loopSteps.push_back(firOpBuilder.createIntegerConstant( currentLocation, firOpBuilder.getIntegerType(32), 1)); } iv.push_back(bounds->name.thing.symbol); diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h index 4340c3278cebc..2c4b3857fda9f 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.h +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h @@ -55,7 +55,7 @@ class ClauseProcessor { // 'Unique' clauses: They can appear at most once in the clause list. bool processCollapse(mlir::Location currentLocation, lower::pft::Evaluation &eval, - mlir::omp::CollapseClauseOps &result, + mlir::omp::LoopRelatedOps &result, llvm::SmallVectorImpl<const semantics::Symbol *> &iv) const; bool processDefault() const; bool processDevice(lower::StatementContext &stmtCtx, diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index cf5001de909a1..d0a47fa5ab598 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -268,8 +268,8 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) { llvm::SmallVector<mlir::Value> vs; vs.reserve(loopOp.getIVs().size()); for (auto [iv, ub, step] : - llvm::zip_equal(loopOp.getIVs(), loopOp.getCollapseUpperBounds(), - loopOp.getCollapseSteps())) { + llvm::zip_equal(loopOp.getIVs(), loopOp.getLoopUpperBounds(), + loopOp.getLoopSteps())) { // v = iv + step // cmp = step < 0 ? v < ub : v > ub mlir::Value v = firOpBuilder.create<mlir::arith::AddIOp>(loc, iv, step); diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauseOperands.h b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauseOperands.h index 4730d544e8739..a680a85db0f25 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauseOperands.h +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauseOperands.h @@ -43,11 +43,6 @@ struct CancelDirectiveNameClauseOps { ClauseCancellationConstructTypeAttr cancelDirective; }; -struct CollapseClauseOps { - llvm::SmallVector<Value> collapseLowerBounds, collapseUpperBounds, - collapseSteps; -}; - struct CopyprivateClauseOps { llvm::SmallVector<Value> copyprivateVars; llvm::SmallVector<Attribute> copyprivateSyms; @@ -125,6 +120,7 @@ struct LinearClauseOps { }; struct LoopRelatedOps { + llvm::SmallVector<Value> loopLowerBounds, loopUpperBounds, loopSteps; UnitAttr loopInclusive; }; @@ -261,7 +257,7 @@ using DistributeOperands = detail::Clauses<AllocateClauseOps, DistScheduleClauseOps, OrderClauseOps, PrivateClauseOps>; -using LoopNestOperands = detail::Clauses<CollapseClauseOps, LoopRelatedOps>; +using LoopNestOperands = detail::Clauses<LoopRelatedOps>; using MaskedOperands = detail::Clauses<FilterClauseOps>; diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td index 24bd9b69e6cc1..e703c323edbc8 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td @@ -107,37 +107,6 @@ class OpenMP_CancelDirectiveNameClauseSkip< def OpenMP_CancelDirectiveNameClause : OpenMP_CancelDirectiveNameClauseSkip<>; -//===----------------------------------------------------------------------===// -// V5.2: [4.4.3] `collapse` clause -//===----------------------------------------------------------------------===// - -class OpenMP_CollapseClauseSkip< - bit traits = false, bit arguments = false, bit assemblyFormat = false, - bit description = false, bit extraClassDeclaration = false - > : OpenMP_Clause</*isRequired=*/false, traits, arguments, assemblyFormat, - description, extraClassDeclaration> { - let traits = [ - AllTypesMatch< - ["collapse_lower_bounds", "collapse_upper_bounds", "collapse_steps"]> - ]; - - let arguments = (ins - Variadic<IntLikeType>:$collapse_lower_bounds, - Variadic<IntLikeType>:$collapse_upper_bounds, - Variadic<IntLikeType>:$collapse_steps - ); - - let extraClassDeclaration = [{ - /// Returns the number of loops in the loop nest. - unsigned getNumLoops() { return getCollapseLowerBounds().size(); } - }]; - - // Description and formatting integrated in the `omp.loop_nest` operation, - // which is the only one currently accepting this clause. -} - -def OpenMP_CollapseClause : OpenMP_CollapseClauseSkip<>; - //===----------------------------------------------------------------------===// // V5.2: [5.7.2] `copyprivate` clause //===----------------------------------------------------------------------===// @@ -573,10 +542,23 @@ class OpenMP_LoopRelatedClauseSkip< bit description = false, bit extraClassDeclaration = false > : OpenMP_Clause</*isRequired=*/false, traits, arguments, assemblyFormat, description, extraClassDeclaration> { + let traits = [ + AllTypesMatch< + ["loop_lower_bounds", "loop_upper_bounds", "loop_steps"]> + ]; + let arguments = (ins + Variadic<IntLikeType>:$loop_lower_bounds, + Variadic<IntLikeType>:$loop_upper_bounds, + Variadic<IntLikeType>:$loop_steps, UnitAttr:$loop_inclusive ); + let extraClassDeclaration = [{ + /// Returns the number of loops in the loop nest. + unsigned getNumLoops() { return getLoopLowerBounds().size(); } + }]; + // Description and formatting integrated in the `omp.loop_nest` operation, // which is the only one currently accepting this clause. } diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td index 978b359e13280..00dadc6e6538d 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -297,7 +297,7 @@ def SingleOp : OpenMP_Op<"single", traits = [ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ RecursiveMemoryEffects, SameVariadicOperandSize ], clauses = [ - OpenMP_CollapseClause, OpenMP_LoopRelatedClause + OpenMP_LoopRelatedClause ], singleRegion = true> { let summary = "rectangular loop nest"; let description = [{ @@ -312,8 +312,8 @@ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ The body region can contain any number of blocks. The region is terminated by an `omp.yield` instruction without operands. The induction variables, represented as entry block arguments to the loop nest operation's single - region, match the types of the `collapse_lower_bounds`, - `collapse_upper_bounds` and `collapse_steps` arguments. + region, match the types of the `loop_lower_bounds`, `loop_upper_bounds` and + `loop_steps` arguments. ```mlir omp.loop_nest (%i1, %i2) : i32 = (%c0, %c0) to (%c10, %c10) step (%c1, %c1) { diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp index bcee137604eb7..84de63b7806dc 100644 --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -2075,28 +2075,28 @@ void LoopNestOp::print(OpAsmPrinter &p) { Region ®ion = getRegion(); auto args = region.getArguments(); p << " (" << args << ") : " << args[0].getType() << " = (" - << getCollapseLowerBounds() << ") to (" << getCollapseUpperBounds() << ") "; + << getLoopLowerBounds() << ") to (" << getLoopUpperBounds() << ") "; if (getLoopInclusive()) p << "inclusive "; - p << "step (" << getCollapseSteps() << ") "; + p << "step (" << getLoopSteps() << ") "; p.printRegion(region, /*printEntryBlockArgs=*/false); } void LoopNestOp::build(OpBuilder &builder, OperationState &state, const LoopNestOperands &clauses) { - LoopNestOp::build(builder, state, clauses.collapseLowerBounds, - clauses.collapseUpperBounds, clauses.collapseSteps, + LoopNestOp::build(builder, state, clauses.loopLowerBounds, + clauses.loopUpperBounds, clauses.loopSteps, clauses.loopInclusive); } LogicalResult LoopNestOp::verify() { - if (getCollapseLowerBounds().empty()) + if (getLoopLowerBounds().empty()) return emitOpError() << "must represent at least one loop"; - if (getCollapseLowerBounds().size() != getIVs().size()) + if (getLoopLowerBounds().size() != getIVs().size()) return emitOpError() << "number of range arguments and IVs do not match"; - for (auto [lb, iv] : llvm::zip_equal(getCollapseLowerBounds(), getIVs())) { + for (auto [lb, iv] : llvm::zip_equal(getLoopLowerBounds(), getIVs())) { if (lb.getType() != iv.getType()) return emitOpError() << "range argument type does not match corresponding IV type"; diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index f8a67de6e9f6c..fe3f258156690 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -1109,8 +1109,7 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder, wsloopOp.getScheduleKind().value_or(omp::ClauseScheduleKind::Static); // Find the loop configuration. - llvm::Value *step = - moduleTranslation.lookupValue(loopOp.getCollapseSteps()[0]); + llvm::Value *step = moduleTranslation.lookupValue(loopOp.getLoopSteps()[0]); llvm::Type *ivType = step->getType(); llvm::Value *chunk = nullptr; if (wsloopOp.getScheduleChunk()) { @@ -1179,11 +1178,10 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder, llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder(); for (unsigned i = 0, e = loopOp.getNumLoops(); i < e; ++i) { llvm::Value *lowerBound = - moduleTranslation.lookupValue(loopOp.getCollapseLowerBounds()[i]); + moduleTranslation.lookupValue(loopOp.getLoopLowerBounds()[i]); llvm::Value *upperBound = - moduleTranslation.lookupValue(loopOp.getCollapseUpperBounds()[i]); - llvm::Value *step = - moduleTranslation.lookupValue(loopOp.getCollapseSteps()[i]); + moduleTranslation.lookupValue(loopOp.getLoopUpperBounds()[i]); + llvm::Value *step = moduleTranslation.lookupValue(loopOp.getLoopSteps()[i]); // Make sure loop trip count are emitted in the preheader of the outermost // loop at the latest so that they are all available for the new collapsed @@ -1644,11 +1642,10 @@ convertOmpSimd(Operation &opInst, llvm::IRBuilderBase &builder, llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder(); for (unsigned i = 0, e = loopOp.getNumLoops(); i < e; ++i) { llvm::Value *lowerBound = - moduleTranslation.lookupValue(loopOp.getCollapseLowerBounds()[i]); + moduleTranslation.lookupValue(loopOp.getLoopLowerBounds()[i]); llvm::Value *upperBound = - moduleTranslation.lookupValue(loopOp.getCollapseUpperBounds()[i]); - llvm::Value *step = - moduleTranslation.lookupValue(loopOp.getCollapseSteps()[i]); + moduleTranslation.lookupValue(loopOp.getLoopUpperBounds()[i]); + llvm::Value *step = moduleTranslation.lookupValue(loopOp.getLoopSteps()[i]); // Make sure loop trip count are emitted in the preheader of the outermost // loop at the latest so that they are all available for the new collapsed _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits