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/06/14 17:33:41 UTC
[GitHub] [tvm] gromero commented on pull request #8055: apps: microtvm: Disable `CONFIG_FPU ` for Zephyr runtime
gromero commented on pull request #8055:
URL: https://github.com/apache/tvm/pull/8055#issuecomment-860863569
@microbuilder @mehrdadh Hi folks, sorry for the delay on reviewing it.
@mehrdadh thanks for the additional checks!
I figured out why the link error pasted above happened on my local environment but not on the CI.
It happens that the CI is using Zephyr SDK 0.12 whilst I was using Zephyr SDK 0.11 and the following Zephyr SDK fix is only in 0.12:
```
commit 2c1077298b169c39a1badb4d4a4236a2f2eaf769
Author: Daniel Leung <da...@intel.com>
Date: Thu Dec 10 12:27:08 2020 -0800
x86_64: fix soft float for x86 32-bit
This fixes soft-float build for x86 32-bit (-m32 -msoft-float)
under x86_64-zephyr-elf multilib build. This now actually
includes the soft float functions.
Signed-off-by: Daniel Leung <da...@intel.com>
```
hence the soft-float functions were not present in my environment, causing the linking errors when the patch in question is applied.
That, on the other hand, exposed the fact that `qemu_x86` .elf image was not really using `CONFIG_FPU=y` and that can be confirmed by looking for soft-float specific functions present (or absent) in the final .elf image when its built with Zephyr SDK 0.12 (fixed version). Hence no soft-float functions are present in the `qemu_x86` image without the patch applied:
```
$ objdump -t ./zephyr.elf | fgrep float
$
```
but are included (and used) in the .elf image when the patch is applied (for `CONFIG_FPU=y` is not kicking in):
```
$ objdump -t ./zephyr.elf | fgrep float
00000000 l df *ABS* 00000000 floatunsidf.c
00000000 l df *ABS* 00000000 floatsidf.c
00000000 l df *ABS* 00000000 soft_float_stubs.c
00103020 g F text 0000007f .hidden __floatsidf
00101dd0 g F text 00000084 .hidden __floatunsidf
$
$ objdump -t ./zephyr.elf | fgrep __subdf3
001012b0 g F text 00000b1e .hidden __subdf3
```
This seems expected because although `CPU_HAS_FPU` is selected in Zephyr's `boards/x86/qemu_x86/Kconfig.board` `CONFIG_FPU=y` is not set by default in Zephyr for that board. `CONFIG_FPU` depends on `CPU_HAS_FPU` to be set but setting `CPU_HAS_FPU` doesn't imply `CONFIG_FPU=y` in my understanding.
Thus one might think it was just a matter of enabling `CONFIG_FPU` per board, like:
```
diff --git a/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf b/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf
index f314f59a5..12c67367f 100644
--- a/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf
@@ -23,3 +23,5 @@ CONFIG_TIMER_RANDOM_GENERATOR=y
# Default stack size is 1k, this is required for debug mode.
CONFIG_MAIN_STACK_SIZE=1536
+
+CONFIG_FPU=y
```
However although the build will finish ok the following error will be caught by the CI when testing against models that rely on floating point operations:
```
____________________________________________________________________ test_byoc_utvm[host] ____________________________________________________________________
platform = 'host', west_cmd = 'west', skip_build = False, tvm_debug = False
def test_byoc_utvm(platform, west_cmd, skip_build, tvm_debug):
"""This is a simple test case to check BYOC capabilities of uTVM"""
model, zephyr_board = PLATFORMS[platform]
build_config = {"skip_build": skip_build, "debug": tvm_debug}
x = relay.var("x", shape=(10, 10))
w0 = relay.var("w0", shape=(10, 10))
w1 = relay.var("w1", shape=(10, 10))
w2 = relay.var("w2", shape=(10, 10))
w3 = relay.var("w3", shape=(10, 10))
w4 = relay.var("w4", shape=(10, 10))
w5 = relay.var("w5", shape=(10, 10))
w6 = relay.var("w6", shape=(10, 10))
w7 = relay.var("w7", shape=(10, 10))
# C compiler
z0 = relay.add(x, w0)
p0 = relay.subtract(z0, w1)
q0 = relay.multiply(p0, w2)
z1 = relay.add(x, w3)
p1 = relay.subtract(z1, w4)
q1 = relay.multiply(p1, w5)
# Other parts on TVM
z2 = relay.add(x, w6)
q2 = relay.subtract(z2, w7)
r = relay.concatenate((q0, q1, q2), axis=0)
f = relay.Function([x, w0, w1, w2, w3, w4, w5, w6, w7], r)
mod = tvm.IRModule()
ann = CcompilerAnnotator()
mod["main"] = ann.visit(f)
mod = tvm.relay.transform.PartitionGraph()(mod)
mod = tvm.relay.transform.InferType()(mod)
x_data = np.random.rand(10, 10).astype("float32")
w_data = []
for _ in range(8):
w_data.append(np.random.rand(10, 10).astype("float32"))
map_inputs = {"w{}".format(i): w_data[i] for i in range(8)}
map_inputs["x"] = x_data
> check_result(
relay_mod=mod,
map_inputs=map_inputs,
out_shape=(30, 10),
result=np.concatenate(
(
((x_data + w_data[0]) - w_data[1]) * w_data[2],
((x_data + w_data[3]) - w_data[4]) * w_data[5],
x_data + w_data[6] - w_data[7],
),
axis=0,
),
model=model,
zephyr_board=zephyr_board,
west_cmd=west_cmd,
build_config=build_config,
)
tests/micro/zephyr/test_zephyr.py:379:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/micro/zephyr/test_zephyr.py:334: in check_result
tvm.testing.assert_allclose(out.numpy(), results[idx], rtol=TOL, atol=TOL)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
actual = array([[-0.0e+00, 7.0e-44, 7.0e-44, 7.0e-44, -0.0e+00, -0.0e+00,
7.0e-44, 7.0e-44, 7.0e-44, 7.0e-44],
...-44, 4.8e-44, -0.0e+00, 4.8e-44, 4.8e-44, -0.0e+00,
4.8e-44, 4.8e-44, 4.8e-44, 4.8e-44]], dtype=float32)
desired = array([[ 3.30861807e-01, 2.14869566e-02, -3.43096778e-02,
-3.45205562e-03, 2.59472132e-01, 2.34518517e-02,
..., 1.21057093e+00,
5.22404790e-01, 8.47700536e-01, -2.38762259e-01,
-7.96570182e-02]], dtype=float32)
rtol = 1e-05, atol = 1e-05
def assert_allclose(actual, desired, rtol=1e-7, atol=1e-7):
"""Version of np.testing.assert_allclose with `atol` and `rtol` fields set
in reasonable defaults.
Arguments `actual` and `desired` are not interchangable, since the function
compares the `abs(actual-desired)` with `atol+rtol*abs(desired)`. Since we
often allow `desired` to be close to zero, we generally want non-zero `atol`.
"""
actual = np.asanyarray(actual)
desired = np.asanyarray(desired)
np.testing.assert_allclose(actual.shape, desired.shape)
> np.testing.assert_allclose(actual, desired, rtol=rtol, atol=atol, verbose=True)
E AssertionError:
E Not equal to tolerance rtol=1e-05, atol=1e-05
E
E Mismatched elements: 300 / 300 (100%)
E Max absolute difference: 1.7392546
E Max relative difference: 1.
E x: array([[-0.0e+00, 7.0e-44, 7.0e-44, 7.0e-44, -0.0e+00, -0.0e+00,
E 7.0e-44, 7.0e-44, 7.0e-44, 7.0e-44],
E [ 7.0e-44, -0.0e+00, 7.0e-44, -0.0e+00, -0.0e+00, 7.0e-44,...
E y: array([[ 3.308618e-01, 2.148696e-02, -3.430968e-02, -3.452056e-03,
E 2.594721e-01, 2.345185e-02, 6.401843e-01, -9.157152e-02,
E 7.447193e-01, -9.966984e-03],...
python/tvm/testing.py:82: AssertionError
```
This happens because microTVM-Zephyr build system ends up linking mixed objects, i.e. objects built with soft-float and without soft-float functions. This bug was recently fixed by PR https://github.com/apache/tvm/pull/8230, which now allows to set `CONFIG_FPU` per board correctly.
Kevin, since Zephyr defaults regarding CONFIG_FPU might not be what microTVM needs (which is ok) I think we need to enabled FPU per board, for each board (@) So could you please update your patch? It also needs to be rebased since `demo_runtime` was renamed to `host_driven`. Also, there is now an app for AOT, so `CONFIG_FPU` configs in `host_driven` must now be set for `aot_driven`. As a reference, here is a change that works ok for me:
```
diff --git a/apps/microtvm/zephyr/aot_demo/boards/nrf5340dk_nrf5340_cpuapp.conf b/apps/microtvm/zephyr/aot_demo/boards/nrf5340dk_nrf5340_cpuapp.conf
index d298325eb..6c588c86b 100644
--- a/apps/microtvm/zephyr/aot_demo/boards/nrf5340dk_nrf5340_cpuapp.conf
+++ b/apps/microtvm/zephyr/aot_demo/boards/nrf5340dk_nrf5340_cpuapp.conf
@@ -29,3 +29,6 @@ CONFIG_TEST_RANDOM_GENERATOR=y
# For debugging.
CONFIG_LED=y
+
+# For models with floating point.
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/aot_demo/boards/qemu_x86.conf b/apps/microtvm/zephyr/aot_demo/boards/qemu_x86.conf
index 5f3c4a4be..4b0e49406 100644
--- a/apps/microtvm/zephyr/aot_demo/boards/qemu_x86.conf
+++ b/apps/microtvm/zephyr/aot_demo/boards/qemu_x86.conf
@@ -23,3 +23,6 @@ CONFIG_TIMER_RANDOM_GENERATOR=y
# Default stack size is 1k, this is required for debug mode.
CONFIG_MAIN_STACK_SIZE=2000
+
+# For models with floating point.
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/aot_demo/prj.conf b/apps/microtvm/zephyr/aot_demo/prj.conf
index 5f4d7a068..c6ab10e9d 100644
--- a/apps/microtvm/zephyr/aot_demo/prj.conf
+++ b/apps/microtvm/zephyr/aot_demo/prj.conf
@@ -28,8 +28,5 @@ CONFIG_UART_INTERRUPT_DRIVEN=y
CONFIG_CPLUSPLUS=y
CONFIG_NEWLIB_LIBC=y
-# For models with floating point.
-CONFIG_FPU=y
-
# For TVMPlatformAbort().
CONFIG_REBOOT=y
diff --git a/apps/microtvm/zephyr/host_driven/boards/nrf5340dk_nrf5340_cpuapp.conf b/apps/microtvm/zephyr/host_driven/boards/nrf5340dk_nrf5340_cpuapp.conf
index 149a69ea3..511ff0121 100644
--- a/apps/microtvm/zephyr/host_driven/boards/nrf5340dk_nrf5340_cpuapp.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/nrf5340dk_nrf5340_cpuapp.conf
@@ -29,3 +29,6 @@ CONFIG_TEST_RANDOM_GENERATOR=y
# For debugging.
CONFIG_LED=y
+
+# For models with floating point.
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/host_driven/boards/nucleo_f746zg.conf b/apps/microtvm/zephyr/host_driven/boards/nucleo_f746zg.conf
index eba023294..33b08032c 100644
--- a/apps/microtvm/zephyr/host_driven/boards/nucleo_f746zg.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/nucleo_f746zg.conf
@@ -28,3 +28,6 @@ CONFIG_ENTROPY_GENERATOR=y
# For debugging.
CONFIG_LED=y
+
+# For models with floating point.
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/host_driven/boards/qemu_riscv32.conf b/apps/microtvm/zephyr/host_driven/boards/qemu_riscv32.conf
index 3733568ed..b94d96b11 100644
--- a/apps/microtvm/zephyr/host_driven/boards/qemu_riscv32.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/qemu_riscv32.conf
@@ -24,6 +24,9 @@ CONFIG_TIMER_RANDOM_GENERATOR=y
# Default is 512, raised here for operations with large floating point data.
CONFIG_MAIN_STACK_SIZE=2048
+# For models with floating point.
+CONFIG_FPU=y
+
# For floating point operations. It has exception on floating point operations
# without this flag.
CONFIG_FPU_SHARING=y
diff --git a/apps/microtvm/zephyr/host_driven/boards/qemu_riscv64.conf b/apps/microtvm/zephyr/host_driven/boards/qemu_riscv64.conf
index a8a055bcc..1da5f054d 100644
--- a/apps/microtvm/zephyr/host_driven/boards/qemu_riscv64.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/qemu_riscv64.conf
@@ -23,3 +23,6 @@ CONFIG_TIMER_RANDOM_GENERATOR=y
# Default 512, for operations with large floating point data.
CONFIG_MAIN_STACK_SIZE=2048
+
+# For models with floating point.
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf b/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf
index f314f59a5..12c67367f 100644
--- a/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/qemu_x86.conf
@@ -23,3 +23,5 @@ CONFIG_TIMER_RANDOM_GENERATOR=y
# Default stack size is 1k, this is required for debug mode.
CONFIG_MAIN_STACK_SIZE=1536
+
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/host_driven/boards/stm32f746g_disco.conf b/apps/microtvm/zephyr/host_driven/boards/stm32f746g_disco.conf
index 505f1babc..542faf28c 100644
--- a/apps/microtvm/zephyr/host_driven/boards/stm32f746g_disco.conf
+++ b/apps/microtvm/zephyr/host_driven/boards/stm32f746g_disco.conf
@@ -26,3 +26,6 @@ CONFIG_TEST_RANDOM_GENERATOR=y
# For debugging.
CONFIG_LED=n
+
+# For models with floating point.
+CONFIG_FPU=y
diff --git a/apps/microtvm/zephyr/host_driven/prj.conf b/apps/microtvm/zephyr/host_driven/prj.conf
index 5f4d7a068..c6ab10e9d 100644
--- a/apps/microtvm/zephyr/host_driven/prj.conf
+++ b/apps/microtvm/zephyr/host_driven/prj.conf
@@ -28,8 +28,5 @@ CONFIG_UART_INTERRUPT_DRIVEN=y
CONFIG_CPLUSPLUS=y
CONFIG_NEWLIB_LIBC=y
-# For models with floating point.
-CONFIG_FPU=y
-
# For TVMPlatformAbort().
CONFIG_REBOOT=y
```
----
For the records, the "best-case scenario warning" about conflicts during builds mentioned by Kevin in his original commit message is the one that currently happens on the CI, i.e. [0]:
```
Including boilerplate (Zephyr base): /opt/zephyrproject/zephyr/cmake/app/boilerplate.cmake
warning: FPU (defined at soc/arm/nxp_imx/mcimx6x_m4/Kconfig.defconfig.mcimx6x_m4:11,
arch/Kconfig:744) was assigned the value 'y' but got the value 'n'. Check these unsatisfied
dependencies: ((SOC_MCIMX6X_M4 && SOC_SERIES_IMX_6X_M4) || (CPU_HAS_FPU && (ARC || ARM || RISCV ||
SPARC || X86))) (=n). See http://docs.zephyrproject.org/latest/reference/kconfig/CONFIG_FPU.html
and/or look up FPU in the menuconfig/guiconfig interface. The Application Development Primer,
Setting Configuration Values, and Kconfig - Tips and Best Practices sections of the manual might be
helpful too.
CMake Deprecation Warning at /opt/zephyrproject/modules/lib/civetweb/CMakeLists.txt:2 (cmake_minimum_required):
Compatibility with CMake < 2.8.12 will be removed from a future version of
CMake.
Update the VERSION argument <min> value or use a ...<max> suffix to tell
CMake that the project does not need compatibility with older versions.
```
[0] In https://ci.tlcpack.ai/blue/rest/organizations/jenkins/pipelines/tvm/branches/PR-8110/runs/44/nodes/46/steps/154/log/?start=0
--
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