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

[Frederik Gossen via Phabricator via llvm-commits]

frgossen marked 2 inline comments as done.
frgossen 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.
----------------
herhut wrote:
> 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.
As there exists no lowering at the moment, it seems that all of these operations' semantics are embedded in the constant folding mechanism. 
Would it be okay to say the behaviour is undefined in this case? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80280