[PATCH] D74288: [MLIR][Affine] Add affine.parallel op

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula accepted this revision.
bondhugula added inline comments.


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2181
+                             AffineMap ubMap, ValueRange ubArgs) {
+  auto dims = lbMap.getNumResults();
+  // Verify that the number of dims in both maps are the same.
----------------
Nit: dims -> dim or numDims


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2194
+  auto dims = lbMap.getNumResults();
+  // Verify that the number of dims in maps and steps are the same.
+  assert(dims == ubMap.getNumResults());
----------------
Nit: -> Verify that the dimensionality of the maps matches the number of steps. 


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2239
+  // Try to convert all the ranges to constant expressions.
+  SmallVector<int64_t, 8> out;
+  AffineValueMap rangesValueMap = getRangesValueMap();
----------------
`out.reserve(rangesValueMap.getNumResults());`


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2258
+void AffineParallelOp::setSteps(ArrayRef<int64_t> newSteps) {
+  assert(newSteps.size() == getNumDims());
+  setAttr(getStepsAttrName(), getBodyBuilder().getI64ArrayAttr(newSteps));
----------------
Nit: message please - && "fewer/too many steps"


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2263
+static LogicalResult verify(AffineParallelOp op) {
+  auto dims = op.getNumDims();
+  if (op.lowerBoundsMap().getNumResults() != dims ||
----------------
Nit: dims -> numDims / nDims / dim ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74288/new/

https://reviews.llvm.org/D74288