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

[GitBox <gi...@apache.org>]

manupa-arm commented on a change in pull request #7333:
URL: https://github.com/apache/tvm/pull/7333#discussion_r573597149



##########
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:
       Again on the same question as above, however this kind of makes me think do we need something like an environment variable ? Maybe PassContext is a better option for that ? What do you think ?

##########
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:
       Im not too sure whether we need this ? 

##########
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:
       [Clarification] why is this needed suddenly ?

##########
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:
       [Clarification] do we not need them anymore or do they go inside build/crt/include ?




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