[PATCH] D79541: [mlir] Simplify and better document std.view semantics

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.
Herald added a subscriber: jurahul.


================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:3036
     operand_range getDynamicSizes() {
-      return {operand_begin() + getDynamicSizesOperandStart(), operand_end()};
+      return {sizes().begin(), sizes().end()};
     }
----------------
Why not just `sizes()`?


================
Comment at: mlir/lib/Dialect/StandardOps/IR/Ops.cpp:2778
   p << op.getOperationName() << ' ' << op.getOperand(0) << '[';
-  auto dynamicOffset = op.getDynamicOffset();
-  if (dynamicOffset != nullptr)
-    p.printOperand(dynamicOffset);
-  p << "][" << op.getDynamicSizes() << ']';
+  p.printOperand(op.byte_shift());
+  p << "][" << op.sizes() << ']';
----------------
Just stream it:

`p << ... << op.byte_shift() << ...`.

I would also expect that this could now use the declarative format now that the weird 'dynamicOffset' thing is removed.


================
Comment at: mlir/lib/Dialect/StandardOps/IR/Ops.cpp:2795
+  // The result memref should have identity layout map (or none).
+  if (viewType.getAffineMaps().size() > 1 ||
+      (viewType.getAffineMaps().size() == 1 &&
----------------
nit: Can you move the `viewType.getAffineMaps()` to a local variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79541