[PATCH] D79829: [mlir][Affine] Introduce affine memory interfaces

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/lib/Dialect/Affine/IR/AffineOps.cpp:2564-2581
+/// Get memref operand.
+Value AffineVectorLoadOp::getMemRef() {
+  return getOperand(getMemRefOperandIndex());
+}
+
+MemRefType AffineVectorLoadOp::getMemRefType() {
+  return getMemRef().getType().cast<MemRefType>();
----------------
bondhugula wrote:
> dcaballe wrote:
> > bondhugula wrote:
> > > I missed why we need to introduce these here? Shouldn't they be shared via the op interface method?
> > Are you suggesting using the interface also as a base class? I assumed that interfaces shouldn't have any implementation. That could work for some methods but not all of them have the same implementation (e.g., `getMapOperands`). I could move those with the same implementation.
> You are right that some of the ops that use this interface will not have the same implementation. But for my information,  @ftynse, @rriddle, can Op Interfaces have implementations for things that are guaranteed to be shared among all ops using them? I remember seeing a default implementation being provided somewhere but I may be wrong.  
> 
> Anyway, can these be moved into AffineLoadOpBase, AffineStoreOpBase? (extra class declarations)
I believe OpInterface method are type-erased virtual function, they can have a default implementation in which case an op may or may override the default implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79829