[PATCH] D80004: [MLIR] Add `from_index`, `to_index`, and `num_elements` to the shape dialect

Stephan Herhut via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 18 03:10:42 PDT 2020


herhut added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:109
 
+def Shape_FromIndexOp : Shape_Op<"from_index", []> {
+  let summary = "Creates a shape size from a standard index";
----------------
pifon2a wrote:
> silvas wrote:
> > These should be marked NoSideEffect and have folders and InferTypeOpInterface. See https://reviews.llvm.org/D79833 for an example of another op.
> I am not sure `from_index` is enough. In the IR i first read it as if it creates 1D shape with the provided number number of elements. Maybe call this `from_index_rank`?
> 
> Is there some special reason why we have `shape.size` instead of `shape.rank`?
Maybe `index_to_size` and `size_to_index` would better convey what this does. We could also have a single operation `size_cast` in the spirit of the `index_cast` operation in standard that allows both conversions. That would be more uniform and, if we get to a state where we can have a type trait that would allow to use 'index_cast', it would be easier to rewrite.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80004





More information about the llvm-commits mailing list