[PATCH] D80280: [MLIR] Add `index_to_size` and `size_to_index` to the shape dialect

[Stephan Herhut via Phabricator via llvm-commits]

herhut added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:294
+    This operation and its inverse, `index_to_size`, facilitate index conversion
+    between the standard and the shape dialect.
+    Expects a `shape.size` and returns an `index` value.
----------------
jpienaar wrote:
> size represents a non-negative integer with support for being unknown and invalid, what is the conversion/behavior when the operand is invalid?
If the size is invalid, then I'd assume that all code that depends on it has undefined behavior. In particular, `size_to_index` should have an undefined result. For the inverse, we could consider checking that the `index` argument is indeed positive or clamp it at [0,inf). That has the drawback that we cannot compile the conversion from index to size into a no-op.

I would prefer not to expose the internal representation of invalid (-1 I believe) at runtime. However, if we go for undefined behavior, returning -1 would be valid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80280