[microTVM] add the option to open a saved micro project for debugging #12495

mkatanbaf · 2022-08-18T22:45:04Z

This PR adds the option to save a generated micro project at a custom path, and to open a saved project and start communicating with it (instead of generating a new one) for debugging.

cc @alanmacd @areusch @gromero @guberti @mehrdadh

areusch

thanks @mkatanbaf ! cc @mehrdadh @guberti

areusch · 2022-08-31T20:29:56Z

python/tvm/micro/session.py

@@ -259,6 +259,8 @@ def compile_and_create_micro_session(
    mod_src_bytes: bytes,
    template_project_dir: str,
    project_options: dict = None,
+    build_dir: str = None,
+    debug: bool = False,


could we use a more descriptive parameter name? e.g. reuse_project or generate_project: bool = True? alternatively, could derive this from build_dir (e.g. if it already exists, don't regenerate); but that could be troublesome if we want to assert a workflow actually generates a project too.

how about skip_project_generation: bool = False? yes, we can drive it from build_dir but I think that might cause some confusion and it's better to explicitly express it when we would like to skip project generation.

I like reuse_project or use_existing. I also agree that deriving this property from build_dir would cause confusion.

I used use_existing. Thanks @guberti for the suggestion.

areusch · 2022-08-31T20:31:24Z

python/tvm/micro/session.py

+            logging.error("Project Generate Error: %s", str(exception))
+            raise exception
+
+        generated_project.build()


should we still rebuild if we're going to flash? i mean, maybe not..just wondering some rationale here.

based on what I've seen the flash step rebuilds the project if needed. The build step creates the build dir for the first time and I believe it generates an error if the build dir already exists. I'll double check.

I checked and the west flash command rebuilds the project if needed.

I do not believe the Arduino implementation does this - how do we want to handle that? IMO the cleanest/easiest solution is to always rebuild before flashing.

I changed it to clear the existing build dir, and rebuild before flashing.

areusch · 2022-08-31T20:32:06Z

python/tvm/micro/testing/evaluation.py

+                **(project_options or {}),
+            },
+        )
+        project.build()


same question here

guberti

LGTM, just a few nits. I don't have a strong opinion on whether we should rebuild existing projects before flashing - either seems reasonable to me (though I think we would have to remove the existing build dir if we rebuild)

After thinking about it more, I think one of the most common use cases for this feature will be letting folks make manual changes to the generated C++ code, and then evaluating how performance changes. Since calls to this utility will usually be accompanied by code changes, I think rebuilding explicitly by calling project.build() makes sense.

guberti · 2022-09-02T05:14:30Z

python/tvm/micro/session.py

@@ -259,6 +259,8 @@ def compile_and_create_micro_session(
    mod_src_bytes: bytes,
    template_project_dir: str,
    project_options: dict = None,
+    build_dir: str = None,
+    debug: bool = False,


I like reuse_project or use_existing. I also agree that deriving this property from build_dir would cause confusion.

guberti · 2022-09-02T05:18:28Z

python/tvm/micro/testing/evaluation.py

+            {
+                f"{platform}_board": board,
+                "project_type": "host_driven",
+                # {} shouldn't be the default value for project options ({}
+                # is mutable), so we use this workaround
+                **(project_options or {}),


Can we store these in a variable (e.g. project_options) so this code isn't duplicated between the if and else statements?

guberti · 2022-09-02T05:19:56Z

python/tvm/micro/build.py

+        self,
+        template_project_dir: Union[pathlib.Path, str],
+        project_options: dict = None,
+        build_dir: str = None,


Can we give build_dir a path-like object type instead of just str? i.e. Union[os.PathLike, str]?

areusch · 2022-09-07T19:57:03Z

@guberti can you have another look?

guberti

Two small comments, but this still looks good.

guberti · 2022-09-08T15:03:59Z

python/tvm/micro/build.py

+        template_project_dir: Union[pathlib.Path, str],
+        project_options: dict = None,
+        project_dir: Union[pathlib.Path, str] = None,


Nit: the Python docs suggest using os.PathLike for typechecking, which is the abstract base class for pathlib.Path.

Suggested change

template_project_dir: Union[pathlib.Path, str],

project_options: dict = None,

project_dir: Union[pathlib.Path, str] = None,

template_project_dir: Union[os.PathLike, str],

project_options: dict = None,

project_dir: Union[os.PathLike, str] = None,

guberti · 2022-09-08T15:05:22Z

python/tvm/micro/session.py

+
+    if use_existing:
+        project_dir = pathlib.Path(project_dir)
+        assert pathlib.Path(project_dir).is_dir(), f"{project_dir} does not exist."


No need to call pathlib.Path twice.

Suggested change

assert pathlib.Path(project_dir).is_dir(), f"{project_dir} does not exist."

assert project_dir.is_dir(), f"{project_dir} does not exist."

mkatanbaf · 2022-09-12T21:37:49Z

@tvm-bot rerun

mkatanbaf · 2022-09-13T00:44:44Z

@tvm-bot rerun

guberti

LGTM - thanks for the fix!

mkatanbaf · 2022-09-13T07:02:54Z

@tvm-bot rerun

mkatanbaf · 2022-09-13T19:39:12Z

@tvm-bot rerun

mkatanbaf · 2022-09-14T22:04:47Z

@tvm-bot rerun

mkatanbaf · 2022-09-21T18:04:20Z

@areusch could you take another look please?

mkatanbaf · 2022-09-26T17:18:18Z

@tvm-bot rerun

…apache#12495) * add the option to open a saved project for debugging. * addressing comments Co-authored-by: Mohamad <mkatanbaf@users.noreply.github.com>

mkatanbaf changed the title ~~[microTVM] add the option to open a saved project for debugging~~ [microTVM] add the option to open a saved micro project for debugging Aug 18, 2022

mkatanbaf force-pushed the debug_autotune_projects branch from f0da470 to 25c8e77 Compare August 30, 2022 18:09

mkatanbaf marked this pull request as ready for review August 31, 2022 16:57

github-actions bot requested review from areusch and mehrdadh August 31, 2022 17:07

areusch reviewed Aug 31, 2022

View reviewed changes

guberti approved these changes Sep 2, 2022

View reviewed changes

github-actions bot requested a review from gromero September 2, 2022 07:42

mkatanbaf force-pushed the debug_autotune_projects branch from 6c6af0f to 32a652a Compare September 2, 2022 08:28

guberti approved these changes Sep 8, 2022

View reviewed changes

guberti approved these changes Sep 13, 2022

View reviewed changes

add the option to open a saved project for debugging.

9ccdd30

mkatanbaf force-pushed the debug_autotune_projects branch from 1b75133 to 9ccdd30 Compare September 15, 2022 17:08

addressing comments

fb8aea5

areusch approved these changes Sep 27, 2022

View reviewed changes

areusch merged commit 82e6fc4 into apache:main Sep 27, 2022

leandron mentioned this pull request Feb 1, 2023

TVM v0.11.0 Release Candidate Notes #13899

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[microTVM] add the option to open a saved micro project for debugging #12495

[microTVM] add the option to open a saved micro project for debugging #12495

mkatanbaf commented Aug 18, 2022 •

edited by github-actions bot

Loading

areusch left a comment

areusch Aug 31, 2022

mkatanbaf Aug 31, 2022

guberti Sep 2, 2022

mkatanbaf Sep 2, 2022

areusch Aug 31, 2022

mkatanbaf Aug 31, 2022 •

edited

Loading

mkatanbaf Sep 2, 2022

guberti Sep 2, 2022 •

edited

Loading

mkatanbaf Sep 2, 2022

areusch Aug 31, 2022

guberti left a comment •

edited

Loading

guberti Sep 2, 2022

guberti Sep 2, 2022

mkatanbaf Sep 2, 2022

guberti Sep 2, 2022

mkatanbaf Sep 2, 2022

areusch commented Sep 7, 2022

guberti left a comment

guberti Sep 8, 2022

guberti Sep 8, 2022

mkatanbaf commented Sep 12, 2022

mkatanbaf commented Sep 13, 2022

guberti left a comment

mkatanbaf commented Sep 13, 2022

mkatanbaf commented Sep 13, 2022

mkatanbaf commented Sep 14, 2022

mkatanbaf commented Sep 21, 2022

mkatanbaf commented Sep 26, 2022

	assert pathlib.Path(project_dir).is_dir(), f"{project_dir} does not exist."
	assert project_dir.is_dir(), f"{project_dir} does not exist."

[microTVM] add the option to open a saved micro project for debugging #12495

[microTVM] add the option to open a saved micro project for debugging #12495

Conversation

mkatanbaf commented Aug 18, 2022 • edited by github-actions bot Loading

areusch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkatanbaf Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guberti Sep 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guberti left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch commented Sep 7, 2022

guberti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkatanbaf commented Sep 12, 2022

mkatanbaf commented Sep 13, 2022

guberti left a comment

Choose a reason for hiding this comment

mkatanbaf commented Sep 13, 2022

mkatanbaf commented Sep 13, 2022

mkatanbaf commented Sep 14, 2022

mkatanbaf commented Sep 21, 2022

mkatanbaf commented Sep 26, 2022

mkatanbaf commented Aug 18, 2022 •

edited by github-actions bot

Loading

mkatanbaf Aug 31, 2022 •

edited

Loading

guberti Sep 2, 2022 •

edited

Loading

guberti left a comment •

edited

Loading