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

[Frank Laub via Phabricator via llvm-commits]

flaub marked 13 inline comments as done.
flaub added inline comments.


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2268
+    return op.emitOpError("region argument count and num results of upper "
+                          "bounds, lower bounds, and steps must all match");
+  }
----------------
bondhugula wrote:
> If you'd like to have all the canonicalizations surrounding affine operations and extend some of the affine analysis routines to this, you'll need to call verifyDimAndSymbolIdentifiers on the map operands. I don't think any of your current test cases will break, but you'll have to add a couple of invalid test cases where one of your map operands isn't a symbol - for eg. a value loaded right inside an outer loop. 
Thanks! Done.


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2275
+  p << op.getOperationName() << " (";
+  p.printOperands(op.getBody()->getArguments());
+  p << ") = (";
----------------
rriddle wrote:
> nit: Can you stream the arguments?
Looks like no because `getArguments()` returns a `MutableArrayRef<Value>` and currently it appears that `ValueRange` isn't constructible from this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74288