[PATCH] D73944: [mlir][wip] Start Shape dialect

[Lei Zhang via Phabricator via llvm-commits]

antiagainst added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:181
+
+def Shape_BroadcastableOp : Shape_Op<"broadcastable", []> {
+  let summary = "Returns the broadcastable output shape";
----------------
I'd typically think an op means some action so the name is typically a verb but here we have `broadcastable` which sounds like a trait. What about just name it as `broadcast`?


================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:207
+// easier for folks.
+def Shape_JoinOp : Shape_Op<"join", []> {
+  let summary = "Returns the least general shape/dim of its operands";
----------------
My 2c: join sounds better for me as it indicates some operation behind. any_of gives me the feeling that the result will be the same as one of the input, which is not always true. :)


================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:280
+
+def Shape_PrintOp : Shape_Op<"print", []> {
+  let summary = "Prints the input dim or shape";
----------------
Would it make sense to name it as `debug_print` to be more clear?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73944