[PATCH] D72808: [mlir] [VectorOps] Lowering of vector.extract/insert_slices to LLVM IR

[Aart Bik via Phabricator via llvm-commits]

aartbik marked an inline comment as done.
aartbik added a comment.

sounds good, one question though



================
Comment at: mlir/lib/Dialect/VectorOps/VectorOps.cpp:1699
+    if (auto tupleOp =
+            dyn_cast_or_null<TupleOp>(op.getOperand().getDefiningOp())) {
+      rewriter.replaceOp(op, tupleOp.getOperand(op.getIndex()));
----------------
aartbik wrote:
> rriddle wrote:
> > rriddle wrote:
> > > rriddle wrote:
> > > > This should just be in the 'fold' method instead of a canonicalization pattern.
> > > 'fold' should be used whenever possible because it is applicable in many more places, e.g. during dialect conversion and OpBuilder::createOrFold.
> > Also, please split this out into a different revision as this is unrelated.
> I followed the other "canonicalization" patterns in VectorOps. Just for my own understanding, should these strictly speaking have been written as folders too (the name "Folder" seems to imply that)?
Other question, isn't fold for constants only? Here the tuple itself does not need to be constant, the only thing that matters is that get-on-tuple becomes a pass through of the value at the corresponding index?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72808