Mousius commented on a change in pull request #9409:
URL: https://github.com/apache/tvm/pull/9409#discussion_r752083433
##########
File path: src/relay/backend/contrib/cmsisnn/generate_constants.cc
##########
@@ -105,11 +105,25 @@ class GenerateConstantsMutator : public MixedModeMutator {
conv2d_call = requantize_input;
}
- // Transpose weights: HWIO -> OHWI
auto* conv2d_attrs = conv2d_call->attrs.as<Conv2DAttrs>();
- tvm::Attrs new_conv2d_attrs;
- Expr transposed_kernel =
- ConvertKernelLayout(conv2d_call->args[1], conv2d_attrs,
&new_conv2d_attrs);
+ tvm::Attrs new_conv2d_attrs = conv2d_call->attrs;
+ Expr conv2d_kernel = conv2d_call->args[1];
+
+ bool is_depthwise = false;
+ Array<PrimExpr> input_shape =
conv2d_call->args[0]->type_as<TensorTypeNode>()->shape;
+ Array<PrimExpr> kernel_shape =
conv2d_call->args[1]->type_as<TensorTypeNode>()->shape;
+ std::string kernel_layout = conv2d_attrs->kernel_layout.c_str();
+ int kernel_pos_o = kernel_layout.find("O");
+ int groups = conv2d_attrs->groups;
+ if (groups == qnn::get_const_int(input_shape[3]) &&
+ groups == qnn::get_const_int(kernel_shape[kernel_pos_o])) {
+ is_depthwise = true;
+ }
Review comment:
Minor suggestion, initialise `is_depthwise` once.
```suggestion
bool is_depthwise = (
groups == qnn::get_const_int(input_shape[3]) &&
groups == qnn::get_const_int(kernel_shape[kernel_pos_o])
) ;
}
```
##########
File path: tests/python/contrib/test_cmsisnn/test_conv2d.py
##########
@@ -147,40 +149,45 @@ def test_op_int8(
input_scale,
kernel_scale,
out_channels,
+ conv_type,
+ depth_multiplier,
):
interface_api = "c"
use_unpacked_api = True
test_runner = AOT_CORSTONE300_RUNNER
- kernel_zero_point = 0
+ dtype = "int8"
groups = 1
weight_format = "HWIO"
kernel_h = kernel_size[0]
kernel_w = kernel_size[1]
- dtype = "int8"
+ kernel_shape = (kernel_h, kernel_w, ifm_shape[3] // groups, out_channels)
+ kernel_zero_point = 0
in_min, in_max = get_range_for_dtype_str(dtype)
- weight_shape = None
- if weight_format == "HWIO":
- weight_shape = (kernel_h, kernel_w, ifm_shape[3] // groups,
out_channels)
- else:
- weight_shape = (kernel_h, kernel_w, ifm_shape[3], out_channels)
+ if conv_type == "depthwise":
+ groups = ifm_shape[3]
+ weight_format = "HWOI"
+ kernel_shape = (kernel_h, kernel_w, ifm_shape[3], depth_multiplier)
+ out_channels = ifm_shape[3] * depth_multiplier
+ ks_len = len(kernel_scale)
+ kernel_scale = [kernel_scale[i % ks_len] for i in range(out_channels)]
Review comment:
Probably a future one, but we should use a separate test case here,
maybe `test_depthwise_int8`
##########
File path: src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc
##########
@@ -88,8 +88,12 @@ class CodeGenCMSISNN : public codegen::CodeGenCHost {
std::string EmitCMSISNNConvParams(std::ostream& os, int32_t input_offset,
int32_t output_offset,
int32_t stride_w, int32_t stride_h,
int32_t padding_w,
int32_t padding_h, int32_t dilation_w,
int32_t dilation_h,
- int32_t clip_min, int32_t clip_max) {
- std::string struct_name = "conv_params";
+ int32_t clip_min, int32_t clip_max,
int32_t depth_multiplier) {
+ std::string struct_name = "cmsis_nn_conv_params";
+ std::string instance_name = "conv_params";
+ if (depth_multiplier != -1) {
+ struct_name = "cmsis_nn_dw_conv_params";
+ }
Review comment:
Minor suggestion, keep manipulation of the same variable close together:
```suggestion
std::string instance_name = "conv_params";
std::string struct_name = "cmsis_nn_conv_params";
if (depth_multiplier != -1) {
struct_name = "cmsis_nn_dw_conv_params";
}
```
##########
File path: src/relay/backend/contrib/cmsisnn/relay_to_tir.cc
##########
@@ -173,18 +168,42 @@ class RelayToTIRVisitor : public MixedModeMutator {
Array<PrimExpr> input_shape =
conv2d_call->args[0]->type_as<TensorTypeNode>()->shape;
Array<PrimExpr> input_dims = CMSISNNDimensions(input_shape);
- // cmsis_nn_dims *filter_dims (OHWI)
+ // cmsis_nn_dims *filter_dims (OHWI for Conv2D and IHWO for depthwise)
Array<PrimExpr> filter_shape =
conv2d_call->args[1]->type_as<TensorTypeNode>()->shape;
Array<PrimExpr> filter_dims = CMSISNNDimensions(filter_shape);
- // cmsis_nn_dims *bias_dims (1,1,1,output_channels)
- Array<PrimExpr> bias_shape{1, 1, 1, filter_shape[0]};
+ // cmsis_nn_dims *bias_dims
+ Array<PrimExpr> bias_shape{1, 1, 1, out_channels};
Array<PrimExpr> bias_dims = CMSISNNDimensions(bias_shape);
- // cmsis_nn_dims *output_dims (NHWC)
+ // cmsis_nn_dims *output_dims (same order as input_dims)
Array<PrimExpr> output_shape =
conv2d_call->type_as<TensorTypeNode>()->shape;
Array<PrimExpr> output_dims = CMSISNNDimensions(output_shape);
+ int32_t depth_multiplier = -1;
+ int kernel_pos_o = kernel_layout.find("O");
+ if (groups == qnn::get_const_int(input_shape[3]) &&
Review comment:
Is it worth having a `IsDepthwise` function to re-use this logic?
--
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]
