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/02/10 19:21:46 UTC

[GitHub] [tvm] areusch commented on a change in pull request #7333: [µTVM] Use standalone_crt build tree for all µTVM builds

areusch commented on a change in pull request #7333:
URL: https://github.com/apache/tvm/pull/7333#discussion_r574006510



##########
File path: python/tvm/micro/build.py
##########
@@ -55,15 +57,62 @@ def path(self):
 CRT_RUNTIME_LIB_NAMES = ["utvm_rpc_server", "utvm_rpc_common", "common"]
 
 
-TVM_ROOT_DIR = os.path.realpath(os.path.join(os.path.dirname(__file__), "..", "..", ".."))
+STANDALONE_CRT_DIR = None

Review comment:
       do you mean that it's fine to just do the search every time?

##########
File path: python/tvm/micro/build.py
##########
@@ -72,52 +121,73 @@ def path(self):
 _COMMON_CFLAGS = ["-Wall", "-Werror"]
 
 
-_CRT_DEFAULT_OPTIONS = {
-    "cflags": ["-std=c11"] + _COMMON_CFLAGS,
-    "ccflags": ["-std=c++11"] + _COMMON_CFLAGS,
-    "ldflags": ["-std=c++11"],
-    "include_dirs": [
-        f"{TVM_ROOT_DIR}/include",
-        f"{TVM_ROOT_DIR}/3rdparty/dlpack/include",
-        f"{TVM_ROOT_DIR}/3rdparty/libcrc/include",
-        f"{TVM_ROOT_DIR}/3rdparty/dmlc-core/include",

Review comment:
       they go in build/standalone_crt/include according to cmake/modules/StandaloneCrt.cmake.

##########
File path: python/tvm/micro/build.py
##########
@@ -55,15 +57,62 @@ def path(self):
 CRT_RUNTIME_LIB_NAMES = ["utvm_rpc_server", "utvm_rpc_common", "common"]
 
 
-TVM_ROOT_DIR = os.path.realpath(os.path.join(os.path.dirname(__file__), "..", "..", ".."))
+STANDALONE_CRT_DIR = None
 
 
-CRT_ROOT_DIR = os.path.join(TVM_ROOT_DIR, "src", "runtime", "crt")
+class CrtNotFoundError(Exception):
+    """Raised when the standalone CRT dirtree cannot be found."""
 
 
-RUNTIME_LIB_SRC_DIRS = [os.path.join(CRT_ROOT_DIR, n) for n in CRT_RUNTIME_LIB_NAMES] + [
-    os.path.join(TVM_ROOT_DIR, "3rdparty/libcrc/src")
-]
+def get_standalone_crt_dir() -> str:
+    """Find the standalone_crt directory.
+
+    Though the C runtime source lives in the tvm tree, it is intended to be distributed with any
+    binary build of TVM. This source tree is intended to be integrated into user projects to run
+    models targeted with --runtime=c.
+
+    Returns
+    -------
+    str :
+        The path to the standalone_crt
+    """
+    global STANDALONE_CRT_DIR
+    if STANDALONE_CRT_DIR is None:

Review comment:
       I think that ultimately these are going to become data dependencies of the tlcpack wheel and we don't have a standard facility to figure out where the data deps go. I think we should make such a thing in both Python and CMake and then it would reduce this function to: `if not isdir(f'{data_dir}/standalone_crt'): raise CrtNotFoundError()`.

##########
File path: python/tvm/micro/build.py
##########
@@ -72,52 +121,73 @@ def path(self):
 _COMMON_CFLAGS = ["-Wall", "-Werror"]
 
 
-_CRT_DEFAULT_OPTIONS = {
-    "cflags": ["-std=c11"] + _COMMON_CFLAGS,
-    "ccflags": ["-std=c++11"] + _COMMON_CFLAGS,
-    "ldflags": ["-std=c++11"],
-    "include_dirs": [
-        f"{TVM_ROOT_DIR}/include",
-        f"{TVM_ROOT_DIR}/3rdparty/dlpack/include",
-        f"{TVM_ROOT_DIR}/3rdparty/libcrc/include",
-        f"{TVM_ROOT_DIR}/3rdparty/dmlc-core/include",
-        f"{CRT_ROOT_DIR}/include",
-    ],
-}
+def _build_default_compiler_options(standalone_crt_dir: typing.Optional[str] = None) -> str:
+    """Return a dict containing base compile flags for the CRT under gcc common to .
 
+    Parameters
+    ----------
+    standalone_crt_dir : Optional[str]
+        If given, the path to the standalone_crt
+    """
+    if standalone_crt_dir is None:
+        standalone_crt_dir = get_standalone_crt_dir()
+    return {
+        "cflags": ["-std=c11"] + _COMMON_CFLAGS,
+        "ccflags": ["-std=c++11"] + _COMMON_CFLAGS,
+        "ldflags": ["-std=c++11"],
+        "include_dirs": [os.path.join(standalone_crt_dir, "include")],
+    }
 
-_CRT_GENERATED_LIB_OPTIONS = copy.copy(_CRT_DEFAULT_OPTIONS)
 
+def default_options(crt_config_include_dir, standalone_crt_dir=None):
+    """Return default opts passed to Compile commands.
+
+    Parameters
+    ----------
+    crt_config_include_dir : str
+        Path to a directory containing crt_config.h for the target. This will be appended
+        to the include path for cflags and ccflags.
+    standalone_crt_dir : Optional[str]
+
+    Returns
+    -------
+    Dict :
+        A dictionary containing 3 subkeys, each whose value is _build_default_compiler_options()
+        plus additional customization.
+         - "bin_opts" - passed as "options" to Compiler.binary() when building MicroBinary.
+         - "lib_opts" - passed as "options" to Compiler.library() when building bundled CRT
+           libraries (or otherwise, non-generated libraries).
+         - "generated_lib_opts" - passed as "options" to Compiler.library() when building the
+           generated library.
+    """
+    bin_opts = _build_default_compiler_options(standalone_crt_dir)
+    bin_opts["include_dirs"].append(crt_config_include_dir)
 
-# Disable due to limitation in the TVM C codegen, which generates lots of local variable
-# declarations at the top of generated code without caring whether they're used.
-# Example:
-#   void* arg0 = (((TVMValue*)args)[0].v_handle);
-#   int32_t arg0_code = ((int32_t*)arg_type_ids)[(0)];
-_CRT_GENERATED_LIB_OPTIONS["cflags"].append("-Wno-unused-variable")
-_CRT_GENERATED_LIB_OPTIONS["ccflags"].append("-Wno-unused-variable")
+    lib_opts = _build_default_compiler_options(standalone_crt_dir)
+    lib_opts["cflags"] = ["-Wno-error=incompatible-pointer-types"]

Review comment:
       I think it's on line 110 at main--I don't think it's a change 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