delldu commented on a change in pull request #8443:
URL: https://github.com/apache/tvm/pull/8443#discussion_r670541721



##########
File path: include/tvm/relay/attrs/nn.h
##########
@@ -1463,6 +1463,21 @@ struct NLLLossAttrs : public 
tvm::AttrsNode<NLLLossAttrs> {
   }
 };  // struct NLLLossAttrs
 
+/*! \brief Attributes used in Im2col operator */
+struct Im2colAttrs : public tvm::AttrsNode<Im2colAttrs> {
+  Array<IndexExpr> kernel_size;
+  Array<IndexExpr> dilation;
+  Array<IndexExpr> padding;
+  Array<IndexExpr> stride;

Review comment:
       Hello @masahi , @wyc-ruiker , 
   
   First all, thank you help us improve PR. Sounds there is one import issue 
left, I would give my understanding. 
   
   Second, please remember `using IndexExpr = ::tvm::PrimExpr;`
   
   **1. Conclusion**
   Changing `Array<IndexExpr>` to` Array<Integer>` in Im2colAttrs is not good.
   
   **2. Function Patch**
   inline tvm::te::Tensor im2col(const tvm::te::Tensor& data, ...)
   {
     auto input_b = data->shape[0];
     ...
     auto kernel_h = tvm::cast(tvm::DataType::Int(32), kernel_size[0]);
     ...
     tvm::Array<tvm::PrimExpr> output_shape;
     output_shape.push_back(N);
     ...
     auto pad_value = tvm::tir::make_const(data->dtype, 0);
     auto l = [&](tvm::Array<tvm::tir::Var> args) {
       ...
       tvm::PrimExpr s_c = indexdiv(indexdiv(k, kernel_h), kernel_w);
       indices.push_back(s_c);  // C, source channel
       ...
       return tvm::if_then_else(..., data(indices), pad_value);
     };
     return tvm::te::compute(output_shape, l, name, tag);
   }
   
   **3. Cause analysis**
   1)  Input
   The shape of input data is `Array<PrimExpr>`;
   `inline tvm::te::Tensor im2col(const tvm::te::Tensor& data`
   2)  Output
   output shape is also `Array<PrimExpr>`.
   `TVM_DLL Tensor compute(Array<PrimExpr> shape, ...);
   `
   3)  Process
   3.1) Many expression in lambda function such as indices, are 
`Array<PrimExpr>`, NOT `Array<Integer>`.
   3.2) Some function parameters are PrimExpr.
        For an example: prototye of indexmod is: 
   ```
   TVM_DLL PrimExpr indexdiv(PrimExpr a, PrimExpr b, Span span = Span());
   ```
   If we give Integer parameters to indexdiv, only got compile fatal warning 
and errors ...
   
   **4. Test Result**
   We try to change `Array<IndexExpr>` to `Array<Integer>`, modified related 
source code, 
   fighting again and again, using many methods, finally got many and many 
building(compile) errors.
   
   **5. Inference**
   `tvm::cast` is good choice than `as_const_int` for parsing parameters.
   The reason comes from their prototype
   ```
   inline const *int64_t as_const_int(const PrimExpr& x);
   TVM_DLL PrimExpr cast(const DataType& t, PrimExpr value, Span span = Span());
   ```
   
   Following above results, we could not use `Array<Integer>`.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to