[PATCH] D72432: [mlir] Add shaped container component type interface
Jacques Pienaar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 08:03:38 PST 2020
jpienaar added a comment.
Herald added a subscriber: liufengdb.
Apparently I forgot to hit submit (seeing unsubmitted comments below).
================
Comment at: mlir/docs/ShapeInference.md:28
+ constraining the input type to be tensor/vector elements or that the
+ elemental type be of a specific type (e.g., output of sign is of elemental
+ type `i1`) or class (e.g., float like).
----------------
rriddle wrote:
> Is `sign` a know thing we can link to?
Not really, I'm referring to basic math. I'll spell it out a bit more.
================
Comment at: mlir/include/mlir/Analysis/InferTypeOpInterface.h:73
+ ShapeStorageT dims;
+ bool ranked;
+ Type elementType;
----------------
rriddle wrote:
> nit: Can you pack this bool with either the type or the attribute?
Maybe, but it doesn't belong there logically: it matches dims and not the others. I'd prefer not pack it until needed (e.g., we could likely change how dims are stored here and then it could change the packing).
================
Comment at: mlir/include/mlir/Analysis/InferTypeOpInterface.h:86
+LogicalResult inferReturnTensorTypes(
+ LogicalResult (*componentTypeFn)(
+ MLIRContext *, Optional<Location> location, ValueRange operands,
----------------
rriddle wrote:
> Why not function_ref here? Also, is this function intended to be user facing? i.e. should other users just call this function?
Made function_ref and moved to namespace detail as I expect this to be called via inferReturnTypes, and this is just to extract common function out to reduce size of InferTensorType<...> below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72432/new/
https://reviews.llvm.org/D72432
More information about the llvm-commits
mailing list