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 2020/09/30 22:49:27 UTC

[GitHub] [incubator-tvm] areusch opened a new pull request #6603: Add µTVM Zephyr support + QEMU regression test

areusch opened a new pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603


   This PR adds support for Zephyr using QEMU (support for physical hardware to come in the next PR). It also adds a QEMU-based regression test to validate most of the Zephyr pipeline.
   
   The ci-qemu dockerfile was added in #6485.
   
   @tqchen @tom-gall @u99127 @liangfu 


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



[GitHub] [incubator-tvm] areusch commented on pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#issuecomment-708632486


   thanks @liangfu, please take another look when you have a minute


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



[GitHub] [incubator-tvm] liangfu merged pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu merged pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603


   


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



[GitHub] [incubator-tvm] liangfu commented on a change in pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu commented on a change in pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#discussion_r505293470



##########
File path: tests/python/unittest/test_micro_artifact.py
##########
@@ -0,0 +1,137 @@
+# 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.
+
+"""Unit tests for the artifact module."""
+
+import json
+import os
+import shutil
+
+from tvm.contrib import util
+from tvm.micro import artifact
+
+
+FILE_LIST = ["label1", "label2", "label12", "unlabelled"]
+
+
+TEST_METADATA = {"foo": "bar"}
+
+
+TEST_LABELS = {"label1": ["label1", "label12"], "label2": ["label2", "label12"]}
+
+
+def build_artifact(artifact_path, immobile=False):
+    os.mkdir(artifact_path)
+
+    for f in FILE_LIST:
+        with open(os.path.join(artifact_path, f), "w") as lib_f:
+            lib_f.write(f"{f}\n")
+
+    sub_dir = os.path.join(artifact_path, "sub_dir")
+    os.mkdir(sub_dir)
+    os.symlink("label1", os.path.join(artifact_path, "rel_symlink"))
+    os.symlink("label2", os.path.join(artifact_path, "abs_symlink"), "label2")
+    os.symlink(
+        os.path.join(artifact_path, "sub_dir"), os.path.join(artifact_path, "abs_dir_symlink")
+    )
+
+    art = artifact.Artifact(artifact_path, TEST_LABELS, TEST_METADATA, immobile=immobile)
+
+    return art
+
+
+def test_basic_functionality():
+    temp_dir = util.tempdir()
+    artifact_path = temp_dir.relpath("foo")
+    art = build_artifact(artifact_path)
+
+    assert art.abspath("bar") == os.path.join(artifact_path, "bar")
+
+    for label, paths in TEST_LABELS.items():
+        assert art.label(label) == paths
+        assert art.label_abspath(label) == [os.path.join(artifact_path, p) for p in paths]
+
+
+def test_archive():
+    temp_dir = util.tempdir()
+    art = build_artifact(temp_dir.relpath("foo"))
+
+    # Create archive
+    archive_path = art.archive(temp_dir.temp_dir)
+    assert archive_path == temp_dir.relpath("foo.tar")
+
+    # Inspect created archive
+    unpack_dir = temp_dir.relpath("unpack")
+    os.mkdir(unpack_dir)
+    shutil.unpack_archive(archive_path, unpack_dir)
+
+    for path in FILE_LIST:
+        with open(os.path.join(unpack_dir, "foo", path)) as f:
+            assert f.read() == f"{path}\n"
+
+    with open(os.path.join(unpack_dir, "foo", "metadata.json")) as metadata_f:
+        metadata = json.load(metadata_f)
+
+    assert metadata["version"] == 2
+    assert metadata["labelled_files"] == TEST_LABELS
+    assert metadata["metadata"] == TEST_METADATA
+
+    # Unarchive and verify basic functionality
+    unarchive_base_dir = temp_dir.relpath("unarchive")
+    unarch = artifact.Artifact.unarchive(archive_path, unarchive_base_dir)
+
+    assert unarch.metadata == TEST_METADATA
+    assert unarch.labelled_files == TEST_LABELS
+    for f in FILE_LIST:
+        assert os.path.exists(os.path.join(unarchive_base_dir, f))
+
+
+def test_metadata_only():
+    temp_dir = util.tempdir()
+    base_dir = temp_dir.relpath("foo")
+    art = build_artifact(base_dir)
+
+    artifact_path = art.archive(temp_dir.relpath("foo.artifact"), metadata_only=True)
+    unarch_base_dir = temp_dir.relpath("bar")
+    unarch = artifact.Artifact.unarchive(artifact_path, unarch_base_dir)
+    assert unarch.base_dir == base_dir
+
+    for p in unarch.label_abspath("label1") + unarch.label_abspath("label2"):
+        assert os.path.exists(p)
+
+    os.unlink(art.abspath("label1"))
+    with open(art.abspath("label2"), "w+") as f:
+        f.write("changed line\n")
+
+    try:
+        artifact.Artifact.unarchive(artifact_path, os.path.join(temp_dir.temp_dir, "bar2"))
+        assert False, "unarchive should raise error"
+    except artifact.ArchiveModifiedError as err:
+        assert str(err) == (
+            "Files in metadata-only archive have been modified:\n"
+            " * label1: original file not found\n"
+            " * label2: sha256 mismatch: expected "
+            "6aa3c5668c8794c791400e19ecd7123949ded1616eafb0395acdd2d896354e83, got "
+            "ed87db21670a81819d65eccde87c5ae0243b2b61783bf77e9b27993be9a3eca0"

Review comment:
       Just curious, why do we need to hard code these hash here?




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



[GitHub] [incubator-tvm] liangfu commented on pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu commented on pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#issuecomment-706170097


   Also, please rebase latest master branch and resolve conflicts. Thanks!


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



[GitHub] [incubator-tvm] liangfu commented on a change in pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu commented on a change in pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#discussion_r502392723



##########
File path: python/tvm/micro/contrib/zephyr.py
##########
@@ -0,0 +1,621 @@
+# 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.
+
+"""Defines a compiler integration that uses an externally-supplied Zephyr project."""
+
+import collections
+import logging
+import multiprocessing
+import os
+import re
+import tempfile
+import termios
+import textwrap
+import signal
+import shlex
+import shutil
+import subprocess
+import sys
+
+import yaml
+
+import tvm.micro
+from . import base
+from .. import compiler
+from .. import debugger
+from ..transport import debug
+from ..transport import file_descriptor
+
+# from ..transport import serial

Review comment:
       clean up

##########
File path: tests/micro/qemu/zephyr-runtime/CMakeLists.txt
##########
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+
+set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
+
+set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+#get_filename_component(QEMU_PID_ABSPATH "./qemu.pid" ABSOLUTE)
+#set(QEMU_PID_ABSPATH "/tmp/qemu.pid")
+#file(TOUCH "${QEMU_PID_ABSPATH}")
+#list(APPEND QEMU_EXTRA_FLAGS "-pidfile" "${QEMU_PID_ABSPATH}")
+
+find_package(Zephyr HINTS $ENV{ZEPHYR_BASE})
+project(hello_world)

Review comment:
       I think the project deserve a better name.

##########
File path: tests/micro/qemu/zephyr-runtime/CMakeLists.txt
##########
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+
+set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
+
+set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+#get_filename_component(QEMU_PID_ABSPATH "./qemu.pid" ABSOLUTE)

Review comment:
       clean up?

##########
File path: tests/micro/qemu/zephyr-runtime/crt/crt_config.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/crt_config.h.template
+ * \brief Template for CRT configuration, to be modified on each target.
+ */
+#ifndef TVM_RUNTIME_CRT_CONFIG_H_
+#define TVM_RUNTIME_CRT_CONFIG_H_
+
+/*! Log level of the CRT runtime */
+#define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG

Review comment:
       I think we may need to include the header where `TVM_CRT_LOG_LEVEL_DEBUG` is defined, otherwise we might encounter errors with incorrect include sequence.

##########
File path: src/runtime/crt/utvm_rpc_server/rpc_server.cc
##########
@@ -126,25 +125,33 @@ class MicroRPCServer {
 
   /*! \brief Process one message from the receive buffer, if possible.
    *
-   * \return true if additional messages could be processed. false if the server shutdown request
-   * has been received.
+   * \param new_data If not nullptr, a pointer to a buffer pointer, which should point at new input
+   *     data to process. On return, updated to point past data that has been consumed.
+   * \param new_data_size_bytes Points to the number of valid bytes in `new_data`. On return,
+   *     updated to the number of unprocessed bytes remaining in `new_data` (usually 0).
+   * \return an error code indicating the outcome of the processing loop.
    */
-  bool Loop() {
-    if (has_pending_byte_) {
-      size_t bytes_consumed;
-      CHECK_EQ(unframer_.Write(&pending_byte_, 1, &bytes_consumed), kTvmErrorNoError,
-               "unframer_.Write");
-      CHECK_EQ(bytes_consumed, 1, "bytes_consumed");
-      has_pending_byte_ = false;
+  tvm_crt_error_t Loop(uint8_t** new_data, size_t* new_data_size_bytes) {
+    if (!is_running_) {
+      return kTvmErrorPlatformShutdown;
     }
 
-    return is_running_;
-  }
+    tvm_crt_error_t err = kTvmErrorNoError;
+    //    printf("loop %d %02x\n", *new_data_size_bytes,  **new_data);
+    if (new_data != nullptr && new_data_size_bytes != nullptr && *new_data_size_bytes > 0) {
+      size_t bytes_consumed;
+      err = unframer_.Write(*new_data, *new_data_size_bytes, &bytes_consumed);
+      *new_data += bytes_consumed;
+      *new_data_size_bytes -= bytes_consumed;
+    }
 
-  void HandleReceivedByte(uint8_t byte) {
-    CHECK(!has_pending_byte_);
-    has_pending_byte_ = true;
-    pending_byte_ = byte;
+    if (err != kTvmErrorNoError) {
+      return err;
+    } else if (is_running_) {
+      return kTvmErrorNoError;
+    } else {
+      return kTvmErrorPlatformShutdown;

Review comment:
       Keep only one return statement when possible.

##########
File path: tests/micro/qemu/zephyr-runtime/crt/crt_config.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/crt_config.h.template
+ * \brief Template for CRT configuration, to be modified on each target.
+ */
+#ifndef TVM_RUNTIME_CRT_CONFIG_H_
+#define TVM_RUNTIME_CRT_CONFIG_H_
+
+/*! Log level of the CRT runtime */
+#define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG
+
+/*! Maximum supported dimension in NDArray */
+#define TVM_CRT_MAX_NDIM 6
+
+/*! Maximum supported arguments in generated functions */
+#define TVM_CRT_MAX_ARGS 10
+
+/*! Size of the global function registry, in bytes. */
+#define TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES 200
+
+/*! Maximum number of registered modules. */
+#define TVM_CRT_MAX_REGISTERED_MODULES 2
+
+/*! Maximum packet size, in bytes, including the length header. */
+#define TVM_CRT_MAX_PACKET_SIZE_BYTES 8192
+
+/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
+#define TVM_CRT_MAX_STRLEN_DLTYPE 10
+
+/*! Maximum supported string length in function names */
+#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
+
+/*! \brief Maximum length of a PackedFunc function name. */
+#define TVM_CRT_MAX_FUNCTION_NAME_LENGTH_BYTES 30
+
+/*! \brief Log2 of the page size (bytes) for a virtual memory page. */
+#define TVM_CRT_PAGE_BITS 10  // 1 kB
+
+/*! \brief Number of pages on device. */
+#define TVM_CRT_MAX_PAGES 300
+
+//#define TVM_CRT_FRAMER_ENABLE_LOGS

Review comment:
       clean up?




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



[GitHub] [incubator-tvm] tqchen commented on pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#issuecomment-703838421


   cc @tmoreau89 , please sync push to ci-docker-staging to confirm if docker-staging passes


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



[GitHub] [incubator-tvm] areusch commented on a change in pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#discussion_r504941656



##########
File path: src/runtime/crt/utvm_rpc_server/rpc_server.cc
##########
@@ -126,25 +125,33 @@ class MicroRPCServer {
 
   /*! \brief Process one message from the receive buffer, if possible.
    *
-   * \return true if additional messages could be processed. false if the server shutdown request
-   * has been received.
+   * \param new_data If not nullptr, a pointer to a buffer pointer, which should point at new input
+   *     data to process. On return, updated to point past data that has been consumed.
+   * \param new_data_size_bytes Points to the number of valid bytes in `new_data`. On return,
+   *     updated to the number of unprocessed bytes remaining in `new_data` (usually 0).
+   * \return an error code indicating the outcome of the processing loop.
    */
-  bool Loop() {
-    if (has_pending_byte_) {
-      size_t bytes_consumed;
-      CHECK_EQ(unframer_.Write(&pending_byte_, 1, &bytes_consumed), kTvmErrorNoError,
-               "unframer_.Write");
-      CHECK_EQ(bytes_consumed, 1, "bytes_consumed");
-      has_pending_byte_ = false;
+  tvm_crt_error_t Loop(uint8_t** new_data, size_t* new_data_size_bytes) {
+    if (!is_running_) {
+      return kTvmErrorPlatformShutdown;
     }
 
-    return is_running_;
-  }
+    tvm_crt_error_t err = kTvmErrorNoError;
+    //    printf("loop %d %02x\n", *new_data_size_bytes,  **new_data);
+    if (new_data != nullptr && new_data_size_bytes != nullptr && *new_data_size_bytes > 0) {
+      size_t bytes_consumed;
+      err = unframer_.Write(*new_data, *new_data_size_bytes, &bytes_consumed);
+      *new_data += bytes_consumed;
+      *new_data_size_bytes -= bytes_consumed;
+    }
 
-  void HandleReceivedByte(uint8_t byte) {
-    CHECK(!has_pending_byte_);
-    has_pending_byte_ = true;
-    pending_byte_ = byte;
+    if (err != kTvmErrorNoError) {
+      return err;
+    } else if (is_running_) {
+      return kTvmErrorNoError;
+    } else {
+      return kTvmErrorPlatformShutdown;

Review comment:
       made this clearer now




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



[GitHub] [incubator-tvm] liangfu commented on a change in pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu commented on a change in pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#discussion_r502392723



##########
File path: python/tvm/micro/contrib/zephyr.py
##########
@@ -0,0 +1,621 @@
+# 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.
+
+"""Defines a compiler integration that uses an externally-supplied Zephyr project."""
+
+import collections
+import logging
+import multiprocessing
+import os
+import re
+import tempfile
+import termios
+import textwrap
+import signal
+import shlex
+import shutil
+import subprocess
+import sys
+
+import yaml
+
+import tvm.micro
+from . import base
+from .. import compiler
+from .. import debugger
+from ..transport import debug
+from ..transport import file_descriptor
+
+# from ..transport import serial

Review comment:
       clean up

##########
File path: tests/micro/qemu/zephyr-runtime/CMakeLists.txt
##########
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+
+set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
+
+set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+#get_filename_component(QEMU_PID_ABSPATH "./qemu.pid" ABSOLUTE)
+#set(QEMU_PID_ABSPATH "/tmp/qemu.pid")
+#file(TOUCH "${QEMU_PID_ABSPATH}")
+#list(APPEND QEMU_EXTRA_FLAGS "-pidfile" "${QEMU_PID_ABSPATH}")
+
+find_package(Zephyr HINTS $ENV{ZEPHYR_BASE})
+project(hello_world)

Review comment:
       I think the project deserve a better name.

##########
File path: tests/micro/qemu/zephyr-runtime/CMakeLists.txt
##########
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+
+set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
+
+set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+#get_filename_component(QEMU_PID_ABSPATH "./qemu.pid" ABSOLUTE)

Review comment:
       clean up?

##########
File path: tests/micro/qemu/zephyr-runtime/crt/crt_config.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/crt_config.h.template
+ * \brief Template for CRT configuration, to be modified on each target.
+ */
+#ifndef TVM_RUNTIME_CRT_CONFIG_H_
+#define TVM_RUNTIME_CRT_CONFIG_H_
+
+/*! Log level of the CRT runtime */
+#define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG

Review comment:
       I think we may need to include the header where `TVM_CRT_LOG_LEVEL_DEBUG` is defined, otherwise we might encounter errors with incorrect include sequence.

##########
File path: src/runtime/crt/utvm_rpc_server/rpc_server.cc
##########
@@ -126,25 +125,33 @@ class MicroRPCServer {
 
   /*! \brief Process one message from the receive buffer, if possible.
    *
-   * \return true if additional messages could be processed. false if the server shutdown request
-   * has been received.
+   * \param new_data If not nullptr, a pointer to a buffer pointer, which should point at new input
+   *     data to process. On return, updated to point past data that has been consumed.
+   * \param new_data_size_bytes Points to the number of valid bytes in `new_data`. On return,
+   *     updated to the number of unprocessed bytes remaining in `new_data` (usually 0).
+   * \return an error code indicating the outcome of the processing loop.
    */
-  bool Loop() {
-    if (has_pending_byte_) {
-      size_t bytes_consumed;
-      CHECK_EQ(unframer_.Write(&pending_byte_, 1, &bytes_consumed), kTvmErrorNoError,
-               "unframer_.Write");
-      CHECK_EQ(bytes_consumed, 1, "bytes_consumed");
-      has_pending_byte_ = false;
+  tvm_crt_error_t Loop(uint8_t** new_data, size_t* new_data_size_bytes) {
+    if (!is_running_) {
+      return kTvmErrorPlatformShutdown;
     }
 
-    return is_running_;
-  }
+    tvm_crt_error_t err = kTvmErrorNoError;
+    //    printf("loop %d %02x\n", *new_data_size_bytes,  **new_data);
+    if (new_data != nullptr && new_data_size_bytes != nullptr && *new_data_size_bytes > 0) {
+      size_t bytes_consumed;
+      err = unframer_.Write(*new_data, *new_data_size_bytes, &bytes_consumed);
+      *new_data += bytes_consumed;
+      *new_data_size_bytes -= bytes_consumed;
+    }
 
-  void HandleReceivedByte(uint8_t byte) {
-    CHECK(!has_pending_byte_);
-    has_pending_byte_ = true;
-    pending_byte_ = byte;
+    if (err != kTvmErrorNoError) {
+      return err;
+    } else if (is_running_) {
+      return kTvmErrorNoError;
+    } else {
+      return kTvmErrorPlatformShutdown;

Review comment:
       Keep only one return statement when possible.

##########
File path: tests/micro/qemu/zephyr-runtime/crt/crt_config.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/crt_config.h.template
+ * \brief Template for CRT configuration, to be modified on each target.
+ */
+#ifndef TVM_RUNTIME_CRT_CONFIG_H_
+#define TVM_RUNTIME_CRT_CONFIG_H_
+
+/*! Log level of the CRT runtime */
+#define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG
+
+/*! Maximum supported dimension in NDArray */
+#define TVM_CRT_MAX_NDIM 6
+
+/*! Maximum supported arguments in generated functions */
+#define TVM_CRT_MAX_ARGS 10
+
+/*! Size of the global function registry, in bytes. */
+#define TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES 200
+
+/*! Maximum number of registered modules. */
+#define TVM_CRT_MAX_REGISTERED_MODULES 2
+
+/*! Maximum packet size, in bytes, including the length header. */
+#define TVM_CRT_MAX_PACKET_SIZE_BYTES 8192
+
+/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
+#define TVM_CRT_MAX_STRLEN_DLTYPE 10
+
+/*! Maximum supported string length in function names */
+#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
+
+/*! \brief Maximum length of a PackedFunc function name. */
+#define TVM_CRT_MAX_FUNCTION_NAME_LENGTH_BYTES 30
+
+/*! \brief Log2 of the page size (bytes) for a virtual memory page. */
+#define TVM_CRT_PAGE_BITS 10  // 1 kB
+
+/*! \brief Number of pages on device. */
+#define TVM_CRT_MAX_PAGES 300
+
+//#define TVM_CRT_FRAMER_ENABLE_LOGS

Review comment:
       clean up?




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



[GitHub] [incubator-tvm] liangfu commented on pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu commented on pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#issuecomment-706170097


   Also, please rebase latest master branch and resolve conflicts. Thanks!


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



[GitHub] [incubator-tvm] tqchen commented on pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#issuecomment-703837862


   @jroesch can you help manage the PR?


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



[GitHub] [incubator-tvm] areusch commented on a change in pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#discussion_r504941832



##########
File path: tests/micro/qemu/zephyr-runtime/crt/crt_config.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/crt_config.h.template
+ * \brief Template for CRT configuration, to be modified on each target.
+ */
+#ifndef TVM_RUNTIME_CRT_CONFIG_H_
+#define TVM_RUNTIME_CRT_CONFIG_H_
+
+/*! Log level of the CRT runtime */
+#define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG
+
+/*! Maximum supported dimension in NDArray */
+#define TVM_CRT_MAX_NDIM 6
+
+/*! Maximum supported arguments in generated functions */
+#define TVM_CRT_MAX_ARGS 10
+
+/*! Size of the global function registry, in bytes. */
+#define TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES 200
+
+/*! Maximum number of registered modules. */
+#define TVM_CRT_MAX_REGISTERED_MODULES 2
+
+/*! Maximum packet size, in bytes, including the length header. */
+#define TVM_CRT_MAX_PACKET_SIZE_BYTES 8192
+
+/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
+#define TVM_CRT_MAX_STRLEN_DLTYPE 10
+
+/*! Maximum supported string length in function names */
+#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
+
+/*! \brief Maximum length of a PackedFunc function name. */
+#define TVM_CRT_MAX_FUNCTION_NAME_LENGTH_BYTES 30
+
+/*! \brief Log2 of the page size (bytes) for a virtual memory page. */
+#define TVM_CRT_PAGE_BITS 10  // 1 kB
+
+/*! \brief Number of pages on device. */
+#define TVM_CRT_MAX_PAGES 300
+
+//#define TVM_CRT_FRAMER_ENABLE_LOGS

Review comment:
       actually I like to leave this in as it's a convenient debugging option that may not be discoverable otherwise

##########
File path: tests/micro/qemu/zephyr-runtime/crt/crt_config.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/runtime/crt_config.h.template
+ * \brief Template for CRT configuration, to be modified on each target.
+ */
+#ifndef TVM_RUNTIME_CRT_CONFIG_H_
+#define TVM_RUNTIME_CRT_CONFIG_H_
+
+/*! Log level of the CRT runtime */
+#define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG

Review comment:
       added #include

##########
File path: tests/micro/qemu/zephyr-runtime/CMakeLists.txt
##########
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+
+set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
+
+set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+#get_filename_component(QEMU_PID_ABSPATH "./qemu.pid" ABSOLUTE)
+#set(QEMU_PID_ABSPATH "/tmp/qemu.pid")
+#file(TOUCH "${QEMU_PID_ABSPATH}")
+#list(APPEND QEMU_EXTRA_FLAGS "-pidfile" "${QEMU_PID_ABSPATH}")
+
+find_package(Zephyr HINTS $ENV{ZEPHYR_BASE})
+project(hello_world)

Review comment:
       hah, changed :)

##########
File path: tests/micro/qemu/zephyr-runtime/CMakeLists.txt
##########
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+
+set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
+
+set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+#get_filename_component(QEMU_PID_ABSPATH "./qemu.pid" ABSOLUTE)

Review comment:
       done

##########
File path: python/tvm/micro/contrib/zephyr.py
##########
@@ -0,0 +1,621 @@
+# 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.
+
+"""Defines a compiler integration that uses an externally-supplied Zephyr project."""
+
+import collections
+import logging
+import multiprocessing
+import os
+import re
+import tempfile
+import termios
+import textwrap
+import signal
+import shlex
+import shutil
+import subprocess
+import sys
+
+import yaml
+
+import tvm.micro
+from . import base
+from .. import compiler
+from .. import debugger
+from ..transport import debug
+from ..transport import file_descriptor
+
+# from ..transport import serial

Review comment:
       done




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



[GitHub] [incubator-tvm] liangfu commented on pull request #6603: Add µTVM Zephyr support + QEMU regression test

Posted by GitBox <gi...@apache.org>.
liangfu commented on pull request #6603:
URL: https://github.com/apache/incubator-tvm/pull/6603#issuecomment-708971948


   Thanks @areusch @tqchen . This is now merged.


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