[PATCH] D72394: [mlir] Add skeleton implementation of loop.parallel, loop.yield and loop.yield.return operations.

[Adrian Kuegel via Phabricator via llvm-commits]

akuegel added inline comments.


================
Comment at: mlir/lib/Dialect/LoopOps/LoopOps.cpp:343
+  auto type = op.operand()->getType();
+  auto &block = op.operation().front();
+  if (block.getNumArguments() != 2 ||
----------------
rriddle wrote:
> akuegel wrote:
> > rriddle wrote:
> > > I don't see a check for empty block, can you add one.
> > I think this is automatically checked because it is defined as a SizedRegion. See the test yield_no_block in invalid.mlir
> > But as far as I understand Nicolas' comment, I actually should remove that test, too (leaving it here for now for verification if maybe I misunderstood something).
> SizedRegion means the number of blocks, it doesn't constrain the number of operations within the blocks. e.g. an empty block would crash the isa<ReduceReturnOp> check below.
Ah, I mixed up those two. Thanks for explaining :)
I have now added a check for this as well, and modified the test to not test for no block, but instead for whether the block is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72394