[PATCH] D74174: [MLIR] Allow Loop dialect IfOp and ForOp to define values

[Stephen Neuendorffer via Phabricator via llvm-commits]

stephenneuendorffer added a comment.

This looks like a really great start!  Thanks for sketching this out.



================
Comment at: mlir/lib/Dialect/LoopOps/LoopOps.cpp:158-168
+  if (!parser.parseOptionalKeyword("iter_args")) {
+    parser.parseAssignmentList(regionArgs, operands, argTypes);
+    // resolve input operands
+    for (auto operand_type : llvm::zip(operands, argTypes))
+      parser.resolveOperand(std::get<0>(operand_type),std::get<1>(operand_type), result.operands);
+  }
+  argTypes.insert(argTypes.begin(), indexType);
----------------
For the for() loop syntax, I think that iter_args and the result type list should go together.  They are either both present or both absent.  As a result, there should be no need to replicate the type information 2x in the op syntax.

I think this could work two ways:

for()....iter_args(%x=%y) -> (i32) {...}
if %c -> (i32) {...} else {...}  

However, with the else branch of the if being optional, it might make sense to have:

for()....args(%x=%y : i32) {...}
if %c args(%x=%y: i32) {...} else {...}

This would also more conveniently represent the case where %x is assigned in one branch of the if, but not the other.




================
Comment at: mlir/test/Dialect/Loops/invalid.mlir:356
+  return
+}
----------------
Need negative tests for type conflicts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74174