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 2022/05/18 20:22:11 UTC
[GitHub] [tvm] mehrdadh opened a new pull request, #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
mehrdadh opened a new pull request, #11362:
URL: https://github.com/apache/tvm/pull/11362
This PR does the following:
- Adds CMSIS dependencies in Zephyr project API and a test to check this
- disable conv2d_nhwc_dsp.arm_cpu for non integers workload because of the bug
- Change TempDirectory interface to keep it for debugging
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r880846386
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +432,39 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path], cmsis_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(cmsis_path)
+
+ lib_path = pathlib.Path(lib_path)
+ if not lib_path.exists():
+ lib_path.mkdir()
+
+ include_directories = ["CMSIS/DSP/Include", "CMSIS/DSP/Include/dsp", "CMSIS/NN/Include"]
Review Comment:
cool, I'll change 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] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r883001931
##########
tests/scripts/task_python_microtvm.sh:
##########
@@ -28,6 +28,7 @@ make cython3
run_pytest ctypes python-microtvm-zephyr-qemu_x86 tests/micro/zephyr --zephyr-board=qemu_x86
run_pytest ctypes python-microtvm-zephyr-qemu_riscv32 tests/micro/zephyr --zephyr-board=qemu_riscv32
run_pytest ctypes python-microtvm-zephyr-qemu_riscv64 tests/micro/zephyr --zephyr-board=qemu_riscv64
+run_pytest ctypes python-microtvm-zephyr-mps2_an521 tests/micro/zephyr --zephyr-board=mps2_an521
Review Comment:
Only enabling this to run one test for the CMSIS build. The rest are skipped
--
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] areusch commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r880845566
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +432,39 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path], cmsis_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(cmsis_path)
+
+ lib_path = pathlib.Path(lib_path)
+ if not lib_path.exists():
+ lib_path.mkdir()
+
+ include_directories = ["CMSIS/DSP/Include", "CMSIS/DSP/Include/dsp", "CMSIS/NN/Include"]
Review Comment:
yeah the case I was more thinking about was if CMSIS-NN reorganizes and a file inside one of these directories starts depending on a file from another, non-copied directory. I sort of prefer the `-I` for this reason, though it does mean you have to not move CMSIS-NN. On the other hand, examining CMakeLists is a fairly logical thing to do if you get include path problems.
--
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] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r880941733
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +432,39 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path], cmsis_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(cmsis_path)
+
+ lib_path = pathlib.Path(lib_path)
+ if not lib_path.exists():
+ lib_path.mkdir()
+
+ include_directories = ["CMSIS/DSP/Include", "CMSIS/DSP/Include/dsp", "CMSIS/NN/Include"]
Review Comment:
PTAL! 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] alanmacd commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
alanmacd commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r887235029
##########
tests/micro/zephyr/conftest.py:
##########
@@ -76,4 +76,21 @@ def temp_dir(board):
if not os.path.exists(board_workspace.parent):
os.makedirs(board_workspace.parent)
- return tempdir(board_workspace)
+ keep_for_debug = tvm_debug if tvm_debug else None
+ test_temp_dir = tempdir(custom_path=board_workspace, keep_for_debug=keep_for_debug)
+ return test_temp_dir
+
+
+@pytest.fixture(autouse=True)
+def skip_by_board(request, board):
+ """Skip test if board is in the list."""
+ if request.node.get_closest_marker("skip_boards"):
+ if board in request.node.get_closest_marker("skip_boards").args[0]:
+ pytest.skip("skipped on this board: {}".format(board))
+
+
+def pytest_configure(config):
+ config.addinivalue_line(
+ "markers",
+ "skip_by_board(board): skip test for the given board",
Review Comment:
@mehrdadh does this need to be _skip_boards_?
`"skip_boards(board): skip test for the given board",`
--
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] areusch commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r878515012
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +423,28 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(os.environ["CMSIS_PATH"])
Review Comment:
why not make this a project option for now?
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -455,6 +476,15 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
os.makedirs(extract_path)
tf.extractall(path=extract_path)
+ # Add CMSIS libraries if required.
+ if options["project_type"] == "host_driven":
Review Comment:
why only for host-driven?
##########
tests/micro/zephyr/test_zephyr.py:
##########
@@ -503,5 +512,56 @@ def test_autotune_conv2d(temp_dir, board, west_cmd, tvm_debug):
tvm.testing.assert_allclose(output, expected_output, rtol=1e-4, atol=1e-5)
+@tvm.testing.requires_micro
+def test_schedule_build_with_cmsis_dependency(temp_dir, board, west_cmd, tvm_debug):
+ """Test Relay schedule with CMSIS dependency. This test shows if microTVM Auto tuning
+ with Zephyr breaks if CMSIS dependency was required for a schedule.
+ """
+ model = test_utils.ZEPHYR_BOARDS[board]
+ build_config = {"debug": tvm_debug}
+ target = tvm.target.target.micro(model, options=["-keys=arm_cpu,cpu"])
+
+ isa = arm_isa.IsaAnalyzer(target)
+ if not isa.has_dsp_support:
+ pytest.skip(f"ISA does not support DSP. target: {target}")
+
+ # Create a Relay conv2d
+ data_shape = (1, 16, 16, 3)
+ weight_shape = (5, 5, 8, 3)
+ data = relay.var("data", relay.TensorType(data_shape, "int8"))
+ weight = relay.var("weight", relay.TensorType(weight_shape, "int8"))
+ y = relay.nn.conv2d(
+ data,
+ weight,
+ padding=(2, 2),
+ kernel_size=(5, 5),
+ data_layout="NHWC",
+ kernel_layout="HWOI",
+ out_dtype="int32",
+ )
+ func = relay.Function([data, weight], y)
+ ir_mod = tvm.IRModule.from_expr(func)
+
+ runtime = Runtime("crt", {"system-lib": True})
+
+ with tvm.transform.PassContext(opt_level=3, config={"tir.disable_vectorize": True}):
+ mod = tvm.relay.build(ir_mod, target=target, runtime=runtime)
Review Comment:
want to assert that CMSIS is actually used 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r883021622
##########
tests/scripts/task_python_microtvm.sh:
##########
@@ -28,6 +28,7 @@ make cython3
run_pytest ctypes python-microtvm-zephyr-qemu_x86 tests/micro/zephyr --zephyr-board=qemu_x86
run_pytest ctypes python-microtvm-zephyr-qemu_riscv32 tests/micro/zephyr --zephyr-board=qemu_riscv32
run_pytest ctypes python-microtvm-zephyr-qemu_riscv64 tests/micro/zephyr --zephyr-board=qemu_riscv64
+run_pytest ctypes python-microtvm-zephyr-mps2_an521 tests/micro/zephyr --zephyr-board=mps2_an521
Review Comment:
oh oops, ignore. it's okay since your one test is build-only.
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -70,6 +69,8 @@
ZEPHYR_BASE = os.getenv("ZEPHYR_BASE")
+CMSIS_PATH = os.getenv("CMSIS_PATH")
Review Comment:
i think it might be good to avoid hidden env variable dependencies. they can be hard to track down. what do you think?
--
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] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r883052560
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -70,6 +69,8 @@
ZEPHYR_BASE = os.getenv("ZEPHYR_BASE")
+CMSIS_PATH = os.getenv("CMSIS_PATH")
Review Comment:
makes sense, I removed the default value.
--
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] areusch commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r879778735
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +432,39 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path], cmsis_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(cmsis_path)
+
+ lib_path = pathlib.Path(lib_path)
+ if not lib_path.exists():
+ lib_path.mkdir()
+
+ include_directories = ["CMSIS/DSP/Include", "CMSIS/DSP/Include/dsp", "CMSIS/NN/Include"]
Review Comment:
just curious why we copy rather than just add to include path e.g. via `-I` or `target_include_directories` in CMakeLists.txt?
--
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] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r880838554
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +432,39 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path], cmsis_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(cmsis_path)
+
+ lib_path = pathlib.Path(lib_path)
+ if not lib_path.exists():
+ lib_path.mkdir()
+
+ include_directories = ["CMSIS/DSP/Include", "CMSIS/DSP/Include/dsp", "CMSIS/NN/Include"]
Review Comment:
Either would work, this approach has less affect on CMakeLists, also easier for debugging a project since all files would be accessible in the generated project. But I can see that this could be a bad solution if two header files have similar name and we copy everything here. Happy to change it if you suggest?
--
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] areusch commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r882933133
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -70,6 +69,8 @@
ZEPHYR_BASE = os.getenv("ZEPHYR_BASE")
+CMSIS_PATH = os.getenv("CMSIS_PATH")
Review Comment:
do we need this anymore?
##########
tests/scripts/task_python_microtvm.sh:
##########
@@ -28,6 +28,7 @@ make cython3
run_pytest ctypes python-microtvm-zephyr-qemu_x86 tests/micro/zephyr --zephyr-board=qemu_x86
run_pytest ctypes python-microtvm-zephyr-qemu_riscv32 tests/micro/zephyr --zephyr-board=qemu_riscv32
run_pytest ctypes python-microtvm-zephyr-qemu_riscv64 tests/micro/zephyr --zephyr-board=qemu_riscv64
+run_pytest ctypes python-microtvm-zephyr-mps2_an521 tests/micro/zephyr --zephyr-board=mps2_an521
Review Comment:
cc @gromero is this board stable enough to run in CI? i thought we had I/O flakiness 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r879713833
##########
tests/micro/zephyr/test_zephyr.py:
##########
@@ -503,5 +512,56 @@ def test_autotune_conv2d(temp_dir, board, west_cmd, tvm_debug):
tvm.testing.assert_allclose(output, expected_output, rtol=1e-4, atol=1e-5)
+@tvm.testing.requires_micro
+def test_schedule_build_with_cmsis_dependency(temp_dir, board, west_cmd, tvm_debug):
+ """Test Relay schedule with CMSIS dependency. This test shows if microTVM Auto tuning
+ with Zephyr breaks if CMSIS dependency was required for a schedule.
+ """
+ model = test_utils.ZEPHYR_BOARDS[board]
+ build_config = {"debug": tvm_debug}
+ target = tvm.target.target.micro(model, options=["-keys=arm_cpu,cpu"])
+
+ isa = arm_isa.IsaAnalyzer(target)
+ if not isa.has_dsp_support:
+ pytest.skip(f"ISA does not support DSP. target: {target}")
+
+ # Create a Relay conv2d
+ data_shape = (1, 16, 16, 3)
+ weight_shape = (5, 5, 8, 3)
+ data = relay.var("data", relay.TensorType(data_shape, "int8"))
+ weight = relay.var("weight", relay.TensorType(weight_shape, "int8"))
+ y = relay.nn.conv2d(
+ data,
+ weight,
+ padding=(2, 2),
+ kernel_size=(5, 5),
+ data_layout="NHWC",
+ kernel_layout="HWOI",
+ out_dtype="int32",
+ )
+ func = relay.Function([data, weight], y)
+ ir_mod = tvm.IRModule.from_expr(func)
+
+ runtime = Runtime("crt", {"system-lib": True})
+
+ with tvm.transform.PassContext(opt_level=3, config={"tir.disable_vectorize": True}):
+ mod = tvm.relay.build(ir_mod, target=target, runtime=runtime)
Review Comment:
added.
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -424,6 +423,28 @@ def _get_platform_version(self, zephyr_base: str) -> float:
return float(f"{version_major}.{version_minor}")
+ def _load_cmsis(self, lib_path: Union[str, pathlib.Path]):
+ """Copy CMSIS header files to generated project."""
+
+ cmsis_path = pathlib.Path(os.environ["CMSIS_PATH"])
Review Comment:
I think that should be a project options that I missed. Done!
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -455,6 +476,15 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
os.makedirs(extract_path)
tf.extractall(path=extract_path)
+ # Add CMSIS libraries if required.
+ if options["project_type"] == "host_driven":
Review Comment:
I changed it to support both and different module names.
--
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] mehrdadh commented on a diff in pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11362:
URL: https://github.com/apache/tvm/pull/11362#discussion_r883001525
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -70,6 +69,8 @@
ZEPHYR_BASE = os.getenv("ZEPHYR_BASE")
+CMSIS_PATH = os.getenv("CMSIS_PATH")
Review Comment:
yeah, used it as default if project option is not passed
--
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] mehrdadh merged pull request #11362: [microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build
Posted by GitBox <gi...@apache.org>.
mehrdadh merged PR #11362:
URL: https://github.com/apache/tvm/pull/11362
--
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