[GitHub] [incubator-tvm] lhutton1 commented on a change in pull request #5422: [RELAY][Convert Layout] Specify additional layouts in convert layout pass

[GitBox <gi...@apache.org>]

lhutton1 commented on a change in pull request #5422:
URL: https://github.com/apache/incubator-tvm/pull/5422#discussion_r413999028



##########
File path: include/tvm/relay/op_attr_types.h
##########
@@ -158,13 +158,15 @@ using FTVMAlterOpLayout = runtime::TypedPackedFunc<
  * \param tinfos An array of placeholders, use for getting the inferred shape
  *               and dtype of the inputs.
  * \param desired_layout The desired layout.
+ * \param additional_layouts Specify additional layouts, e.g. kernel_layout.
  * \return new_expr The modified expression.
  */
 using FTVMConvertOpLayout = runtime::TypedPackedFunc<
   Expr(const Attrs& attrs,
        const Array<Expr>& args,
        const Array<te::Tensor>& tinfos,
-       const std::string& desired_layout)>;
+       const std::string& desired_layout,

Review comment:
       Thanks, I hope I understood correctly. So the idea is that someone using this pass would specify a map of operator -> [layouts] e.g. `relay.transform.ConvertLayout({"nn.conv2d": ["NCHW", "IOHW"], ...})`? My intention for doing it the additional_layouts way was to minimise the impact on the current state of the pass, although I like your idea.
   
   My only concern with using a list would be an operator that had say 3 different input tensors with different layouts (I don't think one exists currently, could this pop up in the future?). For example data, kernel, some_other. How could we specify a preferred layout for data and some_other whilst leaving kernel set to default?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org