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/04/07 00:01:23 UTC

[GitHub] [tvm] mehrdadh opened a new pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

mehrdadh opened a new pull request #7804:
URL: https://github.com/apache/tvm/pull/7804


   This PR adds following changes:
   - Add target support for testing zephyr on riscv qemu
   - Add zephyr debugger for qemu
   - Add minor changes left from updating zephyr 2.5 https://github.com/apache/tvm/pull/7786
   
   Note: test_zephyr.py fully passes on qemu_riscv64 using the zephyr 2.5. However, for qemu_riscv32 you need to uncomment a line in **main.c** and update zephyr to latest master branch. This issue is mentioned on zephyr: https://github.com/zephyrproject-rtos/zephyr/issues/34026
   
   We will update this when we update our zephyr to next version.


-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -358,14 +358,18 @@ def test_command(args):
 
 
 def release_command(args):
+    vm_name = f"mehrdadh/microtvm-{args.platform}"
+    if args.platform == "zephyr":
+        vm_name = f"{vm_name}-{args.zephyr_version}"

Review comment:
       I changed it to platform_version and rework the input args with sub_parser.




-- 
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] [tvm] areusch commented on pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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


   @tqchen, this method unfortunately makes the review much harder because GH loses the old commits. If you're trying to decide whether an open comment has been addressed, it's really helpful to be able to be able to see the context which generated that comment and a diff. 
   
   I think the strategy adopted by me (and a few others) has been to rebase and force-push until a comment is placed on the PR, then do merges. If we want to consider improving the situation, I think we unfortunately may have to look into a different code review tool e.g. Gerrit (unless I misunderstand repository config settings on GH).


-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: tests/micro/zephyr/test_zephyr.py
##########
@@ -59,7 +66,7 @@ def _make_sess_from_op(model, zephyr_board, west_cmd, op_name, sched, arg_bufs):
 
 
 def _make_session(model, target, zephyr_board, west_cmd, mod):
-    test_name = f"{os.path.splitext(os.path.abspath(__file__))[0]}_{model}"
+    test_name = f"{os.path.splitext(os.path.abspath(__file__))[0]}_{zephyr_board}"

Review comment:
       @areusch  so we distinguish the test based on the zephyr_board. For ex. qemu_x86, qemu_riscv32 and qemu_riscv64 all use the same `model` which is `host`, but they have different zephyr_board. zephyr_board will always be unique.




-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/src/main.c
##########
@@ -265,6 +265,10 @@ static uint8_t main_rx_buf[RING_BUF_SIZE_BYTES];
 // The main function of this application.
 extern void __stdout_hook_install(int (*hook)(int));
 void main(void) {
+  // TODO (mehrdadh): Update this when zephyr version was updated to 2.6. 
+  // Uncomment this for qemu_riscv32, also update zephyr to latest version.

Review comment:
       that works, I think because it is an integer compare to using BOARD flag.




-- 
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] [tvm] tqchen commented on pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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


   BTW @mehrdadh here are some git tips that might be useful(try to rebase to bring the code back instead of merge) https://tvm.apache.org/docs/contribute/git_howto.html


-- 
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] [tvm] areusch merged pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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


   


-- 
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] [tvm] areusch edited a comment on pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

Posted by GitBox <gi...@apache.org>.
areusch edited a comment on pull request #7804:
URL: https://github.com/apache/tvm/pull/7804#issuecomment-815108225


   @tqchen, this method unfortunately makes the review much harder because GH loses the old commits after force-push. If you're trying to decide whether an open comment has been addressed, it's really helpful to be able to be able to see the context which generated that comment and a diff. 
   
   I think the strategy adopted by me (and a few others) has been to rebase and force-push until a comment is placed on the PR, then do merges. If we want to consider improving the situation, I think we unfortunately may have to look into a different code review tool e.g. Gerrit (unless I misunderstand repository config settings on GH).


-- 
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] [tvm] mehrdadh commented on pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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


   @tqchen Thanks for the tips! I will follow the guideline.


-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 

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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -358,14 +358,18 @@ def test_command(args):
 
 
 def release_command(args):
+    vm_name = f"mehrdadh/microtvm-{args.platform}"

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] [tvm] areusch commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 

Review comment:
       ok sounds good




-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -358,14 +358,18 @@ def test_command(args):
 
 
 def release_command(args):
+    vm_name = f"mehrdadh/microtvm-{args.platform}"
+    if args.platform == "zephyr":
+        vm_name = f"{vm_name}-{args.zephyr_version}"

Review comment:
       thanks for catching this!




-- 
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] [tvm] areusch commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: tests/micro/zephyr/test_zephyr.py
##########
@@ -59,7 +66,7 @@ def _make_sess_from_op(model, zephyr_board, west_cmd, op_name, sched, arg_bufs):
 
 
 def _make_session(model, target, zephyr_board, west_cmd, mod):
-    test_name = f"{os.path.splitext(os.path.abspath(__file__))[0]}_{model}"
+    test_name = f"{os.path.splitext(os.path.abspath(__file__))[0]}_{zephyr_board}"

Review comment:
       oh sorry, I was misreading. this looks good




-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/qemu-hack/qemu-system-riscv64
##########
@@ -0,0 +1,38 @@
+#!/bin/bash -e

Review comment:
       it worked!




-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 

Review comment:
       based on my understanding from their doc, the default it 512. [Here](https://docs.zephyrproject.org/2.5.0/reference/kconfig/CONFIG_MAIN_STACK_SIZE.html?highlight=config_main_stack_size#cmdoption-arg-CONFIG_MAIN_STACK_SIZE) it mentioned for RISCV is 512.




-- 
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] [tvm] mehrdadh commented on pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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


   cc @areusch 


-- 
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] [tvm] mehrdadh commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 
+CONFIG_MAIN_STACK_SIZE=2048
+
+# For floating point operations.

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] [tvm] areusch commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 
+CONFIG_MAIN_STACK_SIZE=2048
+
+# For floating point operations.

Review comment:
       could you add a note to explain which error you'll see if you don't include this?

##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -358,14 +358,18 @@ def test_command(args):
 
 
 def release_command(args):
+    vm_name = f"mehrdadh/microtvm-{args.platform}"
+    if args.platform == "zephyr":
+        vm_name = f"{vm_name}-{args.zephyr_version}"

Review comment:
       can you ensure `args.zephyr_version` is not `None`, else it will release microtvm-None, I think?

##########
File path: python/tvm/micro/contrib/zephyr.py
##########
@@ -571,33 +605,49 @@ def write(self, data, timeout_sec):
 class ZephyrQemuTransport(Transport):
     """The user-facing Zephyr QEMU transport class."""
 
-    def __init__(self, base_dir, startup_timeout_sec=5.0, **kwargs):
+    def __init__(self, base_dir, startup_timeout_sec=5.0, debugger=None, **kwargs):
         self.base_dir = base_dir
         self.startup_timeout_sec = startup_timeout_sec
         self.kwargs = kwargs
         self.proc = None
         self.fd_transport = None
         self.pipe_dir = None
+        self.debugger = debugger
 
     def timeouts(self):
         return TransportTimeouts(
             session_start_retry_timeout_sec=2.0,
             session_start_timeout_sec=self.startup_timeout_sec,
-            session_established_timeout_sec=5.0,
+            session_established_timeout_sec=5.0 if self.debugger is None else 0,
         )
 
     def open(self):
         self.pipe_dir = tempfile.mkdtemp()
         self.pipe = os.path.join(self.pipe_dir, "fifo")
         self.write_pipe = os.path.join(self.pipe_dir, "fifo.in")
         self.read_pipe = os.path.join(self.pipe_dir, "fifo.out")
+        
         os.mkfifo(self.write_pipe)
         os.mkfifo(self.read_pipe)
+        if self.debugger is not None:
+            if 'env' in self.kwargs:
+                self.kwargs["env"] = copy.copy(self.kwargs["env"])
+            else:
+                self.kwargs["env"] = copy.copy(os.environ)
+
+            self.kwargs["env"]["TVM_QEMU_DEBUG"] = "1"
+
         self.proc = subprocess.Popen(
-            ["make", "run", f"QEMU_PIPE={self.pipe}"],
+            ["make",
+             "run",
+             f"QEMU_PIPE={self.pipe}"],
             cwd=self.base_dir,
             **self.kwargs,
         )
+        print('START DEBUG', self.debugger)

Review comment:
       delete this line

##########
File path: apps/microtvm/zephyr/demo_runtime/qemu-hack/qemu-system-riscv64
##########
@@ -0,0 +1,38 @@
+#!/bin/bash -e

Review comment:
       I thiiiink you can just checkin a symlink here and `$(basename $0)` will take care of this. could you try it out?
   
   e.g. `ln -s qemu-system-i386 qemu-system-riscv64` in this dir.

##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -358,14 +358,18 @@ def test_command(args):
 
 
 def release_command(args):
+    vm_name = f"mehrdadh/microtvm-{args.platform}"

Review comment:
       can you remove `mehrdadh/`?

##########
File path: apps/microtvm/zephyr/demo_runtime/src/main.c
##########
@@ -265,6 +265,10 @@ static uint8_t main_rx_buf[RING_BUF_SIZE_BYTES];
 // The main function of this application.
 extern void __stdout_hook_install(int (*hook)(int));
 void main(void) {
+  // TODO (mehrdadh): Update this when zephyr version was updated to 2.6. 
+  // Uncomment this for qemu_riscv32, also update zephyr to latest version.

Review comment:
       Actually, can you just use the `#ifdef CONFIG_BOARD_QEMU_RISCV32`?

##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 

Review comment:
       why 512?




-- 
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] [tvm] areusch commented on a change in pull request #7804: [microTVM] Zephyr: RISCV support for Zephyr QEMU RISCV-32/64

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



##########
File path: apps/microtvm/zephyr/demo_runtime/boards/qemu_riscv32.conf
##########
@@ -0,0 +1,28 @@
+# 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.
+
+# This file is specific to the QEMU-emulated RISCV32 microTVM board.
+
+# For TVMPlatformGenerateRandom(). Remember, these values do not need to be truly random.
+CONFIG_TEST_RANDOM_GENERATOR=y
+CONFIG_TIMER_RANDOM_GENERATOR=y
+
+# Default 512, for operations with large floating point data. 

Review comment:
       could you clarify this to "default is 512, raised here for operations with large floating point data."

##########
File path: tests/micro/zephyr/test_zephyr.py
##########
@@ -59,7 +66,7 @@ def _make_sess_from_op(model, zephyr_board, west_cmd, op_name, sched, arg_bufs):
 
 
 def _make_session(model, target, zephyr_board, west_cmd, mod):
-    test_name = f"{os.path.splitext(os.path.abspath(__file__))[0]}_{model}"
+    test_name = f"{os.path.splitext(os.path.abspath(__file__))[0]}_{zephyr_board}"

Review comment:
       why this change? seems like we should put both in the path, if anything

##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -358,14 +358,18 @@ def test_command(args):
 
 
 def release_command(args):
+    vm_name = f"mehrdadh/microtvm-{args.platform}"
+    if args.platform == "zephyr":
+        vm_name = f"{vm_name}-{args.zephyr_version}"

Review comment:
       I think it would be better to not by default publish any version, but instead require that the user explicitly state it. otherwise, we'll wind up publishing lots of zephyr-2.5 boxes :)
   
   also--we could probably call this platform-version, since `tlcpack/microtvm-<platform>-<platform_version>` is a pretty generic format we'll keep if we add more platforms




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