-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[MLIR][OpenMP] NFC: Remove LoopControl parsing/printing code #88909
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.
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
Conversation
This patch introduces an operation intended to hold loop information associated to the `omp.distribute`, `omp.simdloop`, `omp.taskloop` and `omp.wsloop` operations. This is a stopgap solution to unblock work on transitioning these operations to becoming wrappers, as discussed in [this RFC](https://discourse.llvm.org/t/rfc-representing-combined-composite-constructs-in-the-openmp-dialect/76986). Long-term, this operation will likely be replaced by `omp.canonical_loop`, which is being designed to address missing support for loop transformations, etc.
This patch defines a common interface to be shared by all OpenMP loop wrapper operations. The main restrictions these operations must meet in order to be considered a wrapper are: - They contain a single region. - Their region contains a single block. - Their block only contains another loop wrapper or `omp.loop_nest` and a terminator. The new interface is attached to the `omp.parallel`, `omp.wsloop`, `omp.simdloop`, `omp.distribute` and `omp.taskloop` operations. It is not currently enforced that these operations meet the wrapper restrictions, which would break existing OpenMP loop-generating code. Rather, this will be introduced progressively in subsequent patches.
…/spr/loop-nest-02-wrapper-iface
This patch updates the definition of `omp.simdloop` to enforce the restrictions of a wrapper operation. It has been renamed to `omp.simd`, to better reflect the naming used in the spec. All uses of "simdloop" in function names have been updated accordingly. Some changes to Flang lowering and OpenMP to LLVM IR translation are introduced to prevent the introduction of compilation/test failures. The eventual long term solution might be different.
This patch removes the LoopControl parsing/printing functions that are no longer used after transitioning `omp.simdloop` and `omp.taskloop` into loop wrapper operations.
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-openmp Author: Sergio Afonso (skatrak) ChangesThis patch removes the LoopControl parsing/printing functions that are no longer used after transitioning Full diff: https://github.com/llvm/llvm-project/pull/88909.diff 1 Files Affected:
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index caf0ac3f860172..5d2281ce6094fd 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1548,58 +1548,6 @@ void printWsloop(OpAsmPrinter &p, Operation *op, Region ®ion,
p.printRegion(region, /*printEntryBlockArgs=*/false);
}
-/// loop-control ::= `(` ssa-id-list `)` `:` type `=` loop-bounds
-/// loop-bounds := `(` ssa-id-list `)` to `(` ssa-id-list `)` inclusive? steps
-/// steps := `step` `(`ssa-id-list`)`
-ParseResult
-parseLoopControl(OpAsmParser &parser, Region ®ion,
- SmallVectorImpl<OpAsmParser::UnresolvedOperand> &lowerBound,
- SmallVectorImpl<OpAsmParser::UnresolvedOperand> &upperBound,
- SmallVectorImpl<OpAsmParser::UnresolvedOperand> &steps,
- SmallVectorImpl<Type> &loopVarTypes, UnitAttr &inclusive) {
- // Parse an opening `(` followed by induction variables followed by `)`
- SmallVector<OpAsmParser::Argument> ivs;
- Type loopVarType;
- if (parser.parseArgumentList(ivs, OpAsmParser::Delimiter::Paren) ||
- parser.parseColonType(loopVarType) ||
- // Parse loop bounds.
- parser.parseEqual() ||
- parser.parseOperandList(lowerBound, ivs.size(),
- OpAsmParser::Delimiter::Paren) ||
- parser.parseKeyword("to") ||
- parser.parseOperandList(upperBound, ivs.size(),
- OpAsmParser::Delimiter::Paren))
- return failure();
-
- if (succeeded(parser.parseOptionalKeyword("inclusive")))
- inclusive = UnitAttr::get(parser.getBuilder().getContext());
-
- // Parse step values.
- if (parser.parseKeyword("step") ||
- parser.parseOperandList(steps, ivs.size(), OpAsmParser::Delimiter::Paren))
- return failure();
-
- // Now parse the body.
- loopVarTypes = SmallVector<Type>(ivs.size(), loopVarType);
- for (auto &iv : ivs)
- iv.type = loopVarType;
-
- return parser.parseRegion(region, ivs);
-}
-
-void printLoopControl(OpAsmPrinter &p, Operation *op, Region ®ion,
- ValueRange lowerBound, ValueRange upperBound,
- ValueRange steps, TypeRange loopVarTypes,
- UnitAttr inclusive) {
- auto args = region.front().getArguments();
- p << " (" << args << ") : " << args[0].getType() << " = (" << lowerBound
- << ") to (" << upperBound << ") ";
- if (inclusive)
- p << "inclusive ";
- p << "step (" << steps << ") ";
- p.printRegion(region, /*printEntryBlockArgs=*/false);
-}
-
//===----------------------------------------------------------------------===//
// Simd construct [2.9.3.1]
//===----------------------------------------------------------------------===//
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This patch removes the LoopControl parsing/printing functions that are no longer used after transitioning
omp.simdloop
andomp.taskloop
into loop wrapper operations.