[PATCH] D75837: Introduce std.execute_region op

Sean Silva via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 18 22:15:10 PDT 2020


silvas added a comment.

Okay, let's land this without allowing explicit captures, given that's the most restrictive semantics. We can loosen it later if there's a compelling need.

@nicolasvasilache is that ok with you?



================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:978
+def ExecuteRegionOp : Std_Op<"execute_region"> {
+  let summary = "execute_region operation";
+  let description = [{
----------------
make the summary more descriptive.


================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:979
+  let summary = "execute_region operation";
+  let description = [{
+      The execute_region operation executes the region held exactly once. The op
----------------
Indent description by two spaces (instead of 4) for consistency with the rest of the file


================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:1025
+
+  let verifier = ?;
+
----------------
Can you add a verifier that the region doesn't have args?

Also, I'm not super familiar with ODS, but does this specification autogenerate a verifier that there are no operands, or does it just not check anything about the op's operands? If the latter, please change it so that the verifier checks that there are no operands to this op. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75837





More information about the llvm-commits mailing list