You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/10/21 08:44:11 UTC

[GitHub] [tvm] Mousius opened a new pull request #9338: [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Mousius opened a new pull request #9338:
URL: https://github.com/apache/tvm/pull/9338


   This correctly calculates the buffer sizes for a variety of targets based on the `-mcpu` and `-mattr` flags passed to the `cmsis-nn` code generator.
   
   Currently this is stacked on https://github.com/apache/tvm/pull/9331 which is dependent on https://github.com/apache/tvm/issues/9296


-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734413965



##########
File path: tests/cpp/relay/backend/contrib/cmsisnn/compiler_attrs_test.cc
##########
@@ -0,0 +1,125 @@
+/*

Review comment:
       I don't think the `tvmc` tests actually compile anything into a model?

##########
File path: tests/cpp/relay/backend/contrib/cmsisnn/compiler_attrs_test.cc
##########
@@ -0,0 +1,125 @@
+/*

Review comment:
       I don't think the `tvmc` tests actually compile anything into a runnable model?




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] ashutosh-arm commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734381363



##########
File path: src/relay/backend/contrib/cmsisnn/compiler_attrs.cc
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include "compiler_attrs.h"
+
+#include <tvm/ir/attrs.h>
+#include <tvm/ir/transform.h>
+
+#include <string>
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
+
+static const char* mveCPUs[] = {"cortex-m55"};
+static const char* dspCPUs[] = {"cortex-m7", "cortex-m33", "cortex-m35p"};
+
+TVM_REGISTER_NODE_TYPE(CMSISNNCompilerConfigNode);
+TVM_REGISTER_PASS_CONFIG_OPTION("relay.ext.cmsisnn.options", CMSISNNCompilerConfig);
+
+template <typename Container>
+static inline bool MatchesCpu(std::string mcpu, const Container& cpus) {
+  auto matches_cpu = [mcpu](const char* cpu) { return mcpu.find(cpu) == 0; };
+  return std::find_if(std::begin(cpus), std::end(cpus), matches_cpu) != std::end(cpus);
+}
+
+static inline bool HasFlag(std::string attr, std::string flag) {
+  return attr.find(flag) != std::string::npos;
+}
+
+CMSISNNFlags GetCompilerFlags(const tvm::transform::PassContext& ctx) {
+  auto cfg = ctx->GetConfig<CMSISNNCompilerConfig>("relay.ext.cmsisnn.options");
+  if (!cfg.defined()) {
+    return {false, false};

Review comment:
       Can we use variable names instead of {false,true} etc for clarity?

##########
File path: tests/cpp/relay/backend/contrib/cmsisnn/compiler_attrs_test.cc
##########
@@ -0,0 +1,125 @@
+/*

Review comment:
       should we add tvmc tests for some combinations of attrs as well?

##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -140,6 +140,11 @@ class AOTTestRunner(NamedTuple):
     """,
     includes=["uart.h"],
     parameters={"NPU_VARIANT": "256"},
+    pass_config={

Review comment:
       For my understanding, how are the flags (mcpu, mattr) being passed down to the fvp? Based on the mattr the op implementations do change.

##########
File path: src/relay/backend/contrib/cmsisnn/buffer_size.cc
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/ir/attrs.h>
+#include <tvm/ir/transform.h>
+
+#include "compiler_attrs.h"
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
+
+int Conv2dBufferSize(CMSISNNFlags flags, int32_t padding_w, int32_t padding_h, int32_t input_n,

Review comment:
       CMSISNNDimensions proposed in #9331 can be used to reduce the list of args 




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r776377681



##########
File path: src/relay/backend/contrib/cmsisnn/buffer_size.cc
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/ir/attrs.h>
+#include <tvm/ir/transform.h>
+
+#include "compiler_attrs.h"
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
+
+int Conv2dBufferSize(CMSISNNFlags flags, int32_t padding_w, int32_t padding_h, int32_t input_n,

Review comment:
       These are currently only available in `tir_to_runtime.cc`, something we can come back to though - the further buffer functions use far less parameters though.




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] ashutosh-arm commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate CMSIS-NN buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r778221116



##########
File path: tests/cpp/relay/backend/contrib/cmsisnn/buffer_size_test.cc
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_USE_CMSISNN
+
+#include "../../../../../../src/relay/backend/contrib/cmsisnn/buffer_size.h"
+
+#include <gtest/gtest.h>
+#include <tvm/ir/transform.h>
+
+#include <cmath>
+#include <random>
+#include <string>
+
+#include "../../../../../../src/relay/backend/contrib/cmsisnn/compiler_attrs.h"
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
+
+static std::random_device rd;
+static std::mt19937 gen(rd());
+static std::uniform_int_distribution<> fake_parameters(1, 100);
+
+class CMSISNNCalculatedBufferSize : public testing::TestWithParam<std::array<int32_t, 3>> {};
+
+TEST(CMSISNNConv2dBufferSize, Conv1x1) {
+  int32_t any = fake_parameters(gen);
+  auto conv2d_1x1 = [=](CMSISNNFlags flags, int32_t input_c) {
+    return Conv2dBufferSize(flags, 0, 0, any, any, input_c, any, any, 1, 1, 1, 1);
+  };
+
+  ASSERT_EQ(conv2d_1x1(kNoExt, 4), 0);
+  ASSERT_EQ(conv2d_1x1(kNoExt, 8), 0);
+  ASSERT_EQ(conv2d_1x1(kNoExt, 12), 0);
+  ASSERT_EQ(conv2d_1x1(kNoExt, 16), 0);
+  ASSERT_EQ(conv2d_1x1(kNoExt, 32), 0);
+
+  ASSERT_EQ(conv2d_1x1(kHasDSP, 4), 0);
+  ASSERT_EQ(conv2d_1x1(kHasDSP, 8), 0);
+  ASSERT_EQ(conv2d_1x1(kHasDSP, 12), 0);
+  ASSERT_EQ(conv2d_1x1(kHasDSP, 16), 0);
+  ASSERT_EQ(conv2d_1x1(kHasDSP, 32), 0);
+
+  ASSERT_EQ(conv2d_1x1(kHasMVE, 4), 0);
+  ASSERT_EQ(conv2d_1x1(kHasMVE, 8), 0);
+  ASSERT_EQ(conv2d_1x1(kHasMVE, 12), 0);
+  ASSERT_EQ(conv2d_1x1(kHasMVE, 16), 0);
+  ASSERT_EQ(conv2d_1x1(kHasMVE, 32), 0);
+}
+
+TEST(CMSISNNConv2dBufferSize, Conv1xN) {
+  int32_t any = fake_parameters(gen);
+  int32_t input_c = fake_parameters(gen);
+  int32_t filter_w = fake_parameters(gen);
+  int32_t filter_h = 1;
+  int32_t calculated_buffer = (2 * input_c * filter_w * filter_h) * (int32_t)sizeof(int16_t);
+
+  auto conv2d_1xn = [=](CMSISNNFlags flags, int32_t output_w) {
+    return Conv2dBufferSize(flags, any, any, 1, 1, input_c, 1, output_w, any, any, filter_w,
+                            filter_h);
+  };
+
+  ASSERT_EQ(conv2d_1xn(kNoExt, 4), 0);
+  ASSERT_EQ(conv2d_1xn(kNoExt, 8), 0);
+  ASSERT_EQ(conv2d_1xn(kNoExt, 12), 0);
+  ASSERT_EQ(conv2d_1xn(kNoExt, 16), 0);
+  ASSERT_EQ(conv2d_1xn(kNoExt, 32), 0);
+
+  ASSERT_EQ(conv2d_1xn(kHasDSP, 4), calculated_buffer);
+  ASSERT_EQ(conv2d_1xn(kHasDSP, 8), calculated_buffer);
+  ASSERT_EQ(conv2d_1xn(kHasDSP, 12), calculated_buffer);
+  ASSERT_EQ(conv2d_1xn(kHasDSP, 16), calculated_buffer);
+  ASSERT_EQ(conv2d_1xn(kHasDSP, 32), calculated_buffer);
+
+  ASSERT_EQ(conv2d_1xn(kHasMVE, 4), 0);
+  ASSERT_EQ(conv2d_1xn(kHasMVE, 8), 0);
+  ASSERT_EQ(conv2d_1xn(kHasMVE, 12), 0);
+  ASSERT_EQ(conv2d_1xn(kHasMVE, 16), 0);
+  ASSERT_EQ(conv2d_1xn(kHasMVE, 32), 0);
+}
+
+TEST(CMSISNNConv2dBufferSize, Default) {
+  int32_t any = fake_parameters(gen);
+
+  int32_t input_c = fake_parameters(gen);
+  int32_t filter_w = fake_parameters(gen);
+  int32_t filter_h = fake_parameters(gen);
+  int32_t calculated_buffer = (2 * input_c * filter_w * filter_h) * (int32_t)sizeof(int16_t);
+
+  auto conv2d = [=](CMSISNNFlags flags, int32_t output_w) {
+    return Conv2dBufferSize(flags, any, any, 1, 1, input_c, 1, output_w, any, any, filter_w,
+                            filter_h);
+  };
+
+  ASSERT_EQ(conv2d(kNoExt, 4), 0);
+  ASSERT_EQ(conv2d(kNoExt, 8), 0);
+  ASSERT_EQ(conv2d(kNoExt, 12), 0);
+  ASSERT_EQ(conv2d(kNoExt, 16), 0);
+  ASSERT_EQ(conv2d(kNoExt, 32), 0);
+
+  ASSERT_EQ(conv2d(kHasDSP, 4), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasDSP, 8), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasDSP, 12), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasDSP, 16), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasDSP, 32), calculated_buffer);
+
+  ASSERT_EQ(conv2d(kHasMVE, 4), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasMVE, 8), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasMVE, 12), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasMVE, 16), calculated_buffer);
+  ASSERT_EQ(conv2d(kHasMVE, 32), calculated_buffer);
+}
+
+TEST(CMSISNNDepthwiseConv2dBufferSize, UnEvenChannels) {
+  int32_t filter_w = fake_parameters(gen);
+  int32_t filter_h = fake_parameters(gen);
+  int32_t input_n = 1;
+
+  auto depthwise_conv2d_with_channels = [=](CMSISNNFlags flags, int32_t input_c, int32_t output_c) {
+    return DepthwiseConv2dBufferSize(flags, input_n, input_c, output_c, filter_w, filter_h);
+  };
+
+  ASSERT_EQ(depthwise_conv2d_with_channels(kNoExt, 4, 6), 0);
+  ASSERT_EQ(depthwise_conv2d_with_channels(kNoExt, 8, 7), 0);
+  ASSERT_EQ(depthwise_conv2d_with_channels(kHasDSP, 4, 6), 0);
+  ASSERT_EQ(depthwise_conv2d_with_channels(kHasDSP, 8, 7), 0);
+  ASSERT_EQ(depthwise_conv2d_with_channels(kHasMVE, 4, 6), 0);
+  ASSERT_EQ(depthwise_conv2d_with_channels(kHasMVE, 8, 7), 0);

Review comment:
       I like it that you have included multiples of 4 cases here for channel :smile: 




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734499115



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -140,6 +140,11 @@ class AOTTestRunner(NamedTuple):
     """,
     includes=["uart.h"],
     parameters={"NPU_VARIANT": "256"},
+    pass_config={

Review comment:
       Only if we want to manage multiple artefacts for CMSIS-NN, we can't add them to this PR as is.




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on pull request #9338: [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#issuecomment-948391946


   CC @ashutosh-arm @manupa-arm 


-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734543003



##########
File path: tests/cpp/relay/backend/contrib/cmsisnn/compiler_attrs_test.cc
##########
@@ -0,0 +1,125 @@
+/*

Review comment:
       Analysing the C output files from tvm isn't a clean way of testing this. The unit tests could cover cases of both in combination though, it'd match both strings and produce the most disabling one - I can add that.




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r776377971



##########
File path: src/relay/backend/contrib/cmsisnn/compiler_attrs.cc
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include "compiler_attrs.h"
+
+#include <tvm/ir/attrs.h>
+#include <tvm/ir/transform.h>
+
+#include <string>
+
+namespace tvm {
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
+
+static const char* mveCPUs[] = {"cortex-m55"};
+static const char* dspCPUs[] = {"cortex-m7", "cortex-m33", "cortex-m35p"};
+
+TVM_REGISTER_NODE_TYPE(CMSISNNCompilerConfigNode);
+TVM_REGISTER_PASS_CONFIG_OPTION("relay.ext.cmsisnn.options", CMSISNNCompilerConfig);
+
+template <typename Container>
+static inline bool MatchesCpu(std::string mcpu, const Container& cpus) {
+  auto matches_cpu = [mcpu](const char* cpu) { return mcpu.find(cpu) == 0; };
+  return std::find_if(std::begin(cpus), std::end(cpus), matches_cpu) != std::end(cpus);
+}
+
+static inline bool HasFlag(std::string attr, std::string flag) {
+  return attr.find(flag) != std::string::npos;
+}
+
+CMSISNNFlags GetCompilerFlags(const tvm::transform::PassContext& ctx) {
+  auto cfg = ctx->GetConfig<CMSISNNCompilerConfig>("relay.ext.cmsisnn.options");
+  if (!cfg.defined()) {
+    return {false, false};

Review comment:
       Re-used the constants here to clarify it.




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] ashutosh-arm commented on pull request #9338: [4a/10] [CMSIS-NN] Calculate CMSIS-NN buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#issuecomment-1004960805


   Thanks @Mousius for this support and also taking care of adding the buffer support for all the other layers that needed buffer size calculations.


-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] leandron merged pull request #9338: [4a/10] [CMSIS-NN] Calculate CMSIS-NN buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
leandron merged pull request #9338:
URL: https://github.com/apache/tvm/pull/9338


   


-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] ashutosh-arm commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734424043



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -140,6 +140,11 @@ class AOTTestRunner(NamedTuple):
     """,
     includes=["uart.h"],
     parameters={"NPU_VARIANT": "256"},
+    pass_config={

Review comment:
       So should we add +nodsp and +nomve in op tests for testing functional correctness across all flows?




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] Mousius commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734418005



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -140,6 +140,11 @@ class AOTTestRunner(NamedTuple):
     """,
     includes=["uart.h"],
     parameters={"NPU_VARIANT": "256"},
+    pass_config={

Review comment:
       The flags here decide on the buffer sizes in TVM, in the precompile for CMSIS-NN it passes `-mcpu=cortex-m55` which results in that configuration for that device for the libraries linked in the tests. This is why I've covered this in unit tests, our reference system tests cover `-mcpu=cortex-m55` end to end but we test the buffer size is calculated properly for various architectures. The best we could potentially do here is do `+nodsp` and `+nomve` with `cortex-m55` but not the other CPUs, and that'd require precompiling those variants of CMSIS-NN in the container.




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] ashutosh-arm commented on a change in pull request #9338: [4a/10] [CMSIS-NN] Calculate Conv2d buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#discussion_r734420148



##########
File path: tests/cpp/relay/backend/contrib/cmsisnn/compiler_attrs_test.cc
##########
@@ -0,0 +1,125 @@
+/*

Review comment:
       The generated api does not change, but context buffer size might change based on the flags supplied. For a particular combination of network params, we could check on the buffer size in the tvmc output? or is it too much testing? :thinking: 
   
   Another thing is how are we going to check that the output for variety of combinations of mattrs is correct? 




-- 
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: commits-unsubscribe@tvm.apache.org

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



[GitHub] [tvm] ashutosh-arm edited a comment on pull request #9338: [4a/10] [CMSIS-NN] Calculate CMSIS-NN buffer size with respect to architecture extensions

Posted by GitBox <gi...@apache.org>.
ashutosh-arm edited a comment on pull request #9338:
URL: https://github.com/apache/tvm/pull/9338#issuecomment-1004960805


   Thanks @Mousius for this support and also taking care of adding the buffer support for all the other layers that needed buffer size calculations. LGTM!


-- 
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: commits-unsubscribe@tvm.apache.org

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