merrymercy edited a comment on pull request #6750:
URL: https://github.com/apache/incubator-tvm/pull/6750#issuecomment-716930809


   The code looks good to me. But the naming is not intuitive. For example, the 
"RewriteWithPlaceholder" option in your code actually accept "PreTransposed" 
tensors.
   The key difference between the two options is whether adding new stages. We 
can make the names much clearer as follows.
   ```c++
   /*!
    * \brief Options for applying layout rewrite.
    * This is an optimization to rewrite the layout of input tensors according 
to the schedule we get.
    */
   enum class LayoutRewriteOption : int {
     /*! \brief Do not process layout rewrite. */
     NoRewrite = 0,
     /*! \brief Insert layout transform stages for input placeholders in the 
compute DAG */
     AddTransformStage = 1,
     /*!
      * \brief Do not insert layout transform stages and assume the input 
placeholders
      * are pre-transformed.
      * \note The lowered function with this option does not accept the origial 
input shapes,
      * so this option must be used along with a layout conversion pass in 
Relay.
      */
     RewriteForPreTransformed = 2,
   };
   
   ```


----------------------------------------------------------------
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:
[email protected]


Reply via email to