[PATCH] D73296: [mlir] Add MemRefTypeBuilder and refactor some MemRefType::get().

[Tim Shen via Phabricator via llvm-commits]

timshen added a comment.

In D73296#1837240 <https://reviews.llvm.org/D73296#1837240>, @nicolasvasilache wrote:

> I personally love the API with chainable calls but we should really get @rriddle 's buyin on this.


FWIW I'm open to any API designs that allow implicit propagation of fields like memory space (and alignment in my next patches).

> I would change the return by reference to just return a new type by value: Types (and recently Values) are small value-type wrappers around a pointer, we should use value semantics everywhere.

Which function specifically were you talking about?

> Also, if @rriddle agrees to go this way, can we hide MemRef::get and friends from the public API and advertise this as the way to go?

My understanding is that ::get and ::getChecked is somewhat generic w.r.t. the contrats with TypeBase, and they might be hard to get off from.

I also won't advertise too much for the MemRefTypeBuilder style API, in my personal preference of designated initializer style "named parameters":

  class MemRefType {
    struct BuildOptions {
      ArrayRef<AffineMap> affineMaps;
      unsigned memorySpace;
    };
    static MemRefType::get(ArrayRef<int64_t> shape, Type elementType, BuildOptions);
  };
  
  MemRefType(shape, elementType, { .memorySpace = 3 });

But we will have to wait for MSVC 2019.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73296